From 322c0b86cd902af6cd42aa7f99b18c672a528a90 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 15 Aug 2022 10:49:13 +0200 Subject: [PATCH 1/3] s3:smbd: share_mode_flags_set() takes SMB2_LEASE_* values We currently only ever pass SMB2_LEASE_READ and both have the same value of 0x1, so for now it's only cosmetic, but that will change soon. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15148 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme (cherry picked from commit 7592aad4d7a84d0ac66a156a22af3ad77803e55c) --- source3/smbd/open.c | 2 +- source3/smbd/smb2_oplock.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 803c0fbd347a..992656563bf9 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -2789,7 +2789,7 @@ grant: if (granted & SMB2_LEASE_READ) { uint32_t acc, sh, ls; share_mode_flags_get(lck, &acc, &sh, &ls); - ls |= SHARE_MODE_LEASE_READ; + ls |= SMB2_LEASE_READ; share_mode_flags_set(lck, acc, sh, ls, NULL); } diff --git a/source3/smbd/smb2_oplock.c b/source3/smbd/smb2_oplock.c index 367b0fd3191a..a555d3ae4fe2 100644 --- a/source3/smbd/smb2_oplock.c +++ b/source3/smbd/smb2_oplock.c @@ -1310,7 +1310,7 @@ static void contend_level2_oplocks_begin_default(files_struct *fsp, */ uint32_t acc, sh, ls; share_mode_flags_get(lck, &acc, &sh, &ls); - ls &= ~SHARE_MODE_LEASE_READ; + ls &= ~SMB2_LEASE_READ; share_mode_flags_set(lck, acc, sh, ls, NULL); } -- 2.34.1 From 4db4bda518cda872fad673255a4113c6b8962eb8 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 17 Aug 2022 17:07:08 +0200 Subject: [PATCH 2/3] s4:torture/smb2: add smb2.lease.v[1,2]_bug_15148 This demonstrates the bug that happens with a write to a file handle holding an R lease, while there are other openers without any lease. When one of the other openers writes to the file, the R lease of the only lease holder isn't broken to NONE. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15148 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme (cherry picked from commit 9e5ff607eb1b9c45c8836d3cff9d51b418740b87) --- selftest/knownfail.d/lease_bug_15148 | 2 + source4/torture/smb2/lease.c | 208 +++++++++++++++++++++++++++ 2 files changed, 210 insertions(+) create mode 100644 selftest/knownfail.d/lease_bug_15148 diff --git a/selftest/knownfail.d/lease_bug_15148 b/selftest/knownfail.d/lease_bug_15148 new file mode 100644 index 000000000000..e06a3d1b030e --- /dev/null +++ b/selftest/knownfail.d/lease_bug_15148 @@ -0,0 +1,2 @@ +^samba3.smb2.lease.v1_bug15148 +^samba3.smb2.lease.v2_bug15148 diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c index 43b418c5acf4..a2c354dc02ad 100644 --- a/source4/torture/smb2/lease.c +++ b/source4/torture/smb2/lease.c @@ -4556,6 +4556,210 @@ done: return ret; } +static bool test_lease_v1_bug_15148(struct torture_context *tctx, + struct smb2_tree *tree) +{ + TALLOC_CTX *mem_ctx = talloc_new(tctx); + struct smb2_create io1; + struct smb2_create io2; + struct smb2_lease ls1; + struct smb2_lease ls2; + struct smb2_handle h1 = {{0}}; + struct smb2_handle h2 = {{0}}; + struct smb2_write w; + NTSTATUS status; + const char *fname = "lease_v1_bug_15148.dat"; + bool ret = true; + uint32_t caps; + + caps = smb2cli_conn_server_capabilities(tree->session->transport->conn); + if (!(caps & SMB2_CAP_LEASING)) { + torture_skip(tctx, "leases are not supported"); + } + + tree->session->transport->lease.handler = torture_lease_handler; + tree->session->transport->lease.private_data = tree; + tree->session->transport->oplock.handler = torture_oplock_handler; + tree->session->transport->oplock.private_data = tree; + + smb2_util_unlink(tree, fname); + + torture_reset_lease_break_info(tctx, &lease_break_info); + + /* Grab R lease over connection 1a */ + smb2_lease_create(&io1, &ls1, false, fname, LEASE1, smb2_util_lease_state("R")); + status = smb2_create(tree, mem_ctx, &io1); + CHECK_STATUS(status, NT_STATUS_OK); + h1 = io1.out.file.handle; + CHECK_CREATED(&io1, CREATED, FILE_ATTRIBUTE_ARCHIVE); + CHECK_LEASE(&io1, "R", true, LEASE1, 0); + + CHECK_NO_BREAK(tctx); + + /* Contend with LEASE2. */ + smb2_lease_create(&io2, &ls2, false, fname, LEASE2, smb2_util_lease_state("R")); + status = smb2_create(tree, mem_ctx, &io2); + CHECK_STATUS(status, NT_STATUS_OK); + h2 = io2.out.file.handle; + CHECK_CREATED(&io2, EXISTED, FILE_ATTRIBUTE_ARCHIVE); + CHECK_LEASE(&io2, "R", true, LEASE2, 0); + + CHECK_NO_BREAK(tctx); + + ZERO_STRUCT(w); + w.in.file.handle = h1; + w.in.offset = 0; + w.in.data = data_blob_talloc(mem_ctx, NULL, 4096); + memset(w.in.data.data, 'o', w.in.data.length); + status = smb2_write(tree, &w); + CHECK_STATUS(status, NT_STATUS_OK); + + ls2.lease_epoch += 1; + CHECK_BREAK_INFO("R", "", LEASE2); + + torture_reset_lease_break_info(tctx, &lease_break_info); + + ZERO_STRUCT(w); + w.in.file.handle = h1; + w.in.offset = 0; + w.in.data = data_blob_talloc(mem_ctx, NULL, 4096); + memset(w.in.data.data, 'O', w.in.data.length); + status = smb2_write(tree, &w); + CHECK_STATUS(status, NT_STATUS_OK); + + CHECK_NO_BREAK(tctx); + + ZERO_STRUCT(w); + w.in.file.handle = h2; + w.in.offset = 0; + w.in.data = data_blob_talloc(mem_ctx, NULL, 4096); + memset(w.in.data.data, 'o', w.in.data.length); + status = smb2_write(tree, &w); + CHECK_STATUS(status, NT_STATUS_OK); + + ls1.lease_epoch += 1; + CHECK_BREAK_INFO("R", "", LEASE1); + + done: + smb2_util_close(tree, h1); + smb2_util_close(tree, h2); + + smb2_util_unlink(tree, fname); + + talloc_free(mem_ctx); + + return ret; +} + +static bool test_lease_v2_bug_15148(struct torture_context *tctx, + struct smb2_tree *tree) +{ + TALLOC_CTX *mem_ctx = talloc_new(tctx); + struct smb2_create io1; + struct smb2_create io2; + struct smb2_lease ls1; + struct smb2_lease ls2; + struct smb2_handle h1 = {{0}}; + struct smb2_handle h2 = {{0}}; + struct smb2_write w; + NTSTATUS status; + const char *fname = "lease_v2_bug_15148.dat"; + bool ret = true; + uint32_t caps; + enum protocol_types protocol; + + caps = smb2cli_conn_server_capabilities(tree->session->transport->conn); + if (!(caps & SMB2_CAP_LEASING)) { + torture_skip(tctx, "leases are not supported"); + } + + protocol = smbXcli_conn_protocol(tree->session->transport->conn); + if (protocol < PROTOCOL_SMB3_00) { + torture_skip(tctx, "v2 leases are not supported"); + } + + tree->session->transport->lease.handler = torture_lease_handler; + tree->session->transport->lease.private_data = tree; + tree->session->transport->oplock.handler = torture_oplock_handler; + tree->session->transport->oplock.private_data = tree; + + smb2_util_unlink(tree, fname); + + torture_reset_lease_break_info(tctx, &lease_break_info); + + /* Grab R lease over connection 1a */ + smb2_lease_v2_create(&io1, &ls1, false, fname, LEASE1, NULL, + smb2_util_lease_state("R"), 0x4711); + status = smb2_create(tree, mem_ctx, &io1); + CHECK_STATUS(status, NT_STATUS_OK); + h1 = io1.out.file.handle; + CHECK_CREATED(&io1, CREATED, FILE_ATTRIBUTE_ARCHIVE); + ls1.lease_epoch += 1; + CHECK_LEASE_V2(&io1, "R", true, LEASE1, + 0, 0, ls1.lease_epoch); + + CHECK_NO_BREAK(tctx); + + /* Contend with LEASE2. */ + smb2_lease_v2_create(&io2, &ls2, false, fname, LEASE2, NULL, + smb2_util_lease_state("R"), 0x11); + status = smb2_create(tree, mem_ctx, &io2); + CHECK_STATUS(status, NT_STATUS_OK); + h2 = io2.out.file.handle; + CHECK_CREATED(&io2, EXISTED, FILE_ATTRIBUTE_ARCHIVE); + ls2.lease_epoch += 1; + CHECK_LEASE_V2(&io2, "R", true, LEASE2, + 0, 0, ls2.lease_epoch); + + CHECK_NO_BREAK(tctx); + + ZERO_STRUCT(w); + w.in.file.handle = h1; + w.in.offset = 0; + w.in.data = data_blob_talloc(mem_ctx, NULL, 4096); + memset(w.in.data.data, 'o', w.in.data.length); + status = smb2_write(tree, &w); + CHECK_STATUS(status, NT_STATUS_OK); + + ls2.lease_epoch += 1; + CHECK_BREAK_INFO_V2(tree->session->transport, + "R", "", LEASE2, ls2.lease_epoch); + + torture_reset_lease_break_info(tctx, &lease_break_info); + + ZERO_STRUCT(w); + w.in.file.handle = h1; + w.in.offset = 0; + w.in.data = data_blob_talloc(mem_ctx, NULL, 4096); + memset(w.in.data.data, 'O', w.in.data.length); + status = smb2_write(tree, &w); + CHECK_STATUS(status, NT_STATUS_OK); + + CHECK_NO_BREAK(tctx); + + ZERO_STRUCT(w); + w.in.file.handle = h2; + w.in.offset = 0; + w.in.data = data_blob_talloc(mem_ctx, NULL, 4096); + memset(w.in.data.data, 'o', w.in.data.length); + status = smb2_write(tree, &w); + CHECK_STATUS(status, NT_STATUS_OK); + + ls1.lease_epoch += 1; + CHECK_BREAK_INFO_V2(tree->session->transport, + "R", "", LEASE1, ls1.lease_epoch); + + done: + smb2_util_close(tree, h1); + smb2_util_close(tree, h2); + + smb2_util_unlink(tree, fname); + + talloc_free(mem_ctx); + + return ret; +} + struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx) { struct torture_suite *suite = @@ -4604,6 +4808,10 @@ struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx) test_lease_duplicate_create); torture_suite_add_1smb2_test(suite, "duplicate_open", test_lease_duplicate_open); + torture_suite_add_1smb2_test(suite, "v1_bug15148", + test_lease_v1_bug_15148); + torture_suite_add_1smb2_test(suite, "v2_bug15148", + test_lease_v2_bug_15148); suite->description = talloc_strdup(suite, "SMB2-LEASE tests"); -- 2.34.1 From 9036b7e636036e8b0de69acf647c621422b2f151 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 15 Aug 2022 22:45:17 +0200 Subject: [PATCH 3/3] s3:smbd: only clear LEASE_READ if there's no read lease is left If contend_level2_oplocks_begin_default() skips break it's own lease, we should not clear SHARE_MODE_LEASE_READ in share_mode_data->flags. Otherwise that lease won't see any lease break notifications for writes from other clients (file handles not using the same lease key). So we need to count the number existing read leases (including the one with the same lease key) in order to know it's safe to clear SMB2_LEASE_READ/SHARE_MODE_LEASE_READ. Otherwise the next run (likely from another client) will get the wrong result from file_has_read_lease(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=15148 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Thu Aug 18 19:41:33 UTC 2022 on sn-devel-184 (cherry picked from commit 96e2a82760ea06a89b7387b5cd3e864732afded3) --- selftest/knownfail.d/lease_bug_15148 | 2 -- source3/smbd/smb2_oplock.c | 11 ++++++----- 2 files changed, 6 insertions(+), 7 deletions(-) delete mode 100644 selftest/knownfail.d/lease_bug_15148 diff --git a/selftest/knownfail.d/lease_bug_15148 b/selftest/knownfail.d/lease_bug_15148 deleted file mode 100644 index e06a3d1b030e..000000000000 --- a/selftest/knownfail.d/lease_bug_15148 +++ /dev/null @@ -1,2 +0,0 @@ -^samba3.smb2.lease.v1_bug15148 -^samba3.smb2.lease.v2_bug15148 diff --git a/source3/smbd/smb2_oplock.c b/source3/smbd/smb2_oplock.c index a555d3ae4fe2..1f143840b341 100644 --- a/source3/smbd/smb2_oplock.c +++ b/source3/smbd/smb2_oplock.c @@ -1141,7 +1141,7 @@ struct break_to_none_state { struct file_id id; struct smb2_lease_key lease_key; struct GUID client_guid; - size_t num_broken; + size_t num_read_leases; }; static bool do_break_lease_to_none(struct share_mode_entry *e, @@ -1175,6 +1175,8 @@ static bool do_break_lease_to_none(struct share_mode_entry *e, return false; } + state->num_read_leases += 1; + our_own = smb2_lease_equal(&state->client_guid, &state->lease_key, &e->client_guid, @@ -1190,8 +1192,6 @@ static bool do_break_lease_to_none(struct share_mode_entry *e, send_break_to_none(state->sconn->msg_ctx, &state->id, e); - state->num_broken += 1; - return false; } @@ -1225,11 +1225,12 @@ static bool do_break_oplock_to_none(struct share_mode_entry *e, return false; } + state->num_read_leases += 1; + /* Paranoia .... */ SMB_ASSERT(!EXCLUSIVE_OPLOCK_TYPE(e->op_type)); send_break_to_none(state->sconn->msg_ctx, &state->id, e); - state->num_broken += 1; return false; } @@ -1303,7 +1304,7 @@ static void contend_level2_oplocks_begin_default(files_struct *fsp, DBG_WARNING("share_mode_forall_entries failed\n"); } - if (state.num_broken == 0) { + if (state.num_read_leases == 0) { /* * Lazy update here. It might be that the read lease * has gone in the meantime. -- 2.34.1