Crash in CLEAR_IF_FIRST handling in tdb
Volker Lendecke
Volker.Lendecke at SerNet.DE
Fri Oct 5 02:11:47 MDT 2012
On Thu, Oct 04, 2012 at 05:02:55PM -0700, Jeremy Allison wrote:
> On Tue, Oct 02, 2012 at 04:35:31PM +0200, Volker Lendecke wrote:
> > Hi Rusty!
> >
> > Under
> > http://git.samba.org/?p=vl/samba.git/.git;a=shortlog;h=refs/heads/tdb
> > find a patchset that for me fixes a crash in winbind in tdb.
> > For the explanation, see the second patch from the top.
> >
> > Comments?
>
> Ok, as requested I'm looking into this code :-).
>
> I have a question about the second patch in the
> series, "tdb: Make tdb_expand robust against shrinking tdbs".
>
> This adds an fstat into the tdb_expand_file()
> call just before calling ftruncate, and updates
> tdb->map_size and the passed in size, which
> is actually the value tdb->map_size (so the
> size variable is actually redundent, but
> that's another matter).
True. We could probably just dump the "size" parameter from
the expand_file function.
> tdb_expand_file() is called from tdb_expand(), in
> the code that looks like this:
>
> if (tdb_lock(tdb, -1, F_WRLCK) == -1) {
> TDB_LOG((tdb, TDB_DEBUG_ERROR, "lock failed in tdb_expand\n"));
> return -1;
> }
>
> /* must know about any previous expansions by another process */
> tdb->methods->tdb_oob(tdb, tdb->map_size, 1, 1);
>
> size = tdb_expand_adjust(tdb->map_size, size, tdb->page_size);
>
> /* expand the file itself */
> if (!(tdb->flags & TDB_INTERNAL)) {
> if (tdb->methods->tdb_expand_file(tdb, tdb->map_size, size) != 0)
> goto fail;
> }
>
> However, inside tdb_oob() we already call
> fstat(tdb->fd, &st), and then set:
> tdb->map_size = st.st_size;
> before returning from the tdb_oob() call.
>
> So we've already done an fstat and updated
> tdb->map_size under the F_WRLCK inside tdb_expand(),
> so why is this particular patch needed ?
Look at the code fragment in tdb_oob():
if (st.st_size < (size_t)off + len) {
if (!probe) {
/* Ensure ecode is set for log fn. */
tdb->ecode = TDB_ERR_IO;
TDB_LOG((tdb, TDB_DEBUG_FATAL,"tdb_oob len %u beyond eof at %u\n",
(int)(off + len), (int)st.st_size));
}
return -1;
}
So if the file has shrunk before tdb_oob, this won't do
anything but bail. tdb_oob is only to expand files, not
shrink. That's why that patch is necessary.
Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
More information about the samba-technical
mailing list