The branch, master has been updated
via 397d193 tdb: Test for readonly lock upgrade bug
via a6f1532 tdb: Do lock upgrades properly
via 97cafdc tdb: Fix some signed/unsigned hickups
via 3667876 selftest: Test for bug 12558
from 94b7672 third_party: Add cmocka 1.1.1
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 397d1936ec34fa573c7d46cc51c971986e8f7fe8
Author: Volker Lendecke <[email protected]>
Date: Tue Nov 8 17:01:56 2016 +0100
tdb: Test for readonly lock upgrade bug
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Jeremy Allison <[email protected]>
Autobuild-User(master): Jeremy Allison <[email protected]>
Autobuild-Date(master): Tue Apr 11 00:33:31 CEST 2017 on sn-devel-144
commit a6f1532d7fedc6b6f36b511aebe8998e9452b7ff
Author: Volker Lendecke <[email protected]>
Date: Mon Nov 7 21:40:15 2016 +0100
tdb: Do lock upgrades properly
When a process holds a readlock and wants to upgrade, this needs to be
reflected in the underlying lock. Without this, it is possible to cheat:
One process holds a readlock, and another process wants to write this
record. All the writer has to do is take a readonly lock on the key and
then do the store.
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Jeremy Allison <[email protected]>
commit 97cafdcfaaaff17ebffe873b5e31462f0d1268b1
Author: Volker Lendecke <[email protected]>
Date: Mon Nov 7 21:38:58 2016 +0100
tdb: Fix some signed/unsigned hickups
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Jeremy Allison <[email protected]>
commit 3667876ebebb7181d89834e6038e2d7218c98797
Author: Volker Lendecke <[email protected]>
Date: Fri Apr 7 16:33:57 2017 +0200
selftest: Test for bug 12558
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12558
Signed-off-by: Volker Lendecke <[email protected]>
Reviewed-by: Jeremy Allison <[email protected]>
-----------------------------------------------------------------------
Summary of changes:
lib/tdb/common/lock.c | 28 ++++-
lib/tdb/test/run-rdlock-upgrade.c | 166 ++++++++++++++++++++++++++++++
lib/tdb/wscript | 1 +
source3/script/tests/test_smbclient_s3.sh | 11 ++
4 files changed, 201 insertions(+), 5 deletions(-)
create mode 100644 lib/tdb/test/run-rdlock-upgrade.c
Changeset truncated at 500 lines:
diff --git a/lib/tdb/common/lock.c b/lib/tdb/common/lock.c
index 195dbb5..4ad70cf 100644
--- a/lib/tdb/common/lock.c
+++ b/lib/tdb/common/lock.c
@@ -294,7 +294,7 @@ fail:
static struct tdb_lock_type *find_nestlock(struct tdb_context *tdb,
tdb_off_t offset)
{
- unsigned int i;
+ int i;
for (i=0; i<tdb->num_lockrecs; i++) {
if (tdb->lockrecs[i].off == offset) {
@@ -321,6 +321,22 @@ int tdb_nest_lock(struct tdb_context *tdb, uint32_t
offset, int ltype,
new_lck = find_nestlock(tdb, offset);
if (new_lck) {
+ if ((new_lck->ltype == F_RDLCK) && (ltype == F_WRLCK)) {
+ if (!tdb_have_mutexes(tdb)) {
+ int ret;
+ /*
+ * Upgrade the underlying fcntl
+ * lock. Mutexes don't do readlocks,
+ * so this only applies to fcntl
+ * locking.
+ */
+ ret = tdb_brlock(tdb, ltype, offset, 1, flags);
+ if (ret != 0) {
+ return ret;
+ }
+ }
+ new_lck->ltype = F_WRLCK;
+ }
/*
* Just increment the in-memory struct, posix locks
* don't stack.
@@ -381,7 +397,7 @@ static int tdb_lock_and_recover(struct tdb_context *tdb)
static bool have_data_locks(const struct tdb_context *tdb)
{
- unsigned int i;
+ int i;
for (i = 0; i < tdb->num_lockrecs; i++) {
if (tdb->lockrecs[i].off >= lock_offset(-1))
@@ -560,7 +576,8 @@ static int tdb_allrecord_check(struct tdb_context *tdb, int
ltype,
return -1;
}
- if (tdb->allrecord_lock.count && tdb->allrecord_lock.ltype == ltype) {
+ if (tdb->allrecord_lock.count &&
+ tdb->allrecord_lock.ltype == (uint32_t)ltype) {
tdb->allrecord_lock.count++;
return 0;
}
@@ -706,7 +723,7 @@ int tdb_allrecord_unlock(struct tdb_context *tdb, int
ltype, bool mark_lock)
}
/* Upgradable locks are marked as write locks. */
- if (tdb->allrecord_lock.ltype != ltype
+ if (tdb->allrecord_lock.ltype != (uint32_t)ltype
&& (!tdb->allrecord_lock.off || ltype != F_RDLCK)) {
tdb->ecode = TDB_ERR_LOCK;
return -1;
@@ -945,7 +962,8 @@ bool tdb_have_extra_locks(struct tdb_context *tdb)
/* The transaction code uses this to remove all locks. */
void tdb_release_transaction_locks(struct tdb_context *tdb)
{
- unsigned int i, active = 0;
+ int i;
+ unsigned int active = 0;
if (tdb->allrecord_lock.count != 0) {
tdb_allrecord_unlock(tdb, tdb->allrecord_lock.ltype, false);
diff --git a/lib/tdb/test/run-rdlock-upgrade.c
b/lib/tdb/test/run-rdlock-upgrade.c
new file mode 100644
index 0000000..042001b
--- /dev/null
+++ b/lib/tdb/test/run-rdlock-upgrade.c
@@ -0,0 +1,166 @@
+#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 "../common/mutex.c"
+#include "tap-interface.h"
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <stdarg.h>
+#include "logging.h"
+
+static TDB_DATA key, data;
+
+static void do_chainlock(const char *name, int tdb_flags, int up, int down)
+{
+ struct tdb_context *tdb;
+ int ret;
+ ssize_t nread, nwritten;
+ char c = 0;
+
+ tdb = tdb_open_ex(name, 3, tdb_flags,
+ O_RDWR|O_CREAT, 0755, &taplogctx, NULL);
+ ok(tdb, "tdb_open_ex should succeed");
+
+ ret = tdb_chainlock_read(tdb, key);
+ ok(ret == 0, "tdb_chainlock_read should succeed");
+
+ nwritten = write(up, &c, sizeof(c));
+ ok(nwritten == sizeof(c), "write should succeed");
+
+ nread = read(down, &c, sizeof(c));
+ ok(nread == 0, "read should succeed");
+
+ exit(0);
+}
+
+static void do_trylock(const char *name, int tdb_flags, int up, int down)
+{
+ struct tdb_context *tdb;
+ int ret;
+ ssize_t nread, nwritten;
+ char c = 0;
+
+ tdb = tdb_open_ex(name, 3, tdb_flags,
+ O_RDWR|O_CREAT, 0755, &taplogctx, NULL);
+ ok(tdb, "tdb_open_ex should succeed");
+
+ /*
+ * tdb used to have a bug where with fcntl locks an upgrade
+ * from a readlock to writelock did not check for the
+ * underlying fcntl lock. Mutexes don't distinguish between
+ * readlocks and writelocks, so that bug does not apply here.
+ */
+
+ ret = tdb_chainlock_read(tdb, key);
+ ok(ret == 0, "tdb_chainlock_read should succeed");
+
+ ret = tdb_chainlock_nonblock(tdb, key);
+ ok(ret == -1, "tdb_chainlock_nonblock should fail");
+
+ nwritten = write(up, &c, sizeof(c));
+ ok(nwritten == sizeof(c), "write should succeed");
+
+ nread = read(down, &c, sizeof(c));
+ ok(nread == 0, "read should succeed");
+
+ exit(0);
+}
+
+static int do_tests(const char *name, int tdb_flags)
+{
+ int ret;
+ pid_t chainlock_child, store_child;
+ int chainlock_down[2];
+ int chainlock_up[2];
+ int store_down[2];
+ int store_up[2];
+ char c;
+ ssize_t nread;
+
+ key.dsize = strlen("hi");
+ key.dptr = discard_const_p(uint8_t, "hi");
+ data.dsize = strlen("world");
+ data.dptr = discard_const_p(uint8_t, "world");
+
+ ret = pipe(chainlock_down);
+ ok(ret == 0, "pipe should succeed");
+
+ ret = pipe(chainlock_up);
+ ok(ret == 0, "pipe should succeed");
+
+ ret = pipe(store_down);
+ ok(ret == 0, "pipe should succeed");
+
+ ret = pipe(store_up);
+ ok(ret == 0, "pipe should succeed");
+
+ chainlock_child = fork();
+ ok(chainlock_child != -1, "fork should succeed");
+
+ if (chainlock_child == 0) {
+ close(chainlock_up[0]);
+ close(chainlock_down[1]);
+ close(store_up[0]);
+ close(store_up[1]);
+ close(store_down[0]);
+ close(store_down[1]);
+ do_chainlock(name, tdb_flags,
+ chainlock_up[1], chainlock_down[0]);
+ exit(0);
+ }
+ close(chainlock_up[1]);
+ close(chainlock_down[0]);
+
+ nread = read(chainlock_up[0], &c, sizeof(c));
+ ok(nread == sizeof(c), "read should succeed");
+
+ /*
+ * Now we have a process holding a chain read lock. Start
+ * another process trying to write lock. This should fail.
+ */
+
+ store_child = fork();
+ ok(store_child != -1, "fork should succeed");
+
+ if (store_child == 0) {
+ close(chainlock_up[0]);
+ close(chainlock_down[1]);
+ close(store_up[0]);
+ close(store_down[1]);
+ do_trylock(name, tdb_flags,
+ store_up[1], store_down[0]);
+ exit(0);
+ }
+ close(store_up[1]);
+ close(store_down[0]);
+
+ nread = read(store_up[0], &c, sizeof(c));
+ ok(nread == sizeof(c), "read should succeed");
+
+ close(chainlock_up[0]);
+ close(chainlock_down[1]);
+ close(store_up[0]);
+ close(store_down[1]);
+ diag("%s tests done", name);
+ return exit_status();
+}
+
+int main(int argc, char *argv[])
+{
+ int ret;
+
+ ret = do_tests("rdlock-upgrade.tdb",
+ TDB_CLEAR_IF_FIRST |
+ TDB_INCOMPATIBLE_HASH);
+ ok(ret == 0, "rdlock-upgrade.tdb tests should succeed");
+
+ return exit_status();
+}
diff --git a/lib/tdb/wscript b/lib/tdb/wscript
index 0d682eb..693787c 100644
--- a/lib/tdb/wscript
+++ b/lib/tdb/wscript
@@ -34,6 +34,7 @@ tdb1_unit_tests = [
'run-readonly-check',
'run-rescue',
'run-rescue-find_entry',
+ 'run-rdlock-upgrade',
'run-rwlock-check',
'run-summary',
'run-transaction-expand',
diff --git a/source3/script/tests/test_smbclient_s3.sh
b/source3/script/tests/test_smbclient_s3.sh
index a7bffcb..0ae85e8 100755
--- a/source3/script/tests/test_smbclient_s3.sh
+++ b/source3/script/tests/test_smbclient_s3.sh
@@ -346,6 +346,17 @@ test_msdfs_link()
tmpfile=$PREFIX/smbclient.in.$$
prompt=" msdfs-target "
+ cmd='$SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/msdfs-share -I
$SERVER_IP $ADDARGS -m nt1 -c dir 2>&1'
+ out=`eval $cmd`
+ ret=$?
+
+ if [ $ret != 0 ] ; then
+ echo "$out"
+ echo "failed listing msfds-share\ with error $ret"
+ false
+ return
+ fi
+
cat > $tmpfile <<EOF
ls
cd \\msdfs-src1
--
Samba Shared Repository
