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