tdb: put example hashes into header, so we notice incorrect hash_fn.
authorRusty Russell <rusty@rustcorp.com.au>
Mon, 13 Sep 2010 10:12:34 +0000 (19:42 +0930)
committerRusty Russell <rusty@rustcorp.com.au>
Mon, 13 Sep 2010 10:12:34 +0000 (19:42 +0930)
This is Stefan Metzmacher <metze@samba.org>'s patch with minor
changes:
1) Use the TDB_MAGIC constant so both hashes aren't of strings.
2) Check the hash in tdb_check (paranoia, really).
3) Additional check in the (unlikely!) case where both examples hash to 0.
4) Cosmetic changes to var names and complaint message.

17 files changed:
ccan/tdb/check.c
ccan/tdb/open.c
ccan/tdb/tdb_private.h
ccan/tdb/test/jenkins-be-hash.tdb
ccan/tdb/test/jenkins-le-hash.tdb
ccan/tdb/test/old-nohash-be.tdb [new file with mode: 0644]
ccan/tdb/test/old-nohash-le.tdb [new file with mode: 0644]
ccan/tdb/test/run-check.c
ccan/tdb/test/run-corrupt.c
ccan/tdb/test/run-oldhash.c [new file with mode: 0644]
ccan/tdb/test/run-rwlock-check.c [new file with mode: 0644]
ccan/tdb/test/run-wronghash-fail.c
ccan/tdb/test/run-wronghash-old.c [deleted file]
ccan/tdb/test/rwlock-be.tdb [new file with mode: 0644]
ccan/tdb/test/rwlock-le.tdb [new file with mode: 0644]
ccan/tdb/test/tdb-be.tdb [deleted file]
ccan/tdb/test/tdb-le.tdb [deleted file]

index 53fb286613a3e829e8f2aa159109124c3e182503..955f255484a720a08a07a9e33a1fde6b0036b49c 100644 (file)
@@ -29,6 +29,7 @@
 static bool tdb_check_header(struct tdb_context *tdb, tdb_off_t *recovery)
 {
        struct tdb_header hdr;
+       uint32_t h1, h2;
 
        if (tdb->methods->tdb_read(tdb, 0, &hdr, sizeof(hdr), 0) == -1)
                return false;
@@ -39,7 +40,12 @@ static bool tdb_check_header(struct tdb_context *tdb, tdb_off_t *recovery)
        if (hdr.version != TDB_VERSION)
                goto corrupt;
 
-       if (hdr.hashcheck != hashcheck(tdb))
+       if (hdr.rwlocks != 0)
+               goto corrupt;
+
+       tdb_header_hash(tdb, &h1, &h2);
+       if (hdr.magic1_hash && hdr.magic2_hash &&
+           (hdr.magic1_hash != h1 || hdr.magic2_hash != h2))
                goto corrupt;
 
        if (hdr.hash_size == 0)
index 437a5fc38d1706e9b5f3e846876f8aa4e8d9da75..aa32a45eda3957aa04e8c9444a9ba2bd749b1207 100644 (file)
@@ -44,6 +44,25 @@ static unsigned int default_tdb_hash(TDB_DATA *key)
        return (1103515243 * value + 12345);  
 }
 
+/* We use two hashes to double-check they're using the right hash function. */
+void tdb_header_hash(struct tdb_context *tdb,
+                    uint32_t *magic1_hash, uint32_t *magic2_hash)
+{
+       TDB_DATA hash_key;
+       uint32_t tdb_magic = TDB_MAGIC;
+
+       hash_key.dptr = (unsigned char *)TDB_MAGIC_FOOD;
+       hash_key.dsize = sizeof(TDB_MAGIC_FOOD);
+       *magic1_hash = tdb->hash_fn(&hash_key);
+
+       hash_key.dptr = CONVERT(tdb_magic);
+       hash_key.dsize = sizeof(tdb_magic);
+       *magic2_hash = tdb->hash_fn(&hash_key);
+
+       /* Make sure at least one hash is non-zero! */
+       if (*magic1_hash == 0 && *magic2_hash == 0)
+               *magic1_hash = 1;
+}
 
 /* initialise a new database with a specified hash size */
 static int tdb_new_database(struct tdb_context *tdb, int hash_size)
@@ -63,7 +82,9 @@ static int tdb_new_database(struct tdb_context *tdb, int hash_size)
        /* Fill in the header */
        newdb->version = TDB_VERSION;
        newdb->hash_size = hash_size;
-       newdb->hashcheck = hashcheck(tdb);
+
+       tdb_header_hash(tdb, &newdb->magic1_hash, &newdb->magic2_hash);
+
        if (tdb->flags & TDB_INTERNAL) {
                tdb->map_size = size;
                tdb->map_ptr = (char *)newdb;
@@ -144,18 +165,6 @@ static void null_log_fn(struct tdb_context *tdb, enum tdb_debug_level level, con
 {
 }
 
-uint32_t hashcheck(struct tdb_context *tdb)
-{
-       uint32_t vals[] = { TDB_VERSION, TDB_MAGIC };
-       TDB_DATA hashkey = { (unsigned char *)vals, sizeof(vals) };
-
-       /* If we're using the default hash, let old code still open the db. */
-       if (tdb->hash_fn == default_tdb_hash)
-               return 0;
-
-       /* Only let new hash-aware TDB code open it (must not be zero!) */
-       return (tdb->hash_fn(&hashkey) | 1);
-}
 
 struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
                                int open_flags, mode_t mode,
@@ -168,6 +177,9 @@ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
        unsigned char *vp;
        uint32_t vertest;
        unsigned v;
+       uint32_t magic1_hash;
+       uint32_t magic2_hash;
+       const char *hash_alg;
 
        if (!(tdb = (struct tdb_context *)calloc(1, sizeof *tdb))) {
                /* Can't log this */
@@ -189,7 +201,14 @@ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
                tdb->log.log_fn = null_log_fn;
                tdb->log.log_private = NULL;
        }
-       tdb->hash_fn = hash_fn ? hash_fn : default_tdb_hash;
+
+       if (hash_fn) {
+               tdb->hash_fn = hash_fn;
+               hash_alg = "user defined";
+       } else {
+               tdb->hash_fn = default_tdb_hash;
+               hash_alg = "default";
+       }
 
        /* cache the page size */
        tdb->page_size = getpagesize();
@@ -301,8 +320,29 @@ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
        if (fstat(tdb->fd, &st) == -1)
                goto fail;
 
-       if (tdb->header.hashcheck != hashcheck(tdb)) {
-               TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_open_ex: wrong hash?\n"));
+       if (tdb->header.rwlocks != 0) {
+               TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_open_ex: spinlocks no longer supported\n"));
+               goto fail;
+       }
+
+       tdb_header_hash(tdb, &magic1_hash, &magic2_hash);
+
+       if ((tdb->header.magic1_hash == 0) && (tdb->header.magic2_hash == 0)) {
+               /* older TDB without magic hash references */
+       } else if ((tdb->header.magic1_hash != magic1_hash) ||
+                  (tdb->header.magic2_hash != magic2_hash)) {
+               TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_open_ex: "
+                        "%s was not created with the %s hash function we are using\n"
+                        "magic1_hash[0x%08X %s 0x%08X] "
+                        "magic2_hash[0x%08X %s 0x%08X]\n",
+                        name, hash_alg,
+                        tdb->header.magic1_hash,
+                        (tdb->header.magic1_hash == magic1_hash) ? "==" : "!=",
+                        magic1_hash,
+                        tdb->header.magic2_hash,
+                        (tdb->header.magic2_hash == magic2_hash) ? "==" : "!=",
+                        magic2_hash));
+               errno = EINVAL;
                goto fail;
        }
 
index 16f5d925c0954e2105ed63eeb969a45a374ad9cb..7f3497975e5cb7f1d6a2aaa30a32d1b32ebfafe7 100644 (file)
@@ -176,10 +176,12 @@ struct tdb_header {
        char magic_food[32]; /* for /etc/magic */
        uint32_t version; /* version of the code */
        uint32_t hash_size; /* number of hash entries */
-       tdb_off_t hashcheck; /* 0 for default hash. */
+       tdb_off_t rwlocks; /* obsolete - kept to detect old formats */
        tdb_off_t recovery_start; /* offset of transaction recovery region */
        tdb_off_t sequence_number; /* used when TDB_SEQNUM is set */
-       tdb_off_t reserved[29];
+       uint32_t magic1_hash; /* hash of TDB_MAGIC_FOOD. */
+       uint32_t magic2_hash; /* hash of TDB_MAGIC. */
+       tdb_off_t reserved[27];
 };
 
 struct tdb_lock_type {
@@ -248,7 +250,6 @@ struct tdb_context {
 /*
   internal prototypes
 */
-uint32_t hashcheck(struct tdb_context *tdb);
 int tdb_munmap(struct tdb_context *tdb);
 void tdb_mmap(struct tdb_context *tdb);
 int tdb_lock(struct tdb_context *tdb, int list, int ltype);
@@ -299,6 +300,7 @@ void tdb_io_init(struct tdb_context *tdb);
 int tdb_expand(struct tdb_context *tdb, tdb_off_t size);
 int tdb_rec_free_read(struct tdb_context *tdb, tdb_off_t off,
                      struct tdb_record *rec);
-
+void tdb_header_hash(struct tdb_context *tdb,
+                    uint32_t *magic1_hash, uint32_t *magic2_hash);
 
 #endif
index 45b5f09a1b619b4f79201057dd811781d4151d33..b652840414857969987a986c848ede159804166e 100644 (file)
Binary files a/ccan/tdb/test/jenkins-be-hash.tdb and b/ccan/tdb/test/jenkins-be-hash.tdb differ
index 45b5f09a1b619b4f79201057dd811781d4151d33..007e0a336843520fc4077e3ce29b1fff8ca51b1c 100644 (file)
Binary files a/ccan/tdb/test/jenkins-le-hash.tdb and b/ccan/tdb/test/jenkins-le-hash.tdb differ
diff --git a/ccan/tdb/test/old-nohash-be.tdb b/ccan/tdb/test/old-nohash-be.tdb
new file mode 100644 (file)
index 0000000..1c49116
Binary files /dev/null and b/ccan/tdb/test/old-nohash-be.tdb differ
diff --git a/ccan/tdb/test/old-nohash-le.tdb b/ccan/tdb/test/old-nohash-le.tdb
new file mode 100644 (file)
index 0000000..0655072
Binary files /dev/null and b/ccan/tdb/test/old-nohash-le.tdb differ
index 4a57eb6f88d109819e2d54b096d40b65cffaeb82..4b2dcc1884df09252a32e5122a8dd91070896127 100644 (file)
@@ -49,13 +49,13 @@ int main(int argc, char *argv[])
        tdb_close(tdb);
 
        /* Big and little endian should work! */
-       tdb = tdb_open_ex("test/tdb-le.tdb", 1024, 0, O_RDWR, 0,
+       tdb = tdb_open_ex("test/old-nohash-le.tdb", 1024, 0, O_RDWR, 0,
                          &taplogctx, NULL);
        ok1(tdb);
        ok1(tdb_check(tdb, NULL, NULL) == 0);
        tdb_close(tdb);
 
-       tdb = tdb_open_ex("test/tdb-be.tdb", 1024, 0, O_RDWR, 0,
+       tdb = tdb_open_ex("test/old-nohash-be.tdb", 1024, 0, O_RDWR, 0,
                          &taplogctx, NULL);
        ok1(tdb);
        ok1(tdb_check(tdb, NULL, NULL) == 0);
index 067ac861e4913acef1964da692b88fe625d08159..bea7dbcdf490218e4cd8ab0a4983246f6fb8be13 100644 (file)
@@ -73,7 +73,8 @@ static void check_test(struct tdb_context *tdb)
        /* This is how many bytes we expect to be verifiable. */
        /* From the file header. */
        verifiable = strlen(TDB_MAGIC_FOOD) + 1
-               + 2 * sizeof(uint32_t) + 2 * sizeof(tdb_off_t);
+               + 2 * sizeof(uint32_t) + 2 * sizeof(tdb_off_t)
+               + 2 * sizeof(uint32_t);
        /* From the free list chain and hash chains. */
        verifiable += 3 * sizeof(tdb_off_t);
        /* From the record headers & tailer */
diff --git a/ccan/tdb/test/run-oldhash.c b/ccan/tdb/test/run-oldhash.c
new file mode 100644 (file)
index 0000000..8e5a76f
--- /dev/null
@@ -0,0 +1,56 @@
+#define _XOPEN_SOURCE 500
+#include <ccan/tdb/tdb.h>
+#include <ccan/tdb/io.c>
+#include <ccan/tdb/tdb.c>
+#include <ccan/tdb/lock.c>
+#include <ccan/tdb/freelist.c>
+#include <ccan/tdb/traverse.c>
+#include <ccan/tdb/transaction.c>
+#include <ccan/tdb/error.c>
+#include <ccan/tdb/open.c>
+#include <ccan/tdb/check.c>
+#include <ccan/hash/hash.h>
+#include <ccan/tap/tap.h>
+#include <stdlib.h>
+#include <err.h>
+#include "logging.h"
+
+static unsigned int jenkins_hash(TDB_DATA *key)
+{
+       return hash_stable(key->dptr, key->dsize, 0);
+}
+
+int main(int argc, char *argv[])
+{
+       struct tdb_context *tdb;
+
+       plan_tests(8);
+
+       /* Old format (with zeroes in the hash magic fields) should
+        * open with any hash (since we don't know what hash they used). */
+       tdb = tdb_open_ex("test/old-nohash-le.tdb", 0, 0, O_RDWR, 0,
+                         &taplogctx, NULL);
+       ok1(tdb);
+       ok1(tdb_check(tdb, NULL, NULL) == 0);
+       tdb_close(tdb);
+
+       tdb = tdb_open_ex("test/old-nohash-be.tdb", 0, 0, O_RDWR, 0,
+                         &taplogctx, NULL);
+       ok1(tdb);
+       ok1(tdb_check(tdb, NULL, NULL) == 0);
+       tdb_close(tdb);
+
+       tdb = tdb_open_ex("test/old-nohash-le.tdb", 0, 0, O_RDWR, 0,
+                         &taplogctx, jenkins_hash);
+       ok1(tdb);
+       ok1(tdb_check(tdb, NULL, NULL) == 0);
+       tdb_close(tdb);
+
+       tdb = tdb_open_ex("test/old-nohash-be.tdb", 0, 0, O_RDWR, 0,
+                         &taplogctx, jenkins_hash);
+       ok1(tdb);
+       ok1(tdb_check(tdb, NULL, NULL) == 0);
+       tdb_close(tdb);
+
+       return exit_status();
+}
diff --git a/ccan/tdb/test/run-rwlock-check.c b/ccan/tdb/test/run-rwlock-check.c
new file mode 100644 (file)
index 0000000..1f742f4
--- /dev/null
@@ -0,0 +1,47 @@
+#define _XOPEN_SOURCE 500
+#include <ccan/tdb/tdb.h>
+#include <ccan/tdb/io.c>
+#include <ccan/tdb/tdb.c>
+#include <ccan/tdb/lock.c>
+#include <ccan/tdb/freelist.c>
+#include <ccan/tdb/traverse.c>
+#include <ccan/tdb/transaction.c>
+#include <ccan/tdb/error.c>
+#include <ccan/tdb/open.c>
+#include <ccan/tdb/check.c>
+#include <ccan/hash/hash.h>
+#include <ccan/tap/tap.h>
+#include <stdlib.h>
+#include <err.h>
+
+static void log_fn(struct tdb_context *tdb, enum tdb_debug_level level, const char *fmt, ...)
+{
+       unsigned int *count = tdb_get_logging_private(tdb);
+       if (strstr(fmt, "spinlocks"))
+               (*count)++;
+}
+
+/* The code should barf on TDBs created with rwlocks. */
+int main(int argc, char *argv[])
+{
+       struct tdb_context *tdb;
+       unsigned int log_count;
+       struct tdb_logging_context log_ctx = { log_fn, &log_count };
+
+       plan_tests(4);
+
+       /* We should fail to open rwlock-using tdbs of either endian. */
+       log_count = 0;
+       tdb = tdb_open_ex("test/rwlock-le.tdb", 0, 0, O_RDWR, 0,
+                         &log_ctx, NULL);
+       ok1(!tdb);
+       ok1(log_count == 1);
+
+       log_count = 0;
+       tdb = tdb_open_ex("test/rwlock-be.tdb", 0, 0, O_RDWR, 0,
+                         &log_ctx, NULL);
+       ok1(!tdb);
+       ok1(log_count == 1);
+
+       return exit_status();
+}
index 2804515a363b19b4c82752560cc0b81f100710d1..1267f22f3f621ec8fe7567f6708e2ea356d7f2e3 100644 (file)
@@ -22,9 +22,7 @@ static unsigned int jenkins_hash(TDB_DATA *key)
 static void log_fn(struct tdb_context *tdb, enum tdb_debug_level level, const char *fmt, ...)
 {
        unsigned int *count = tdb_get_logging_private(tdb);
-       /* Old code used to complain about spinlocks when we put something
-          here. */
-       if (strstr(fmt, "wrong hash") || strstr(fmt, "spinlock"))
+       if (strstr(fmt, "hash"))
                (*count)++;
 }
 
@@ -34,7 +32,7 @@ int main(int argc, char *argv[])
        unsigned int log_count;
        struct tdb_logging_context log_ctx = { log_fn, &log_count };
 
-       plan_tests(16);
+       plan_tests(18);
 
        /* Create with default hash. */
        log_count = 0;
@@ -84,6 +82,7 @@ int main(int argc, char *argv[])
                          0, &log_ctx, jenkins_hash);
        ok1(tdb);
        ok1(log_count == 0);
+       ok1(tdb_check(tdb, NULL, NULL) == 0);
        tdb_close(tdb);
 
        log_count = 0;
@@ -91,6 +90,7 @@ int main(int argc, char *argv[])
                          0, &log_ctx, jenkins_hash);
        ok1(tdb);
        ok1(log_count == 0);
+       ok1(tdb_check(tdb, NULL, NULL) == 0);
        tdb_close(tdb);
 
        return exit_status();
diff --git a/ccan/tdb/test/run-wronghash-old.c b/ccan/tdb/test/run-wronghash-old.c
deleted file mode 100644 (file)
index af82ea2..0000000
+++ /dev/null
@@ -1,67 +0,0 @@
-#define _XOPEN_SOURCE 500
-#include <ccan/tdb/tdb.h>
-#include <ccan/tdb/io.c>
-#include <ccan/tdb/tdb.c>
-#include <ccan/tdb/lock.c>
-#include <ccan/tdb/freelist.c>
-#include <ccan/tdb/traverse.c>
-#include <ccan/tdb/transaction.c>
-#include <ccan/tdb/error.c>
-#include <ccan/tdb/open.c>
-#include <ccan/tdb/check.c>
-#include <ccan/hash/hash.h>
-#include <ccan/tap/tap.h>
-#include <stdlib.h>
-#include <err.h>
-
-static unsigned int non_jenkins_hash(TDB_DATA *key)
-{
-       return ~hash_stable(key->dptr, key->dsize, 0);
-}
-
-static void log_fn(struct tdb_context *tdb, enum tdb_debug_level level, const char *fmt, ...)
-{
-       unsigned int *count = tdb_get_logging_private(tdb);
-       if (strstr(fmt, "wrong hash"))
-               (*count)++;
-}
-
-/* The old code should barf on new-style TDBs created with a non-default hash.
- */
-int main(int argc, char *argv[])
-{
-       struct tdb_context *tdb;
-       unsigned int log_count;
-       struct tdb_logging_context log_ctx = { log_fn, &log_count };
-
-       plan_tests(8);
-
-       /* We should fail to open new-style non-default-hash tdbs of
-        * either endian. */
-       log_count = 0;
-       tdb = tdb_open_ex("test/jenkins-le-hash.tdb", 0, 0, O_RDWR, 0,
-                         &log_ctx, NULL);
-       ok1(!tdb);
-       ok1(log_count == 1);
-
-       log_count = 0;
-       tdb = tdb_open_ex("test/jenkins-be-hash.tdb", 0, 0, O_RDWR, 0,
-                         &log_ctx, NULL);
-       ok1(!tdb);
-       ok1(log_count == 1);
-
-       /* And of course, if we use the wrong hash it will still fail. */
-       log_count = 0;
-       tdb = tdb_open_ex("test/jenkins-le-hash.tdb", 0, 0, O_RDWR, 0,
-                         &log_ctx, non_jenkins_hash);
-       ok1(!tdb);
-       ok1(log_count == 1);
-
-       log_count = 0;
-       tdb = tdb_open_ex("test/jenkins-be-hash.tdb", 0, 0, O_RDWR, 0,
-                         &log_ctx, non_jenkins_hash);
-       ok1(!tdb);
-       ok1(log_count == 1);
-
-       return exit_status();
-}
diff --git a/ccan/tdb/test/rwlock-be.tdb b/ccan/tdb/test/rwlock-be.tdb
new file mode 100644 (file)
index 0000000..45b5f09
Binary files /dev/null and b/ccan/tdb/test/rwlock-be.tdb differ
diff --git a/ccan/tdb/test/rwlock-le.tdb b/ccan/tdb/test/rwlock-le.tdb
new file mode 100644 (file)
index 0000000..45b5f09
Binary files /dev/null and b/ccan/tdb/test/rwlock-le.tdb differ
diff --git a/ccan/tdb/test/tdb-be.tdb b/ccan/tdb/test/tdb-be.tdb
deleted file mode 100644 (file)
index 1c49116..0000000
Binary files a/ccan/tdb/test/tdb-be.tdb and /dev/null differ
diff --git a/ccan/tdb/test/tdb-le.tdb b/ccan/tdb/test/tdb-le.tdb
deleted file mode 100644 (file)
index 0655072..0000000
Binary files a/ccan/tdb/test/tdb-le.tdb and /dev/null differ