summaryrefslogtreecommitdiff
path: root/lib/tdb
diff options
context:
space:
mode:
authorVolker Lendecke <vl@samba.org>2010-01-29 18:21:09 +0100
committerVolker Lendecke <vl@samba.org>2010-02-01 15:06:29 +0100
commit531059696e17d1ee538310d81af309c107d08e3e (patch)
treec33befb379b9237bb82b8f4340ee81d5e3488ee1 /lib/tdb
parent42f512552190396f69404a135d19e4325bde7d16 (diff)
downloadsamba-531059696e17d1ee538310d81af309c107d08e3e.tar.gz
samba-531059696e17d1ee538310d81af309c107d08e3e.tar.bz2
samba-531059696e17d1ee538310d81af309c107d08e3e.zip
tdb: fix an early release of the global lock that can cause data corruption
There was a bug in tdb where the tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1); (ending the transaction-"mutex") was done before the /* remove the recovery marker */ This means that when a transaction is committed there is a window where another opener of the file sees the transaction marker while the transaction committer is still fully functional and working on it. This led to transaction being rolled back by that second opener of the file while transaction_commit() gave no error to the caller. This patch moves the F_UNLCK to after the recovery marker was removed, closing this window.
Diffstat (limited to 'lib/tdb')
-rw-r--r--lib/tdb/common/transaction.c15
1 files changed, 10 insertions, 5 deletions
diff --git a/lib/tdb/common/transaction.c b/lib/tdb/common/transaction.c
index 20f2bfc2cd..b8988ea830 100644
--- a/lib/tdb/common/transaction.c
+++ b/lib/tdb/common/transaction.c
@@ -135,6 +135,9 @@ struct tdb_transaction {
bool prepared;
tdb_off_t magic_offset;
+ /* set when the GLOBAL_LOCK has been taken */
+ bool global_lock_taken;
+
/* old file size before transaction */
tdb_len_t old_map_size;
@@ -603,6 +606,11 @@ int _tdb_transaction_cancel(struct tdb_context *tdb)
}
}
+ if (tdb->transaction->global_lock_taken) {
+ tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1);
+ tdb->transaction->global_lock_taken = false;
+ }
+
/* remove any global lock created during the transaction */
if (tdb->global_lock.count != 0) {
tdb_brlock(tdb, FREELIST_TOP, F_UNLCK, F_SETLKW, 0, 4*tdb->header.hash_size);
@@ -947,11 +955,12 @@ static int _tdb_transaction_prepare_commit(struct tdb_context *tdb)
return -1;
}
+ tdb->transaction->global_lock_taken = true;
+
if (!(tdb->flags & TDB_NOSYNC)) {
/* write the recovery data to the end of the file */
if (transaction_setup_recovery(tdb, &tdb->transaction->magic_offset) == -1) {
TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_transaction_prepare_commit: failed to setup recovery data\n"));
- tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1);
_tdb_transaction_cancel(tdb);
return -1;
}
@@ -966,7 +975,6 @@ static int _tdb_transaction_prepare_commit(struct tdb_context *tdb)
tdb->transaction->old_map_size) == -1) {
tdb->ecode = TDB_ERR_IO;
TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_transaction_prepare_commit: expansion failed\n"));
- tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1);
_tdb_transaction_cancel(tdb);
return -1;
}
@@ -1056,7 +1064,6 @@ int tdb_transaction_commit(struct tdb_context *tdb)
tdb_transaction_recover(tdb);
_tdb_transaction_cancel(tdb);
- tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1);
TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_transaction_commit: write failed\n"));
return -1;
@@ -1072,8 +1079,6 @@ int tdb_transaction_commit(struct tdb_context *tdb)
return -1;
}
- tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1);
-
/*
TODO: maybe write to some dummy hdr field, or write to magic
offset without mmap, before the last sync, instead of the