From: Rusty Russell Date: Mon, 13 Sep 2010 10:12:34 +0000 (+0930) Subject: tdb: put example hashes into header, so we notice incorrect hash_fn. X-Git-Url: http://git.ozlabs.org/?p=ccan;a=commitdiff_plain;h=44eea6ca52a2de5a817cf54d5d206969845dee3d tdb: put example hashes into header, so we notice incorrect hash_fn. This is Stefan Metzmacher '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. --- diff --git a/ccan/tdb/check.c b/ccan/tdb/check.c index 53fb2866..955f2554 100644 --- a/ccan/tdb/check.c +++ b/ccan/tdb/check.c @@ -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) diff --git a/ccan/tdb/open.c b/ccan/tdb/open.c index 437a5fc3..aa32a45e 100644 --- a/ccan/tdb/open.c +++ b/ccan/tdb/open.c @@ -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; } diff --git a/ccan/tdb/tdb_private.h b/ccan/tdb/tdb_private.h index 16f5d925..7f349797 100644 --- a/ccan/tdb/tdb_private.h +++ b/ccan/tdb/tdb_private.h @@ -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 diff --git a/ccan/tdb/test/jenkins-be-hash.tdb b/ccan/tdb/test/jenkins-be-hash.tdb index 45b5f09a..b6528404 100644 Binary files a/ccan/tdb/test/jenkins-be-hash.tdb and b/ccan/tdb/test/jenkins-be-hash.tdb differ diff --git a/ccan/tdb/test/jenkins-le-hash.tdb b/ccan/tdb/test/jenkins-le-hash.tdb index 45b5f09a..007e0a33 100644 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 index 00000000..1c49116c 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 index 00000000..0655072d Binary files /dev/null and b/ccan/tdb/test/old-nohash-le.tdb differ diff --git a/ccan/tdb/test/run-check.c b/ccan/tdb/test/run-check.c index 4a57eb6f..4b2dcc18 100644 --- a/ccan/tdb/test/run-check.c +++ b/ccan/tdb/test/run-check.c @@ -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); diff --git a/ccan/tdb/test/run-corrupt.c b/ccan/tdb/test/run-corrupt.c index 067ac861..bea7dbcd 100644 --- a/ccan/tdb/test/run-corrupt.c +++ b/ccan/tdb/test/run-corrupt.c @@ -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 index 00000000..8e5a76f8 --- /dev/null +++ b/ccan/tdb/test/run-oldhash.c @@ -0,0 +1,56 @@ +#define _XOPEN_SOURCE 500 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#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 index 00000000..1f742f4f --- /dev/null +++ b/ccan/tdb/test/run-rwlock-check.c @@ -0,0 +1,47 @@ +#define _XOPEN_SOURCE 500 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +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(); +} diff --git a/ccan/tdb/test/run-wronghash-fail.c b/ccan/tdb/test/run-wronghash-fail.c index 2804515a..1267f22f 100644 --- a/ccan/tdb/test/run-wronghash-fail.c +++ b/ccan/tdb/test/run-wronghash-fail.c @@ -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 index af82ea28..00000000 --- a/ccan/tdb/test/run-wronghash-old.c +++ /dev/null @@ -1,67 +0,0 @@ -#define _XOPEN_SOURCE 500 -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -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 index 00000000..45b5f09a 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 index 00000000..45b5f09a 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 index 1c49116c..00000000 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 index 0655072d..00000000 Binary files a/ccan/tdb/test/tdb-le.tdb and /dev/null differ