From: Rusty Russell Date: Mon, 19 Dec 2011 05:17:50 +0000 (+1030) Subject: tdb: be more careful on 4G files. X-Git-Url: http://git.samba.org/?p=mat%2Fsamba.git;a=commitdiff_plain;h=b64494535dc62f4073fc6302847593ed6e6ec38b tdb: be more careful on 4G files. I came across a tdb which had wrapped to 4G + 4K, and the contents had been destroyed by processes which thought it only 4k long. Fix this by checking on open, and making tdb_oob() check for wrap itself. Signed-off-by: Rusty Russell Autobuild-User: Rusty Russell Autobuild-Date: Mon Dec 19 07:52:01 CET 2011 on sn-devel-104 --- diff --git a/lib/tdb/common/check.c b/lib/tdb/common/check.c index 3387fbd098..313f55cbb0 100644 --- a/lib/tdb/common/check.c +++ b/lib/tdb/common/check.c @@ -92,7 +92,7 @@ static bool tdb_check_record(struct tdb_context *tdb, off, rec->next)); goto corrupt; } - if (tdb->methods->tdb_oob(tdb, rec->next+sizeof(*rec), 0)) + if (tdb->methods->tdb_oob(tdb, rec->next, sizeof(*rec), 0)) goto corrupt; /* Check rec_len: similar to rec->next, implies next record. */ @@ -110,7 +110,7 @@ static bool tdb_check_record(struct tdb_context *tdb, goto corrupt; } /* OOB allows "right at the end" access, so this works for last rec. */ - if (tdb->methods->tdb_oob(tdb, off+sizeof(*rec)+rec->rec_len, 0)) + if (tdb->methods->tdb_oob(tdb, off, sizeof(*rec)+rec->rec_len, 0)) goto corrupt; /* Check tailer. */ @@ -345,7 +345,7 @@ _PUBLIC_ int tdb_check(struct tdb_context *tdb, } /* Make sure we know true size of the underlying file. */ - tdb->methods->tdb_oob(tdb, tdb->map_size + 1, 1); + tdb->methods->tdb_oob(tdb, tdb->map_size, 1, 1); /* Header must be OK: also gets us the recovery ptr, if any. */ if (!tdb_check_header(tdb, &recovery_start)) diff --git a/lib/tdb/common/freelist.c b/lib/tdb/common/freelist.c index 927078a7aa..6358f64a04 100644 --- a/lib/tdb/common/freelist.c +++ b/lib/tdb/common/freelist.c @@ -56,7 +56,7 @@ int tdb_rec_free_read(struct tdb_context *tdb, tdb_off_t off, struct tdb_record rec->magic, off)); return -1; } - if (tdb->methods->tdb_oob(tdb, rec->next+sizeof(*rec), 0) != 0) + if (tdb->methods->tdb_oob(tdb, rec->next, sizeof(*rec), 0) != 0) return -1; return 0; } diff --git a/lib/tdb/common/io.c b/lib/tdb/common/io.c index 78bbf2ec77..a2db3bf4bd 100644 --- a/lib/tdb/common/io.c +++ b/lib/tdb/common/io.c @@ -31,19 +31,29 @@ /* check for an out of bounds access - if it is out of bounds then see if the database has been expanded by someone else and expand if necessary - note that "len" is the minimum length needed for the db */ -static int tdb_oob(struct tdb_context *tdb, tdb_off_t len, int probe) +static int tdb_oob(struct tdb_context *tdb, tdb_off_t off, tdb_len_t len, + int probe) { struct stat st; - if (len <= tdb->map_size) + if (len + off < len) { + if (!probe) { + /* Ensure ecode is set for log fn. */ + tdb->ecode = TDB_ERR_IO; + TDB_LOG((tdb, TDB_DEBUG_FATAL,"tdb_oob off %d len %d wrap\n", + (int)off, (int)len)); + } + return -1; + } + + if (off + len <= tdb->map_size) return 0; if (tdb->flags & TDB_INTERNAL) { if (!probe) { /* Ensure ecode is set for log fn. */ tdb->ecode = TDB_ERR_IO; - TDB_LOG((tdb, TDB_DEBUG_FATAL,"tdb_oob len %d beyond internal malloc size %d\n", - (int)len, (int)tdb->map_size)); + TDB_LOG((tdb, TDB_DEBUG_FATAL,"tdb_oob len %u beyond internal malloc size %u\n", + (int)(off + len), (int)tdb->map_size)); } return -1; } @@ -53,16 +63,25 @@ static int tdb_oob(struct tdb_context *tdb, tdb_off_t len, int probe) return -1; } - if (st.st_size < (size_t)len) { + 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 %d beyond eof at %d\n", - (int)len, (int)st.st_size)); + 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. */ + tdb->ecode = TDB_ERR_IO; + TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_oob len %llu too large!\n", + (long long)st.st_size)); + return -1; + } + /* Unmap, update size, remap */ if (tdb_munmap(tdb) == -1) { tdb->ecode = TDB_ERR_IO; @@ -86,7 +105,7 @@ static int tdb_write(struct tdb_context *tdb, tdb_off_t off, return -1; } - if (tdb->methods->tdb_oob(tdb, off + len, 0) != 0) + if (tdb->methods->tdb_oob(tdb, off, len, 0) != 0) return -1; if (tdb->map_ptr) { @@ -134,7 +153,7 @@ void *tdb_convert(void *buf, uint32_t size) static int tdb_read(struct tdb_context *tdb, tdb_off_t off, void *buf, tdb_len_t len, int cv) { - if (tdb->methods->tdb_oob(tdb, off + len, 0) != 0) { + if (tdb->methods->tdb_oob(tdb, off, len, 0) != 0) { return -1; } @@ -307,7 +326,7 @@ int tdb_expand(struct tdb_context *tdb, tdb_off_t size) } /* must know about any previous expansions by another process */ - tdb->methods->tdb_oob(tdb, tdb->map_size + 1, 1); + tdb->methods->tdb_oob(tdb, tdb->map_size, 1, 1); /* limit size in order to avoid using up huge amounts of memory for * in memory tdbs if an oddball huge record creeps in */ @@ -434,7 +453,7 @@ int tdb_parse_data(struct tdb_context *tdb, TDB_DATA key, * Optimize by avoiding the malloc/memcpy/free, point the * parser directly at the mmap area. */ - if (tdb->methods->tdb_oob(tdb, offset+len, 0) != 0) { + if (tdb->methods->tdb_oob(tdb, offset, len, 0) != 0) { return -1; } data.dptr = offset + (unsigned char *)tdb->map_ptr; @@ -461,7 +480,7 @@ int tdb_rec_read(struct tdb_context *tdb, tdb_off_t offset, struct tdb_record *r TDB_LOG((tdb, TDB_DEBUG_FATAL,"tdb_rec_read bad magic 0x%x at offset=%d\n", rec->magic, offset)); return -1; } - return tdb->methods->tdb_oob(tdb, rec->next+sizeof(*rec), 0); + return tdb->methods->tdb_oob(tdb, rec->next, sizeof(*rec), 0); } int tdb_rec_write(struct tdb_context *tdb, tdb_off_t offset, struct tdb_record *rec) diff --git a/lib/tdb/common/open.c b/lib/tdb/common/open.c index ec45689ffc..2965ea77b5 100644 --- a/lib/tdb/common/open.c +++ b/lib/tdb/common/open.c @@ -385,7 +385,17 @@ _PUBLIC_ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int td goto fail; } + /* Beware truncation! */ tdb->map_size = st.st_size; + if (tdb->map_size != st.st_size) { + /* Ensure ecode is set for log fn. */ + tdb->ecode = TDB_ERR_IO; + TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_open_ex: " + "len %llu too large!\n", (long long)st.st_size)); + errno = EIO; + goto fail; + } + tdb->device = st.st_dev; tdb->inode = st.st_ino; tdb_mmap(tdb); diff --git a/lib/tdb/common/tdb_private.h b/lib/tdb/common/tdb_private.h index 140d4ecec5..3c6aabffc7 100644 --- a/lib/tdb/common/tdb_private.h +++ b/lib/tdb/common/tdb_private.h @@ -180,7 +180,7 @@ struct tdb_methods { int (*tdb_read)(struct tdb_context *, tdb_off_t , void *, tdb_len_t , int ); int (*tdb_write)(struct tdb_context *, tdb_off_t, const void *, tdb_len_t); void (*next_hash_chain)(struct tdb_context *, uint32_t *); - int (*tdb_oob)(struct tdb_context *, tdb_off_t , int ); + int (*tdb_oob)(struct tdb_context *, tdb_off_t , tdb_len_t, int ); int (*tdb_expand_file)(struct tdb_context *, tdb_off_t , tdb_off_t ); }; diff --git a/lib/tdb/common/transaction.c b/lib/tdb/common/transaction.c index e4573cb0a9..66ecbfdefb 100644 --- a/lib/tdb/common/transaction.c +++ b/lib/tdb/common/transaction.c @@ -382,9 +382,10 @@ static void transaction_next_hash_chain(struct tdb_context *tdb, uint32_t *chain /* out of bounds check during a transaction */ -static int transaction_oob(struct tdb_context *tdb, tdb_off_t len, int probe) +static int transaction_oob(struct tdb_context *tdb, tdb_off_t off, + tdb_len_t len, int probe) { - if (len <= tdb->map_size) { + if (off + len >= off && off + len <= tdb->map_size) { return 0; } tdb->ecode = TDB_ERR_IO; @@ -507,7 +508,7 @@ static int _tdb_transaction_start(struct tdb_context *tdb, /* make sure we know about any file expansions already done by anyone else */ - tdb->methods->tdb_oob(tdb, tdb->map_size + 1, 1); + tdb->methods->tdb_oob(tdb, tdb->map_size, 1, 1); tdb->transaction->old_map_size = tdb->map_size; /* finally hook the io methods, replacing them with @@ -741,7 +742,7 @@ static int tdb_recovery_allocate(struct tdb_context *tdb, } /* remap the file (if using mmap) */ - methods->tdb_oob(tdb, tdb->map_size + 1, 1); + methods->tdb_oob(tdb, tdb->map_size, 1, 1); /* we have to reset the old map size so that we don't try to expand the file again in the transaction commit, which would destroy the recovery area */ @@ -980,7 +981,7 @@ static int _tdb_transaction_prepare_commit(struct tdb_context *tdb) return -1; } tdb->map_size = tdb->transaction->old_map_size; - methods->tdb_oob(tdb, tdb->map_size + 1, 1); + methods->tdb_oob(tdb, tdb->map_size, 1, 1); } /* Keep the open lock until the actual commit */