[SCM] Samba Shared Repository - branch master updated

Rusty Russell rusty at samba.org
Mon Sep 13 06:27:20 MDT 2010


The branch, master has been updated
       via  d10b2c0 lib/tdb: change version to 1.2.4 after hash checking improvments
       via  786b726 tdb: put example hashes into header, so we notice incorrect hash_fn.
       via  f77708e tdb: fix tdb_check() on other-endian tdbs.
       via  82e5644 tdb: fix tdb_check() on read-only TDBs to actually work.
       via  9e0deff tdb: make check more robust against recovery failures.
      from  fe958c0 Revert "s4:samldb LDB module - simplify the message handling on add and modify operations"

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit d10b2c07be2cfdca09f07d3045ce891989d83a09
Author: Stefan Metzmacher <metze at samba.org>
Date:   Fri Sep 10 04:47:32 2010 +0200

    lib/tdb: change version to 1.2.4 after hash checking improvments
    
    lib/tdb: change version to 1.2.4 after hash checking improvments
    
    metze
    
    Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>

commit 786b7263000dedcb97e7369402e2e9dc967e36c4
Author: Rusty Russell <rusty at rustcorp.com.au>
Date:   Mon Sep 13 20:05:59 2010 +0930

    tdb: put example hashes into header, so we notice incorrect hash_fn.
    
    This is Stefan Metzmacher <metze at 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.
    
    Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>

commit f77708e96268d18abbfb038f4e78fe9e11a2856f
Author: Rusty Russell <rusty at rustcorp.com.au>
Date:   Mon Sep 13 19:59:18 2010 +0930

    tdb: fix tdb_check() on other-endian tdbs.
    
    We must not endian-convert the magic string, just the rest.
    
    Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>

commit 82e5644c9dbf5c2e4b0c4183372e0a79203d32a5
Author: Rusty Russell <rusty at rustcorp.com.au>
Date:   Mon Sep 13 19:58:23 2010 +0930

    tdb: fix tdb_check() on read-only TDBs to actually work.
    
    Commit bc1c82ea137 "Fix tdb_check() to work with read-only tdb databases."
    claimed to do this, but tdb_lockall_read() fails on read-only databases.
    
    Also make sure we can still do tdb_check() inside a transaction (weird,
    but we previously allowed it so don't break the API).
    
    Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>

commit 9e0deff904877068d19b41e965732f145c2554b9
Author: Rusty Russell <rusty at rustcorp.com.au>
Date:   Mon Sep 13 19:55:26 2010 +0930

    tdb: make check more robust against recovery failures.
    
    We can end up with dead areas when we die during transaction commit;
    tdb_check() fails on such a (valid) database.
    
    This is particularly noticable now we no longer truncate on recovery;
    if the recovery area was at the end of the file we used to remove it
    that way.
    
    Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>

-----------------------------------------------------------------------

Summary of changes:
 lib/tdb/common/check.c       |   71 +++++++++++++++++++++++++++++++++++------
 lib/tdb/common/open.c        |   55 ++++++++++++++++++++++++++++++++-
 lib/tdb/common/tdb_private.h |    6 +++-
 lib/tdb/configure.ac         |    2 +-
 lib/tdb/wscript              |    2 +-
 5 files changed, 121 insertions(+), 15 deletions(-)


Changeset truncated at 500 lines:

diff --git a/lib/tdb/common/check.c b/lib/tdb/common/check.c
index 2c64043..b1b98d4 100644
--- a/lib/tdb/common/check.c
+++ b/lib/tdb/common/check.c
@@ -28,8 +28,9 @@
 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), DOCONV()) == -1)
+	if (tdb->methods->tdb_read(tdb, 0, &hdr, sizeof(hdr), 0) == -1)
 		return false;
 	if (strcmp(hdr.magic_food, TDB_MAGIC_FOOD) != 0)
 		goto corrupt;
@@ -41,6 +42,11 @@ static bool tdb_check_header(struct tdb_context *tdb, tdb_off_t *recovery)
 	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)
 		goto corrupt;
 
@@ -301,6 +307,21 @@ static bool tdb_check_free_record(struct tdb_context *tdb,
 	return true;
 }
 
+/* Slow, but should be very rare. */
+static size_t dead_space(struct tdb_context *tdb, tdb_off_t off)
+{
+	size_t len;
+
+	for (len = 0; off + len < tdb->map_size; len++) {
+		char c;
+		if (tdb->methods->tdb_read(tdb, off, &c, 1, 0))
+			return 0;
+		if (c != 0 && c != 0x42)
+			break;
+	}
+	return len;
+}
+
 int tdb_check(struct tdb_context *tdb,
 	      int (*check)(TDB_DATA key, TDB_DATA data, void *private_data),
 	      void *private_data)
@@ -310,9 +331,18 @@ int tdb_check(struct tdb_context *tdb,
 	tdb_off_t off, recovery_start;
 	struct tdb_record rec;
 	bool found_recovery = false;
-
-	if (tdb_lockall_read(tdb) == -1)
-		return -1;
+	tdb_len_t dead;
+	bool locked;
+
+	/* Read-only databases use no locking at all: it's best-effort.
+	 * We may have a write lock already, so skip that case too. */
+	if (tdb->read_only || tdb->allrecord_lock.count != 0) {
+		locked = false;
+	} else {
+		if (tdb_lockall_read(tdb) == -1)
+			return -1;
+		locked = true;
+	}
 
 	/* Make sure we know true size of the underlying file. */
 	tdb->methods->tdb_oob(tdb, tdb->map_size + 1, 1);
@@ -369,8 +399,23 @@ int tdb_check(struct tdb_context *tdb,
 			if (!tdb_check_free_record(tdb, off, &rec, hashes))
 				goto free;
 			break;
-		case TDB_RECOVERY_MAGIC:
+		/* If we crash after ftruncate, we can get zeroes or fill. */
 		case TDB_RECOVERY_INVALID_MAGIC:
+		case 0x42424242:
+			if (recovery_start == off) {
+				found_recovery = true;
+				break;
+			}
+			dead = dead_space(tdb, off);
+			if (dead < sizeof(rec))
+				goto corrupt;
+
+			TDB_LOG((tdb, TDB_DEBUG_ERROR,
+				 "Dead space at %d-%d (of %u)\n",
+				 off, off + dead, tdb->map_size));
+			rec.rec_len = dead - sizeof(rec);
+			break;
+		case TDB_RECOVERY_MAGIC:
 			if (recovery_start != off) {
 				TDB_LOG((tdb, TDB_DEBUG_ERROR,
 					 "Unexpected recovery record at offset %d\n",
@@ -379,7 +424,8 @@ int tdb_check(struct tdb_context *tdb,
 			}
 			found_recovery = true;
 			break;
-		default:
+		default: ;
+		corrupt:
 			tdb->ecode = TDB_ERR_CORRUPT;
 			TDB_LOG((tdb, TDB_DEBUG_ERROR,
 				 "Bad magic 0x%x at offset %d\n",
@@ -405,19 +451,22 @@ int tdb_check(struct tdb_context *tdb,
 	/* We must have found recovery area if there was one. */
 	if (recovery_start != 0 && !found_recovery) {
 		TDB_LOG((tdb, TDB_DEBUG_ERROR,
-			 "Expected %s recovery area, got %s\n",
-			 recovery_start ? "a" : "no",
-			 found_recovery ? "one" : "none"));
+			 "Expected a recovery area at %u\n",
+			 recovery_start));
 		goto free;
 	}
 
 	free(hashes);
-	tdb_unlockall_read(tdb);
+	if (locked) {
+		tdb_unlockall_read(tdb);
+	}
 	return 0;
 
 free:
 	free(hashes);
 unlock:
-	tdb_unlockall_read(tdb);
+	if (locked) {
+		tdb_unlockall_read(tdb);
+	}
 	return -1;
 }
diff --git a/lib/tdb/common/open.c b/lib/tdb/common/open.c
index 7687ff6..401fa74 100644
--- a/lib/tdb/common/open.c
+++ b/lib/tdb/common/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)
@@ -62,6 +81,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;
+
+	tdb_header_hash(tdb, &newdb->magic1_hash, &newdb->magic2_hash);
+
 	if (tdb->flags & TDB_INTERNAL) {
 		tdb->map_size = size;
 		tdb->map_ptr = (char *)newdb;
@@ -140,6 +162,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 */
@@ -161,7 +186,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();
@@ -279,6 +311,27 @@ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
 		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;
+	}
+
 	/* Is it already in the open list?  If so, fail. */
 	if (tdb_already_open(st.st_dev, st.st_ino)) {
 		TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_open_ex: "
diff --git a/lib/tdb/common/tdb_private.h b/lib/tdb/common/tdb_private.h
index 9d0f3bc..eccd688 100644
--- a/lib/tdb/common/tdb_private.h
+++ b/lib/tdb/common/tdb_private.h
@@ -147,7 +147,9 @@ struct tdb_header {
 	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 {
@@ -268,3 +270,5 @@ int tdb_rec_free_read(struct tdb_context *tdb, tdb_off_t off,
 		      struct tdb_record *rec);
 bool tdb_write_all(int fd, const void *buf, size_t count);
 int tdb_transaction_recover(struct tdb_context *tdb);
+void tdb_header_hash(struct tdb_context *tdb,
+		     uint32_t *magic1_hash, uint32_t *magic2_hash);
diff --git a/lib/tdb/configure.ac b/lib/tdb/configure.ac
index 2843d98..8390865 100644
--- a/lib/tdb/configure.ac
+++ b/lib/tdb/configure.ac
@@ -2,7 +2,7 @@ AC_PREREQ(2.50)
 AC_DEFUN([SMB_MODULE_DEFAULT], [echo -n ""])
 AC_DEFUN([SMB_LIBRARY_ENABLE], [echo -n ""])
 AC_DEFUN([SMB_ENABLE], [echo -n ""])
-AC_INIT(tdb, 1.2.3)
+AC_INIT(tdb, 1.2.4)
 AC_CONFIG_SRCDIR([common/tdb.c])
 AC_CONFIG_HEADER(include/config.h)
 AC_LIBREPLACE_ALL_CHECKS
diff --git a/lib/tdb/wscript b/lib/tdb/wscript
index 3dfb5ab..d661e41 100644
--- a/lib/tdb/wscript
+++ b/lib/tdb/wscript
@@ -1,7 +1,7 @@
 #!/usr/bin/env python
 
 APPNAME = 'tdb'
-VERSION = '1.2.3'
+VERSION = '1.2.4'
 
 blddir = 'bin'
 


-- 
Samba Shared Repository


More information about the samba-cvs mailing list