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 <rusty@rustcorp.com.au>
Autobuild-User: Rusty Russell <rusty@rustcorp.com.au>
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 3387fbd098d7eb4d561cb9f223189a1935674536..313f55cbb05ed39632ddc99ff60a189f266be9ec 100644 (file)
--- 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 927078a7aa5cec47b7d67ee44410d8dd007936f6..6358f64a04ab8db19693b05e6f84f282c44c348e 100644 (file)
--- 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 78bbf2ec777a0517c404ee7c1ad66b0c7314df17..a2db3bf4bd81d45f40c42bc6c51c5f2fb31cc159 100644 (file)
--- 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 ec45689ffccd26146538a244e2251ba74c8d1804..2965ea77b5a26c4cd12c60a548b69a218012e54c 100644 (file)
--- 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 140d4ecec5695660e36f7cdcec447c7283296c25..3c6aabffc76a7c244df9a29d42f9003b54cac3dd 100644 (file)
--- 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 e4573cb0a96113cc0d9cb3dcb5eafdebf56f0e82..66ecbfdefb6ef827681eebaf2bb3230bf0b386b1 100644 (file)
--- 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 */