The branch, v3-6-test has been updated via 6356255 lib/tdb: change version to 1.2.4 after hash checking improvments via 08eb42d tdb: put example hashes into header, so we notice incorrect hash_fn. via 72c6697 tdb: fix tdb_check() on other-endian tdbs. via 2c736fd tdb: fix tdb_check() on read-only TDBs to actually work. via fefe740 tdb: make check more robust against recovery failures. from 3670a24 s3: Fix a typo (authentictaion->authentication)
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-6-test - Log ----------------------------------------------------------------- commit 63562559f776c547f18820cf6b3abbb2fd831972 Author: Stefan Metzmacher <me...@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 <ru...@rustcorp.com.au> (cherry picked from commit d10b2c07be2cfdca09f07d3045ce891989d83a09) commit 08eb42d980e5e81fa4a0247c86d00baae37bae0a Author: Rusty Russell <ru...@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 <me...@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 <ru...@rustcorp.com.au> (cherry picked from commit 786b7263000dedcb97e7369402e2e9dc967e36c4) commit 72c6697de54e1ff2d938a3ba4c406718b5c734e4 Author: Rusty Russell <ru...@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 <ru...@rustcorp.com.au> (cherry picked from commit f77708e96268d18abbfb038f4e78fe9e11a2856f) commit 2c736fd0195b78698e5f30edb99a7e0a7c0fe7d0 Author: Rusty Russell <ru...@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 <ru...@rustcorp.com.au> (cherry picked from commit 82e5644c9dbf5c2e4b0c4183372e0a79203d32a5) commit fefe740a2c63e9c44613db58b4d8843dee30fd4c Author: Rusty Russell <ru...@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 <ru...@rustcorp.com.au> (cherry picked from commit 9e0deff904877068d19b41e965732f145c2554b9) ----------------------------------------------------------------------- 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