tdb2: be more careful on 4G files (tdb1).

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>
(Ported from tdb1 SAMBA commit b64494535dc62f4073fc6302847593ed6e6ec38b)

diff --git a/ccan/tdb2/tdb1_check.c b/ccan/tdb2/tdb1_check.c
index a8e54b2ee092f4cb7e1866818551e4e6177f1e5b..07ee07553a633e21cb8855180dc1779a706380d1 100644 (file)
--- a/ccan/tdb2/tdb1_check.c
+++ b/ccan/tdb2/tdb1_check.c
@@ -92,7 +92,7 @@ static bool tdb1_check_record(struct tdb_context *tdb,
                           off, rec->next);
                goto corrupt;
        }
-       if (tdb->tdb1.io->tdb1_oob(tdb, rec->next+sizeof(*rec), 0))
+       if (tdb->tdb1.io->tdb1_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 tdb1_check_record(struct tdb_context *tdb,
                goto corrupt;
        }
        /* OOB allows "right at the end" access, so this works for last rec. */
-       if (tdb->tdb1.io->tdb1_oob(tdb, off+sizeof(*rec)+rec->rec_len, 0))
+       if (tdb->tdb1.io->tdb1_oob(tdb, off, sizeof(*rec)+rec->rec_len, 0))
                goto corrupt;
 
        /* Check tailer. */
@@ -351,7 +351,7 @@ int tdb1_check(struct tdb_context *tdb,
        }
 
        /* Make sure we know true size of the underlying file. */
-       tdb->tdb1.io->tdb1_oob(tdb, tdb->file->map_size + 1, 1);
+       tdb->tdb1.io->tdb1_oob(tdb, tdb->file->map_size, 1, 1);
 
        /* Header must be OK: also gets us the recovery ptr, if any. */
        if (!tdb1_check_header(tdb, &recovery_start))

diff --git a/ccan/tdb2/tdb1_freelist.c b/ccan/tdb2/tdb1_freelist.c
index af0129372173d3b12f048f4960c66cdfb14d844b..ea368ec4aeafdd513921adc3aac48cb6f5e3fa0a 100644 (file)
--- a/ccan/tdb2/tdb1_freelist.c
+++ b/ccan/tdb2/tdb1_freelist.c
@@ -50,7 +50,7 @@ int tdb1_rec_free_read(struct tdb_context *tdb, tdb1_off_t off, struct tdb1_reco
                                        rec->magic, off);
                return -1;
        }
-       if (tdb->tdb1.io->tdb1_oob(tdb, rec->next+sizeof(*rec), 0) != 0)
+       if (tdb->tdb1.io->tdb1_oob(tdb, rec->next, sizeof(*rec), 0) != 0)
                return -1;
        return 0;
 }

diff --git a/ccan/tdb2/tdb1_io.c b/ccan/tdb2/tdb1_io.c
index f3d139d0434a97fc7c9dbb6a4deea6cb6d2cd43e..ceae5285a7a1bb0e4607cca4ec8d81ac45aa994a 100644 (file)
--- a/ccan/tdb2/tdb1_io.c
+++ b/ccan/tdb2/tdb1_io.c
@@ -36,16 +36,26 @@
    if necessary
    note that "len" is the minimum length needed for the db
 */
-static int tdb1_oob(struct tdb_context *tdb, tdb1_off_t len, int probe)
+static int tdb1_oob(struct tdb_context *tdb, tdb1_off_t off, tdb1_len_t len,
+                   int probe)
 {
        struct stat st;
-       if (len <= tdb->file->map_size)
+       if (len + off < len) {
+               if (!probe) {
+                       tdb->last_error = tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR,
+                                                    "tdb1_oob off %d len %d wrap\n",
+                                                    (int)off, (int)len);
+               }
+               return -1;
+       }
+
+       if (off + len <= tdb->file->map_size)
                return 0;
        if (tdb->flags & TDB_INTERNAL) {
                if (!probe) {
                        tdb->last_error = tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR,
-                                               "tdb1_oob len %d beyond internal malloc size %d",
-                                               (int)len, (int)tdb->file->map_size);
+                                                    "tdb1_oob len %d beyond internal malloc size %u",
+                                                    (int)(off + len), (int)tdb->file->map_size);
                }
                return -1;
        }
@@ -55,15 +65,23 @@ static int tdb1_oob(struct tdb_context *tdb, tdb1_off_t len, int probe)
                return -1;
        }
 
-       if (st.st_size < (size_t)len) {
+       if (st.st_size < (size_t)off + len) {
                if (!probe) {
                        tdb->last_error = tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR,
-                                               "tdb1_oob len %d beyond eof at %d",
-                                               (int)len, (int)st.st_size);
+                                                    "tdb1_oob len %u beyond eof at %u",
+                                                    (int)(off + len), (int)st.st_size);
                }
                return -1;
        }
 
+       /* Beware >4G files! */
+       if ((tdb1_off_t)st.st_size != st.st_size) {
+               tdb->last_error = tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR,
+                                            "tdb1_oob len %llu too large!\n",
+                                            (long long)st.st_size);
+               return -1;
+       }
+
        /* Unmap, update size, remap */
        if (tdb1_munmap(tdb) == -1) {
                tdb->last_error = TDB_ERR_IO;
@@ -87,7 +105,7 @@ static int tdb1_write(struct tdb_context *tdb, tdb1_off_t off,
                return -1;
        }
 
-       if (tdb->tdb1.io->tdb1_oob(tdb, off + len, 0) != 0)
+       if (tdb->tdb1.io->tdb1_oob(tdb, off, len, 0) != 0)
                return -1;
 
        if (tdb->file->map_ptr) {
@@ -136,7 +154,7 @@ void *tdb1_convert(void *buf, uint32_t size)
 static int tdb1_read(struct tdb_context *tdb, tdb1_off_t off, void *buf,
                    tdb1_len_t len, int cv)
 {
-       if (tdb->tdb1.io->tdb1_oob(tdb, off + len, 0) != 0) {
+       if (tdb->tdb1.io->tdb1_oob(tdb, off, len, 0) != 0) {
                return -1;
        }
 
@@ -326,7 +344,7 @@ int tdb1_expand(struct tdb_context *tdb, tdb1_off_t size)
        }
 
        /* must know about any previous expansions by another process */
-       tdb->tdb1.io->tdb1_oob(tdb, tdb->file->map_size + 1, 1);
+       tdb->tdb1.io->tdb1_oob(tdb, tdb->file->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 */
@@ -456,7 +474,7 @@ enum TDB_ERROR tdb1_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->tdb1.io->tdb1_oob(tdb, offset+len, 0) != 0) {
+               if (tdb->tdb1.io->tdb1_oob(tdb, offset, len, 0) != 0) {
                        return tdb->last_error;
                }
                data.dptr = offset + (unsigned char *)tdb->file->map_ptr;
@@ -483,7 +501,7 @@ int tdb1_rec_read(struct tdb_context *tdb, tdb1_off_t offset, struct tdb1_record
                                        rec->magic, offset);
                return -1;
        }
-       return tdb->tdb1.io->tdb1_oob(tdb, rec->next+sizeof(*rec), 0);
+       return tdb->tdb1.io->tdb1_oob(tdb, rec->next, sizeof(*rec), 0);
 }
 
 int tdb1_rec_write(struct tdb_context *tdb, tdb1_off_t offset, struct tdb1_record *rec)
@@ -511,6 +529,6 @@ void tdb1_io_init(struct tdb_context *tdb)
 enum TDB_ERROR tdb1_probe_length(struct tdb_context *tdb)
 {
        tdb->last_error = TDB_SUCCESS;
-       tdb->tdb1.io->tdb1_oob(tdb, tdb->file->map_size + 1, true);
+       tdb->tdb1.io->tdb1_oob(tdb, tdb->file->map_size, 1, true);
        return tdb->last_error;
 }

diff --git a/ccan/tdb2/tdb1_private.h b/ccan/tdb2/tdb1_private.h
index 68dc39f6f8e02d1fcce98feb9a2a55b4e45c4763..fe2681ceec582112dd9f584c3cbaf221fcc92e8c 100644 (file)
--- a/ccan/tdb2/tdb1_private.h
+++ b/ccan/tdb2/tdb1_private.h
@@ -112,7 +112,7 @@ struct tdb1_methods {
        int (*tdb1_read)(struct tdb_context *, tdb1_off_t , void *, tdb1_len_t , int );
        int (*tdb1_write)(struct tdb_context *, tdb1_off_t, const void *, tdb1_len_t);
        void (*next_hash_chain)(struct tdb_context *, uint32_t *);
-       int (*tdb1_oob)(struct tdb_context *, tdb1_off_t , int );
+       int (*tdb1_oob)(struct tdb_context *, tdb1_off_t, tdb1_len_t, int );
        int (*tdb1_expand_file)(struct tdb_context *, tdb1_off_t , tdb1_off_t );
 };
 

diff --git a/ccan/tdb2/tdb1_transaction.c b/ccan/tdb2/tdb1_transaction.c
index 411caef360d4ff5e1353b60263e7252d87d9ef1f..c4a2b10b20759685112524f6a101e69964d3ec9f 100644 (file)
--- a/ccan/tdb2/tdb1_transaction.c
+++ b/ccan/tdb2/tdb1_transaction.c
@@ -376,9 +376,9 @@ static void transaction1_next_hash_chain(struct tdb_context *tdb, uint32_t *chai
 /*
   out of bounds check during a transaction
 */
-static int transaction1_oob(struct tdb_context *tdb, tdb1_off_t len, int probe)
+static int transaction1_oob(struct tdb_context *tdb, tdb1_off_t off, tdb1_off_t len, int probe)
 {
-       if (len <= tdb->file->map_size) {
+       if (off + len >= off && off + len <= tdb->file->map_size) {
                return 0;
        }
        tdb->last_error = TDB_ERR_IO;
@@ -520,7 +520,7 @@ static int _tdb1_transaction_start(struct tdb_context *tdb)
 
        /* make sure we know about any file expansions already done by
           anyone else */
-       tdb->tdb1.io->tdb1_oob(tdb, tdb->file->map_size + 1, 1);
+       tdb->tdb1.io->tdb1_oob(tdb, tdb->file->map_size, 1, 1);
        tdb->tdb1.transaction->old_map_size = tdb->file->map_size;
 
        /* finally hook the io methods, replacing them with
@@ -761,7 +761,7 @@ static int tdb1_recovery_allocate(struct tdb_context *tdb,
        tdb->stats.transaction_expand_file++;
 
        /* remap the file (if using mmap) */
-       methods->tdb1_oob(tdb, tdb->file->map_size + 1, 1);
+       methods->tdb1_oob(tdb, tdb->file->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 */
@@ -1022,7 +1022,7 @@ static int _tdb1_transaction_prepare_commit(struct tdb_context *tdb)
                }
                tdb->stats.transaction_expand_file++;
                tdb->file->map_size = tdb->tdb1.transaction->old_map_size;
-               methods->tdb1_oob(tdb, tdb->file->map_size + 1, 1);
+               methods->tdb1_oob(tdb, tdb->file->map_size, 1, 1);
        }
 
        /* Keep the open lock until the actual commit */