tdb: put example hashes into header, so we notice incorrect hash_fn.

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.

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)

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;
 	}
 

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

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);

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 */

ccan/tdb/test/run-oldhash.c

@@ -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();
+}

ccan/tdb/test/run-wronghash-old.c → ccan/tdb/test/run-rwlock-check.c

@@ -14,54 +14,34 @@
 #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"))
+	if (strstr(fmt, "spinlocks"))
 		(*count)++;
 }
 
-/* The old code should barf on new-style TDBs created with a non-default hash.
- */
+/* 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(8);
+	plan_tests(4);
 
-	/* We should fail to open new-style non-default-hash tdbs of
-	 * either endian. */
+	/* We should fail to open rwlock-using tdbs of either endian. */
 	log_count = 0;
-	tdb = tdb_open_ex("test/jenkins-le-hash.tdb", 0, 0, O_RDWR, 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/jenkins-be-hash.tdb", 0, 0, O_RDWR, 0,
+	tdb = tdb_open_ex("test/rwlock-be.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();
 }

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();