Latest leases patchset - getting there !

Jeremy Allison jra at samba.org
Mon Nov 17 22:24:28 MST 2014


On Tue, Nov 18, 2014 at 03:18:28AM +0100, Stefan (metze) Metzmacher wrote:
> 
> Thanks! Can you add a check that we can't upgrade over 2_10
> instead of verifying this only for 3_00?
> 
> > +	/* Close everything.. */
> > +	smb2_util_close(tree_2_1, h);
> > +	smb2_util_close(tree_3_0, h1);
> 
> Here we should add something like this:
> 
> 	/* Try to upgrade to RWH over connection 3_0 */
> 	ls1.lease_state = smb2_util_lease_state("RWH");
> 	status = smb2_create(tree_3_0, mem_ctx, &io);
> 	CHECK_STATUS(status, NT_STATUS_OK);
> 	h1 = io.out.file.handle;
> 	CHECK_CREATED(&io, EXISTED, FILE_ATTRIBUTE_ARCHIVE);
> 	/* Should have been denied. */
> 	CHECK_LEASE(&io, "", true, LEASE1, 0);  <-- what do we get here?
>         smb2_util_close(tree_3_0, h1);
> 
> > +	smb2_util_close(tree_3_0, h2);

Done.

Here is a replacement for lp6, that includes
these tests.

I'll finish fixing the other tests tomorrow,
and the re-upload a complete patchset for
people for follow.

Jeremy.
-------------- next part --------------
From 38341a698f7f9ddf6982bf7466c8e5e45f2efde0 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 17 Nov 2014 12:41:38 -0800
Subject: [PATCH 1/4] s4: smb2 : smbtorture. Fix client libraries to obey
 "client max protocol".

Ensure for SMB2 tests client max protocol is initially set to SMB3,
then use the value given for max_protocol in the options struct.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source4/libcli/smb2/connect.c | 3 ++-
 source4/torture/smb2/smb2.c   | 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/source4/libcli/smb2/connect.c b/source4/libcli/smb2/connect.c
index c81bd67..9535380 100644
--- a/source4/libcli/smb2/connect.c
+++ b/source4/libcli/smb2/connect.c
@@ -149,7 +149,8 @@ static void smb2_connect_socket_done(struct composite_context *creq)
 
 	subreq = smbXcli_negprot_send(state, state->ev,
 				      state->transport->conn, timeout_msec,
-				      PROTOCOL_SMB2_02, PROTOCOL_LATEST);
+				      PROTOCOL_SMB2_02,
+				      state->transport->options.max_protocol);
 	if (tevent_req_nomem(subreq, req)) {
 		return;
 	}
diff --git a/source4/torture/smb2/smb2.c b/source4/torture/smb2/smb2.c
index 853c4f6..598c3ba 100644
--- a/source4/torture/smb2/smb2.c
+++ b/source4/torture/smb2/smb2.c
@@ -23,6 +23,8 @@
 #include "torture/smbtorture.h"
 #include "torture/smb2/proto.h"
 #include "../lib/util/dlinklist.h"
+#include "../lib/cmdline/popt_common.h"
+#include "lib/param/param.h"
 
 static bool wrap_simple_1smb2_test(struct torture_context *torture_ctx,
 				   struct torture_tcase *tcase,
@@ -146,6 +148,7 @@ struct torture_test *torture_suite_add_2smb2_test(struct torture_suite *suite,
 NTSTATUS torture_smb2_init(void)
 {
 	struct torture_suite *suite = torture_suite_create(talloc_autofree_context(), "smb2");
+	lpcfg_set_cmdline(cmdline_lp_ctx, "client max protocol", "SMB3");
 	torture_suite_add_simple_test(suite, "connect", torture_smb2_connect);
 	torture_suite_add_suite(suite, torture_smb2_scan_init());
 	torture_suite_add_suite(suite, torture_smb2_getinfo_init());
-- 
1.9.1


From 71e9e9885e3ac3cc51ee410932f8485f2adf7c0a Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 17 Nov 2014 12:52:23 -0800
Subject: [PATCH 2/4] s3: leases : Fix error in lease_fname_match_parser() when
 creating a new file.

When creating a new file, if lease_fname_match_parser() is called *at all*,
it means that there was an existing lease_key/guid pair for this name.

Ensure we break the existing lease before creating the new file.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/open.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 8833d42..1eaf341 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -4139,19 +4139,21 @@ static void lease_fname_match_parser(
 		return;
 	}
 
-	if (!state->file_existed) {
-		/* New file. */
-		state->match_status = NT_STATUS_OK;
-		return;
-	}
-
-	if (num_file_ids == 1 && file_id_equal(&ids[0],&state->id)) {
+	if (state->file_existed &&
+			num_file_ids == 1 &&
+			file_id_equal(&ids[0],&state->id)) {
 		/* Common case - non-dynamic share. We're ok.. */
 		state->match_status = NT_STATUS_OK;
 		return;
 	}
 
-	/* More than one file id, or not equal. Don't allow leases. */
+	/*
+	 * More than one file id, or not equal, or new file
+	 * being created and there's already an existing lease
+	 * on this (client_guid, lease id) pair.
+	 * Don't allow leases.
+	 */
+
 	state->match_status = NT_STATUS_OPLOCK_NOT_GRANTED;
 	state->num_file_ids = num_file_ids;
 	state->ids = talloc_memdup(talloc_tos(),
@@ -4178,6 +4180,8 @@ static NTSTATUS lease_match(connection_struct *conn,
 	state.file_existed = VALID_STAT(fname->st);
 	if (state.file_existed) {
 		state.id = vfs_file_id_from_sbuf(conn, &fname->st);
+	} else {
+		memset(&state.id, '\0', sizeof(state.id));
 	}
 
 	status = leases_db_parse(&sconn->client->connections->smb2.client.guid,
-- 
1.9.1


From d1a62da5c80b53adb5ce094952a8eb90559d1df5 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 17 Nov 2014 14:17:34 -0800
Subject: [PATCH 3/4] s4: smb2 : torture: Add new dynamic_share leases test.

Depends on new share "dynamic_share" being set up containing an %R
in the path= statement.

Shows we will break leases and fail to grant new ones
if we get a lease_key+client guid pair match on files
with different fileid's, as can happen on dynamic shares.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 selftest/target/Samba3.pm    |  10 +++
 source4/torture/smb2/lease.c | 185 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 195 insertions(+)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index ae0a3a6..50c2642 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -879,6 +879,12 @@ sub provision($$$$$$)
 	my $badnames_shrdir="$shrdir/badnames";
 	push(@dirs,$badnames_shrdir);
 
+	my $lease1_shrdir="$shrdir/SMB2_10";
+	push(@dirs,$lease1_shrdir);
+
+	my $lease2_shrdir="$shrdir/SMB3_00";
+	push(@dirs,$lease2_shrdir);
+
 	# this gets autocreated by winbindd
 	my $wbsockdir="$prefix_abs/winbindd";
 	my $wbsockprivdir="$lockdir/winbindd_privileged";
@@ -1224,6 +1230,10 @@ sub provision($$$$$$)
 [badname-tmp]
 	path = $badnames_shrdir
 	guest ok = yes
+
+[dynamic_share]
+	path = $shrdir/%R
+	guest ok = yes
 	";
 	close(CONF);
 
diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c
index dda54eb..52a6cbb 100644
--- a/source4/torture/smb2/lease.c
+++ b/source4/torture/smb2/lease.c
@@ -27,6 +27,7 @@
 #include "torture/smb2/proto.h"
 #include "torture/util.h"
 #include "libcli/smb/smbXcli_base.h"
+#include "lib/param/param.h"
 
 #define CHECK_VAL(v, correct) do { \
 	if ((v) != (correct)) { \
@@ -2693,6 +2694,189 @@ static bool test_lease_timeout(struct torture_context *tctx,
 	return ret;
 }
 
+static bool test_lease_dynamic_share(struct torture_context *tctx,
+				   struct smb2_tree *tree1a)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
+	struct smb2_create io;
+	struct smb2_lease ls1;
+	struct smb2_handle h, h1, h2;
+	struct smb2_write w;
+	NTSTATUS status;
+	const char *fname = "dynamic_path.dat";
+	bool ret = true;
+	uint32_t caps;
+	struct smb2_tree *tree_2_1 = NULL;
+	struct smb2_tree *tree_3_0 = NULL;
+	struct smbcli_options options2_1;
+	struct smbcli_options options3_0;
+	const char *orig_share = NULL;
+
+	options2_1 = tree1a->session->transport->options;
+	options3_0 = tree1a->session->transport->options;
+
+	caps = smb2cli_conn_server_capabilities(tree1a->session->transport->conn);
+	if (!(caps & SMB2_CAP_LEASING)) {
+		torture_skip(tctx, "leases are not supported");
+	}
+
+	/*
+	 * Save off original share name and change it to dynamic_share.
+	 * This must have been pre-created with a dynamic path containing
+	 * %R.
+	 */
+
+	orig_share = lpcfg_parm_string(tctx->lp_ctx, NULL, "torture", "share");
+	orig_share = talloc_strdup(tctx->lp_ctx, orig_share);
+	if (orig_share == NULL) {
+		torture_result(tctx, TORTURE_FAIL, __location__ "no memory\n");
+                ret = false;
+                goto done;
+	}
+	lpcfg_set_cmdline(tctx->lp_ctx, "torture:share", "dynamic_share");
+
+	/* Set max protocol to SMB2.1 */
+	options2_1.max_protocol = PROTOCOL_SMB2_10;
+	/* create a new connection (same client_guid) */
+	if (!torture_smb2_connection_ext(tctx, 0, &options2_1, &tree_2_1)) {
+		torture_warning(tctx, "couldn't reconnect max protocol 2.1, bailing\n");
+		ret = false;
+		goto done;
+	}
+
+	tree_2_1->session->transport->lease.handler = torture_lease_handler;
+	tree_2_1->session->transport->lease.private_data = tree_2_1;
+	tree_2_1->session->transport->oplock.handler = torture_oplock_handler;
+	tree_2_1->session->transport->oplock.private_data = tree_2_1;
+
+	smb2_util_unlink(tree_2_1, fname);
+
+	/* Set max protocol to SMB3.0 */
+	options3_0.max_protocol = PROTOCOL_SMB3_00;
+	/* create a new connection (same client_guid) */
+	if (!torture_smb2_connection_ext(tctx, 0, &options3_0, &tree_3_0)) {
+		torture_warning(tctx, "couldn't reconnect max protocol 3.0, bailing\n");
+		ret = false;
+		goto done;
+	}
+
+	tree_3_0->session->transport->lease.handler = torture_lease_handler;
+	tree_3_0->session->transport->lease.private_data = tree_3_0;
+	tree_3_0->session->transport->oplock.handler = torture_oplock_handler;
+	tree_3_0->session->transport->oplock.private_data = tree_3_0;
+
+	smb2_util_unlink(tree_3_0, fname);
+
+	ZERO_STRUCT(break_info);
+
+	/* Get RWH lease over connection 2_1 */
+	smb2_lease_create(&io, &ls1, false, fname, LEASE1, smb2_util_lease_state("RWH"));
+	status = smb2_create(tree_2_1, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	CHECK_CREATED(&io, CREATED, FILE_ATTRIBUTE_ARCHIVE);
+	CHECK_LEASE(&io, "RWH", true, LEASE1, 0);
+	h = io.out.file.handle;
+
+	/* Write some data into it. */
+	w.in.file.handle = h;
+	w.in.offset      = 0;
+	w.in.data        = data_blob_talloc(mem_ctx, NULL, 4096);
+	memset(w.in.data.data, '1', w.in.data.length);
+	status = smb2_write(tree_2_1, &w);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	/* Open the same name over connection 3_0. */
+	smb2_lease_create(&io, &ls1, false, fname, LEASE1, smb2_util_lease_state("RWH"));
+	status = smb2_create(tree_3_0, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	h1 = io.out.file.handle;
+	CHECK_CREATED(&io, CREATED, FILE_ATTRIBUTE_ARCHIVE);
+
+	/* h1 should have replied with NONE. */
+	CHECK_LEASE(&io, "", true, LEASE1, 0);
+
+	/* We should have broken h to NONE. */
+	CHECK_BREAK_INFO("RWH", "", LEASE1);
+
+	/* Try to upgrade to RWH over connection 2_1 */
+	smb2_lease_create(&io, &ls1, false, fname, LEASE1, smb2_util_lease_state("RWH"));
+	status = smb2_create(tree_2_1, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	h2 = io.out.file.handle;
+	CHECK_VAL(io.out.create_action, NTCREATEX_ACTION_EXISTED);
+	CHECK_VAL(io.out.size, 4096);
+	CHECK_VAL(io.out.file_attr, FILE_ATTRIBUTE_ARCHIVE);
+	/* Should have been denied. */
+	CHECK_LEASE(&io, "", true, LEASE1, 0);
+	smb2_util_close(tree_2_1, h2);
+
+	/* Try to upgrade to RWH over connection 3_0 */
+	smb2_lease_create(&io, &ls1, false, fname, LEASE1, smb2_util_lease_state("RWH"));
+	status = smb2_create(tree_3_0, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	h2 = io.out.file.handle;
+	CHECK_VAL(io.out.create_action, NTCREATEX_ACTION_EXISTED);
+	CHECK_VAL(io.out.size, 0);
+	CHECK_VAL(io.out.file_attr, FILE_ATTRIBUTE_ARCHIVE);
+	/* Should have been denied. */
+	CHECK_LEASE(&io, "", true, LEASE1, 0);
+	smb2_util_close(tree_3_0, h2);
+
+	/* Write some data into it. */
+	w.in.file.handle = h1;
+	w.in.offset      = 0;
+	w.in.data        = data_blob_talloc(mem_ctx, NULL, 1024);
+	memset(w.in.data.data, '2', w.in.data.length);
+	status = smb2_write(tree_3_0, &w);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	/* Close everything.. */
+	smb2_util_close(tree_2_1, h);
+	smb2_util_close(tree_3_0, h1);
+
+	/* And ensure we can get a lease ! */
+	smb2_lease_create(&io, &ls1, false, fname, LEASE1, smb2_util_lease_state("RWH"));
+	status = smb2_create(tree_2_1, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	CHECK_VAL(io.out.create_action, NTCREATEX_ACTION_EXISTED);
+	CHECK_VAL(io.out.file_attr, FILE_ATTRIBUTE_ARCHIVE);
+	CHECK_LEASE(&io, "RWH", true, LEASE1, 0);
+	h = io.out.file.handle;
+	/* And the file is the right size. */
+	CHECK_VAL(io.out.size, 4096);				\
+	/* Close it. */
+	smb2_util_close(tree_2_1, h);
+
+	/* And ensure we can get a lease ! */
+	smb2_lease_create(&io, &ls1, false, fname, LEASE1, smb2_util_lease_state("RWH"));
+	status = smb2_create(tree_3_0, mem_ctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	CHECK_VAL(io.out.create_action, NTCREATEX_ACTION_EXISTED);
+	CHECK_VAL(io.out.file_attr, FILE_ATTRIBUTE_ARCHIVE);
+	CHECK_LEASE(&io, "RWH", true, LEASE1, 0);
+	h = io.out.file.handle;
+	/* And the file is the right size. */
+	CHECK_VAL(io.out.size, 1024);				\
+	/* Close it. */
+	smb2_util_close(tree_3_0, h);
+
+ done:
+
+	smb2_util_close(tree_2_1, h);
+	smb2_util_close(tree_3_0, h1);
+	smb2_util_close(tree_3_0, h2);
+
+	smb2_util_unlink(tree_2_1, fname);
+	smb2_util_unlink(tree_3_0, fname);
+
+	/* Set sharename back. */
+	lpcfg_set_cmdline(tctx->lp_ctx, "torture:share", orig_share);
+
+	talloc_free(mem_ctx);
+
+	return ret;
+}
+
 
 struct torture_suite *torture_smb2_lease_init(void)
 {
@@ -2722,6 +2906,7 @@ struct torture_suite *torture_smb2_lease_init(void)
 	torture_suite_add_1smb2_test(suite, "v2_epoch2", test_lease_v2_epoch2);
 	torture_suite_add_1smb2_test(suite, "v2_epoch3", test_lease_v2_epoch3);
 	torture_suite_add_1smb2_test(suite, "v2_complex1", test_lease_v2_complex1);
+	torture_suite_add_1smb2_test(suite, "dynamic_share", test_lease_dynamic_share);
 	torture_suite_add_1smb2_test(suite, "timeout", test_lease_timeout);
 
 	suite->description = talloc_strdup(suite, "SMB2-LEASE tests");
-- 
1.9.1


From 064050d0c9970cfa7037eda1c64ab934ab0045dd Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 17 Nov 2014 21:20:44 -0800
Subject: [PATCH 4/4] s3: leases: leases_db - Add some level10 debug to help
 track down issues.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/locking/leases_db.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/source3/locking/leases_db.c b/source3/locking/leases_db.c
index 29b8e35..1154e67 100644
--- a/source3/locking/leases_db.c
+++ b/source3/locking/leases_db.c
@@ -156,6 +156,8 @@ NTSTATUS leases_db_add(const struct GUID *client_guid,
 		value->num_file_ids += 1;
 
 	} else {
+		DEBUG(10, ("%s: new record\n", __func__));
+
 		new_value = (struct leases_db_value) {
 			.num_file_ids = 1,
 			.ids = id,
@@ -175,6 +177,11 @@ NTSTATUS leases_db_add(const struct GUID *client_guid,
 		goto out;
 	}
 
+	if (DEBUGLEVEL >= 10) {
+		DEBUG(10, ("%s:\n", __func__));
+		NDR_PRINT_DEBUG(leases_db_value, value);
+	}
+
 	db_value = make_tdb_data(blob.data, blob.length);
 
 	status = dbwrap_record_store(rec, db_value, 0);
@@ -260,8 +267,10 @@ NTSTATUS leases_db_del(const struct GUID *client_guid,
 	value->num_file_ids -= 1;
 
 	if (value->num_file_ids == 0) {
+		DEBUG(10, ("%s: deleting record\n", __func__));
 		status = dbwrap_record_delete(rec);
 	} else {
+		DEBUG(10, ("%s: updating record\n", __func__));
 		ndr_err = ndr_push_struct_blob(
 			&blob, talloc_tos(), value,
 			(ndr_push_flags_fn_t)ndr_push_leases_db_value);
@@ -272,6 +281,11 @@ NTSTATUS leases_db_del(const struct GUID *client_guid,
 			goto out;
 		}
 
+		if (DEBUGLEVEL >= 10) {
+			DEBUG(10, ("%s:\n", __func__));
+			NDR_PRINT_DEBUG(leases_db_value, value);
+		}
+
 		db_value = make_tdb_data(blob.data, blob.length);
 
 		status = dbwrap_record_store(rec, db_value, 0);
@@ -321,6 +335,11 @@ static void leases_db_parser(TDB_DATA key, TDB_DATA data, void *private_data)
 		return;
 	}
 
+	if (DEBUGLEVEL >= 10) {
+		DEBUG(10, ("%s:\n", __func__));
+		NDR_PRINT_DEBUG(leases_db_value, value);
+	}
+
 	state->parser(value->num_file_ids,
 			value->ids, value->filename, value->stream_name,
 			state->private_data);
-- 
1.9.1



More information about the samba-technical mailing list