Crash in CLEAR_IF_FIRST handling in tdb
Rusty Russell
rusty at rustcorp.com.au
Fri Oct 5 00:22:43 MDT 2012
Jeremy Allison <jra at samba.org> writes:
> 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).
>
> 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.
Well, tdb_oob won't actually, because the file has actually shrunk:
if (st.st_size < (size_t)off + len) {
...
return -1;
}
But I do think it can be polished a little, to move that check to the
bottom so we always map and remap when the size changes, even if it
shrinks. That not only covers this case, but tdb_check and tdb_rescue
as well which use the same tdb_oob(tdb, tdb->map_size, 1, 1) trick.
Volker, what do you think of this patch in place of your "tdb: Make
tdb_expand robust against shrinking tdbs"?
Subject: tdb: Make robust against shrinking tdbs
When probing for a size change (eg. just before tdb_expand, tdb_check,
tdb_rescue) we call tdb_oob(tdb, tdb->map_size, 1, 1). Unfortunately
this does nothing if the tdb has actually shrunk, which as Volker
demonstrated, can actually happen if a "longlived" parent crashes.
So move the map/update size/remap before the limit check.
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
diff --git a/lib/tdb/common/io.c b/lib/tdb/common/io.c
index caf2294..25968bf 100644
--- a/lib/tdb/common/io.c
+++ b/lib/tdb/common/io.c
@@ -63,16 +63,6 @@ static int tdb_oob(struct tdb_context *tdb, tdb_off_t off, tdb_len_t len,
return -1;
}
- 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;
- }
-
/* Beware >4G files! */
if ((tdb_off_t)st.st_size != st.st_size) {
/* Ensure ecode is set for log fn. */
@@ -82,13 +72,31 @@ static int tdb_oob(struct tdb_context *tdb, tdb_off_t off, tdb_len_t len,
return -1;
}
- /* Unmap, update size, remap */
+ /* Unmap, update size, remap. We do this unconditionally, to handle
+ * the unusual case where the db is truncated.
+ *
+ * This can happen to a child using tdb_reopen_all(true) on a
+ * TDB_CLEAR_IF_FIRST tdb whose parent crashes: the next
+ * opener will truncate the database. */
if (tdb_munmap(tdb) == -1) {
tdb->ecode = TDB_ERR_IO;
return -1;
}
tdb->map_size = st.st_size;
- return tdb_mmap(tdb);
+ if (tdb_mmap(tdb) != 0) {
+ return - 1;
+ }
+
+ 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;
+ }
+ return 0;
}
/* write a lump of data at a specified offset */
More information about the samba-technical
mailing list