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 <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>
commit 786b7263000dedcb97e7369402e2e9dc967e36c4
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>
commit f77708e96268d18abbfb038f4e78fe9e11a2856f
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>
commit 82e5644c9dbf5c2e4b0c4183372e0a79203d32a5
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>
commit 9e0deff904877068d19b41e965732f145c2554b9
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>
-----------------------------------------------------------------------
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
