[PATCH 1/2] tdb: don't corrupt database if we go overlength due to transaction expand.
Rusty Russell
rusty at rustcorp.com.au
Tue May 28 01:23:55 MDT 2013
See http://permalink.gmane.org/gmane.network.samba.internals/42906 for
an example of what happens.
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
lib/tdb/common/io.c | 4 ++
lib/tdb/common/transaction.c | 5 ++
lib/tdb/test/run-overlength.c | 146 ++++++++++++++++++++++++++++++++++++++++++
lib/tdb/wscript | 4 +-
4 files changed, 158 insertions(+), 1 deletion(-)
create mode 100644 lib/tdb/test/run-overlength.c
diff --git a/lib/tdb/common/io.c b/lib/tdb/common/io.c
index 7e29c38..60f1d49 100644
--- a/lib/tdb/common/io.c
+++ b/lib/tdb/common/io.c
@@ -364,6 +364,10 @@ tdb_off_t tdb_expand_adjust(tdb_off_t map_size, tdb_off_t size, int page_size)
least 25% more space. if the DB is smaller than 100MiB,
otherwise grow it by 10% only. */
if (map_size > 100 * 1024 * 1024) {
+ /* Beware overflow! */
+ if ((tdb_off_t)(map_size * 1.10) < map_size) {
+ return 0xFFFFFFFF;
+ }
new_size = map_size * 1.10;
} else {
new_size = map_size * 1.25;
diff --git a/lib/tdb/common/transaction.c b/lib/tdb/common/transaction.c
index a2498ec..e670bdf 100644
--- a/lib/tdb/common/transaction.c
+++ b/lib/tdb/common/transaction.c
@@ -745,6 +745,11 @@ static int tdb_recovery_allocate(struct tdb_context *tdb,
- sizeof(rec);
new_end = recovery_head + sizeof(rec) + *recovery_max_size;
+ if (new_end < recovery_head) {
+ tdb->ecode = TDB_ERR_IO;
+ TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_recovery_allocate: TDB has grown too large\n"));
+ return -1;
+ }
if (methods->tdb_expand_file(tdb, tdb->transaction->old_map_size,
new_end - tdb->transaction->old_map_size)
diff --git a/lib/tdb/test/run-overlength.c b/lib/tdb/test/run-overlength.c
new file mode 100644
index 0000000..3fb20ab
--- /dev/null
+++ b/lib/tdb/test/run-overlength.c
@@ -0,0 +1,146 @@
+#include "../common/tdb_private.h"
+#include "../common/io.c"
+#include "../common/tdb.c"
+#include "../common/lock.c"
+#include "../common/freelist.c"
+#include "../common/traverse.c"
+#include "../common/transaction.c"
+#include "../common/error.c"
+#include "../common/open.c"
+#include "../common/check.c"
+#include "../common/hash.c"
+#include "tap-interface.h"
+#include <stdlib.h>
+#include "logging.h"
+
+static int tdb_expand_file_sparse(struct tdb_context *tdb,
+ tdb_off_t size,
+ tdb_off_t addition)
+{
+ if (tdb->read_only || tdb->traverse_read) {
+ tdb->ecode = TDB_ERR_RDONLY;
+ return -1;
+ }
+
+ if (ftruncate(tdb->fd, size+addition) == -1) {
+ char b = 0;
+ ssize_t written = pwrite(tdb->fd, &b, 1, (size+addition) - 1);
+ if (written == 0) {
+ /* try once more, potentially revealing errno */
+ written = pwrite(tdb->fd, &b, 1, (size+addition) - 1);
+ }
+ if (written == 0) {
+ /* again - give up, guessing errno */
+ errno = ENOSPC;
+ }
+ if (written != 1) {
+ TDB_LOG((tdb, TDB_DEBUG_FATAL, "expand_file to %d failed (%s)\n",
+ size+addition, strerror(errno)));
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
+static const struct tdb_methods large_io_methods = {
+ tdb_read,
+ tdb_write,
+ tdb_next_hash_chain,
+ tdb_oob,
+ tdb_expand_file_sparse
+};
+
+static int test_traverse(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data,
+ void *_data)
+{
+ TDB_DATA *expect = _data;
+ ok1(key.dsize == strlen("hi"));
+ ok1(memcmp(key.dptr, "hi", strlen("hi")) == 0);
+ ok1(data.dsize == expect->dsize);
+ ok1(memcmp(data.dptr, expect->dptr, data.dsize) == 0);
+ return 0;
+}
+
+int main(int argc, char *argv[])
+{
+ struct tdb_context *tdb;
+ TDB_DATA key, orig_data, data;
+ uint32_t hash;
+ tdb_off_t rec_ptr;
+ struct tdb_record rec;
+ int ret;
+
+ plan_tests(24);
+ tdb = tdb_open_ex("run-36-file.tdb", 1024, TDB_CLEAR_IF_FIRST,
+ O_CREAT|O_TRUNC|O_RDWR, 0600, &taplogctx, NULL);
+
+ ok1(tdb);
+ tdb->methods = &large_io_methods;
+
+ key.dsize = strlen("hi");
+ key.dptr = (void *)"hi";
+ orig_data.dsize = strlen("world");
+ orig_data.dptr = (void *)"world";
+
+ /* Enlarge the file (internally multiplies by 2). */
+ ret = tdb_expand(tdb, 2147460000);
+#ifdef HAVE_INCOHERENT_MMAP
+ /* This can fail due to mmap failure on 32 bit systems. */
+ if (ret == -1) {
+ /* These should now fail. */
+ ok1(tdb_store(tdb, key, orig_data, TDB_INSERT) == -1);
+ data = tdb_fetch(tdb, key);
+ ok1(data.dptr == NULL);
+ ok1(tdb_traverse(tdb, test_traverse, &orig_data) == -1);
+ ok1(tdb_delete(tdb, key) == -1);
+ ok1(tdb_traverse(tdb, test_traverse, NULL) == -1);
+ /* Skip the rest... */
+ for (ret = 0; ret < 24 - 6; ret++)
+ ok1(1);
+ tdb_close(tdb);
+ return exit_status();
+ }
+#endif
+ ok1(ret == 0);
+
+ /* Put an entry in, and check it. */
+ ok1(tdb_store(tdb, key, orig_data, TDB_INSERT) == 0);
+
+ data = tdb_fetch(tdb, key);
+ ok1(data.dsize == strlen("world"));
+ ok1(memcmp(data.dptr, "world", strlen("world")) == 0);
+ free(data.dptr);
+
+ /* That currently fills at the end, make sure that's true. */
+ hash = tdb->hash_fn(&key);
+ rec_ptr = tdb_find_lock_hash(tdb, key, hash, F_RDLCK, &rec);
+ ok1(rec_ptr);
+ ok1(rec_ptr > 2U*1024*1024*1024);
+ tdb_unlock(tdb, BUCKET(rec.full_hash), F_RDLCK);
+
+ /* Traverse must work. */
+ ok1(tdb_traverse(tdb, test_traverse, &orig_data) == 1);
+
+ /* Delete should work. */
+ ok1(tdb_delete(tdb, key) == 0);
+
+ ok1(tdb_traverse(tdb, test_traverse, NULL) == 0);
+
+ /* Transactions will fail, due to overlength! */
+ ok1(tdb_transaction_start(tdb) == 0);
+ ok1(tdb_store(tdb, key, orig_data, TDB_INSERT) == 0);
+
+ data = tdb_fetch(tdb, key);
+ ok1(data.dsize == strlen("world"));
+ ok1(memcmp(data.dptr, "world", strlen("world")) == 0);
+ free(data.dptr);
+ ok1(tdb_transaction_commit(tdb) == -1);
+ ok1(tdb_error(tdb) == TDB_ERR_IO);
+
+ ok1(tdb_traverse(tdb, test_traverse, NULL) == 0);
+ ok1(tdb_check(tdb, NULL, NULL) == 0);
+ tdb_close(tdb);
+
+ return exit_status();
+}
diff --git a/lib/tdb/wscript b/lib/tdb/wscript
index 7a3d0fa..35287d2 100644
--- a/lib/tdb/wscript
+++ b/lib/tdb/wscript
@@ -141,6 +141,8 @@ def build(bld):
'replace tdb-test-helpers', includes='include', install=False)
bld.SAMBA_BINARY('tdb1-run-open-during-transaction', 'test/run-open-during-transaction.c',
'replace tdb-test-helpers', includes='include', install=False)
+ bld.SAMBA_BINARY('tdb1-run-overlength', 'test/run-overlength.c',
+ 'replace tdb-test-helpers', includes='include', install=False)
bld.SAMBA_BINARY('tdb1-run-readonly-check', 'test/run-readonly-check.c',
'replace tdb-test-helpers', includes='include', install=False)
bld.SAMBA_BINARY('tdb1-run-rescue', 'test/run-rescue.c',
@@ -189,7 +191,7 @@ def testonly(ctx):
if not os.path.exists(link):
os.symlink(os.path.abspath(os.path.join(env.cwd, 'test')), link)
- for f in 'tdb1-run-3G-file', 'tdb1-run-bad-tdb-header', 'tdb1-run', 'tdb1-run-check', 'tdb1-run-corrupt', 'tdb1-run-die-during-transaction', 'tdb1-run-endian', 'tdb1-run-incompatible', 'tdb1-run-nested-transactions', 'tdb1-run-nested-traverse', 'tdb1-run-no-lock-during-traverse', 'tdb1-run-oldhash', 'tdb1-run-open-during-transaction', 'tdb1-run-readonly-check', 'tdb1-run-rescue', 'tdb1-run-rescue-find_entry', 'tdb1-run-rwlock-check', 'tdb1-run-summary', 'tdb1-run-transaction-expand', 'tdb1-run-traverse-in-transaction', 'tdb1-run-wronghash-fail', 'tdb1-run-zero-append':
+ for f in 'tdb1-run-3G-file', 'tdb1-run-bad-tdb-header', 'tdb1-run', 'tdb1-run-check', 'tdb1-run-corrupt', 'tdb1-run-die-during-transaction', 'tdb1-run-endian', 'tdb1-run-incompatible', 'tdb1-run-nested-transactions', 'tdb1-run-nested-traverse', 'tdb1-run-no-lock-during-traverse', 'tdb1-run-oldhash', 'tdb1-run-open-during-transaction', 'tdb1-run-overlength', 'tdb1-run-readonly-check', 'tdb1-run-rescue', 'tdb1-run-rescue-find_entry', 'tdb1-run-rwlock-check', 'tdb1-run-summary', 'tdb1-run-transaction-expand', 'tdb1-run-traverse-in-transaction', 'tdb1-run-wronghash-fail', 'tdb1-run-zero-append':
cmd = "cd " + testdir + " && " + os.path.abspath(os.path.join(Utils.g_module.blddir, f)) + " > test-output 2>&1"
print("..." + f)
ret = samba_utils.RUN_COMMAND(cmd)
--
1.8.1.2
More information about the samba-technical
mailing list