The Samba-Bugzilla – Attachment 18038 Details for
Bug 15346
2-3min delays at reconnect with smb2_validate_sequence_number: bad message_id 2
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patches for v4-18-test
bfixes-tmp418.txt (text/plain), 27.86 KB, created by
Stefan Metzmacher
on 2023-08-08 14:05:08 UTC
(
hide
)
Description:
Patches for v4-18-test
Filename:
MIME Type:
Creator:
Stefan Metzmacher
Created:
2023-08-08 14:05:08 UTC
Size:
27.86 KB
patch
obsolete
>From c39754e003c30ec7c7c953a88e450e42c69935de Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 7 Aug 2023 11:03:41 +0200 >Subject: [PATCH 1/6] s4:torture/smb2: let torture_smb2_con_sopt() use > smb2_connect() > >There's no need for smb2_connect_ext(). > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15346 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> >(cherry picked from commit ade663ee6ca1a2813b203ea667d933f4dab9e7b7) >--- > source4/torture/smb2/util.c | 25 ++++++++++++------------- > 1 file changed, 12 insertions(+), 13 deletions(-) > >diff --git a/source4/torture/smb2/util.c b/source4/torture/smb2/util.c >index ae6c14ebcbd2..01b1b2f0793a 100644 >--- a/source4/torture/smb2/util.c >+++ b/source4/torture/smb2/util.c >@@ -475,19 +475,18 @@ bool torture_smb2_con_sopt(struct torture_context *tctx, > return false; > } > >- status = smb2_connect_ext(tctx, >- host, >- lpcfg_smb_ports(tctx->lp_ctx), >- share, >- lpcfg_resolve_context(tctx->lp_ctx), >- samba_cmdline_get_creds(), >- 0, >- tree, >- tctx->ev, >- &options, >- lpcfg_socket_options(tctx->lp_ctx), >- lpcfg_gensec_settings(tctx, tctx->lp_ctx) >- ); >+ status = smb2_connect(tctx, >+ host, >+ lpcfg_smb_ports(tctx->lp_ctx), >+ share, >+ lpcfg_resolve_context(tctx->lp_ctx), >+ samba_cmdline_get_creds(), >+ tree, >+ tctx->ev, >+ &options, >+ lpcfg_socket_options(tctx->lp_ctx), >+ lpcfg_gensec_settings(tctx, tctx->lp_ctx) >+ ); > if (!NT_STATUS_IS_OK(status)) { > torture_comment(tctx, "Failed to connect to SMB2 share \\\\%s\\%s - %s\n", > host, share, nt_errstr(status)); >-- >2.34.1 > > >From 82b2ba15e2fc2979c90ef65e8cf24d6e47e8ef69 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 7 Aug 2023 11:03:41 +0200 >Subject: [PATCH 2/6] s4:torture/smb2: let us have a common > torture_smb2_con_share() > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15346 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> >(cherry picked from commit dc5a500f0a76720b2a5cb5b1142cf4c35cb6bdea) >--- > source4/torture/smb2/acls.c | 34 --------------------------------- > source4/torture/smb2/util.c | 29 +++++++++++++++++++--------- > source4/torture/vfs/acl_xattr.c | 34 --------------------------------- > 3 files changed, 20 insertions(+), 77 deletions(-) > >diff --git a/source4/torture/smb2/acls.c b/source4/torture/smb2/acls.c >index a5df9da3b454..a27d4e079e67 100644 >--- a/source4/torture/smb2/acls.c >+++ b/source4/torture/smb2/acls.c >@@ -2139,40 +2139,6 @@ done: > } > #endif > >-/** >- * SMB2 connect with explicit share >- **/ >-static bool torture_smb2_con_share(struct torture_context *tctx, >- const char *share, >- struct smb2_tree **tree) >-{ >- struct smbcli_options options; >- NTSTATUS status; >- const char *host = torture_setting_string(tctx, "host", NULL); >- >- lpcfg_smbcli_options(tctx->lp_ctx, &options); >- >- status = smb2_connect_ext(tctx, >- host, >- lpcfg_smb_ports(tctx->lp_ctx), >- share, >- lpcfg_resolve_context(tctx->lp_ctx), >- samba_cmdline_get_creds(), >- 0, >- tree, >- tctx->ev, >- &options, >- lpcfg_socket_options(tctx->lp_ctx), >- lpcfg_gensec_settings(tctx, tctx->lp_ctx) >- ); >- if (!NT_STATUS_IS_OK(status)) { >- torture_comment(tctx, "Failed to connect to SMB2 share \\\\%s\\%s - %s\n", >- host, share, nt_errstr(status)); >- return false; >- } >- return true; >-} >- > static bool test_access_based(struct torture_context *tctx, > struct smb2_tree *tree) > { >diff --git a/source4/torture/smb2/util.c b/source4/torture/smb2/util.c >index 01b1b2f0793a..ecf80d9105b1 100644 >--- a/source4/torture/smb2/util.c >+++ b/source4/torture/smb2/util.c >@@ -459,22 +459,16 @@ bool torture_smb2_connection(struct torture_context *tctx, struct smb2_tree **tr > /** > * SMB2 connect with share from soption > **/ >-bool torture_smb2_con_sopt(struct torture_context *tctx, >- const char *soption, >- struct smb2_tree **tree) >+bool torture_smb2_con_share(struct torture_context *tctx, >+ const char *share, >+ struct smb2_tree **tree) > { > struct smbcli_options options; > NTSTATUS status; > const char *host = torture_setting_string(tctx, "host", NULL); >- const char *share = torture_setting_string(tctx, soption, NULL); > > lpcfg_smbcli_options(tctx->lp_ctx, &options); > >- if (share == NULL) { >- torture_comment(tctx, "No share for option %s\n", soption); >- return false; >- } >- > status = smb2_connect(tctx, > host, > lpcfg_smb_ports(tctx->lp_ctx), >@@ -495,6 +489,23 @@ bool torture_smb2_con_sopt(struct torture_context *tctx, > return true; > } > >+/** >+ * SMB2 connect with share from soption >+ **/ >+bool torture_smb2_con_sopt(struct torture_context *tctx, >+ const char *soption, >+ struct smb2_tree **tree) >+{ >+ const char *share = torture_setting_string(tctx, soption, NULL); >+ >+ if (share == NULL) { >+ torture_comment(tctx, "No share for option %s\n", soption); >+ return false; >+ } >+ >+ return torture_smb2_con_share(tctx, share, tree); >+} >+ > /* > create and return a handle to a test file > with a specific access mask >diff --git a/source4/torture/vfs/acl_xattr.c b/source4/torture/vfs/acl_xattr.c >index 46d8ead6cb03..1deb2b3b9984 100644 >--- a/source4/torture/vfs/acl_xattr.c >+++ b/source4/torture/vfs/acl_xattr.c >@@ -47,40 +47,6 @@ > } \ > } while (0) > >-/** >- * SMB2 connect with explicit share >- **/ >-static bool torture_smb2_con_share(struct torture_context *tctx, >- const char *share, >- struct smb2_tree **tree) >-{ >- struct smbcli_options options; >- NTSTATUS status; >- const char *host = torture_setting_string(tctx, "host", NULL); >- >- lpcfg_smbcli_options(tctx->lp_ctx, &options); >- >- status = smb2_connect_ext(tctx, >- host, >- lpcfg_smb_ports(tctx->lp_ctx), >- share, >- lpcfg_resolve_context(tctx->lp_ctx), >- samba_cmdline_get_creds(), >- 0, >- tree, >- tctx->ev, >- &options, >- lpcfg_socket_options(tctx->lp_ctx), >- lpcfg_gensec_settings(tctx, tctx->lp_ctx) >- ); >- if (!NT_STATUS_IS_OK(status)) { >- printf("Failed to connect to SMB2 share \\\\%s\\%s - %s\n", >- host, share, nt_errstr(status)); >- return false; >- } >- return true; >-} >- > static bool test_default_acl_posix(struct torture_context *tctx, > struct smb2_tree *tree_unused) > { >-- >2.34.1 > > >From 3f069855df2d09f5a03a8e4259afca4c1ea47a1e Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 7 Aug 2023 12:22:43 +0200 >Subject: [PATCH 3/6] s4:torture/smb2: make it possible to pass existing_conn > to smb2_connect_ext() > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15346 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> >(cherry picked from commit 2b93058be3f6e5eaee239ad3b0e707c62089d18e) >--- > source4/libcli/smb2/connect.c | 4 +++- > source4/torture/smb2/util.c | 1 + > 2 files changed, 4 insertions(+), 1 deletion(-) > >diff --git a/source4/libcli/smb2/connect.c b/source4/libcli/smb2/connect.c >index 1f68d90538b2..64b678654468 100644 >--- a/source4/libcli/smb2/connect.c >+++ b/source4/libcli/smb2/connect.c >@@ -405,6 +405,7 @@ NTSTATUS smb2_connect_ext(TALLOC_CTX *mem_ctx, > const char *share, > struct resolve_context *resolve_ctx, > struct cli_credentials *credentials, >+ struct smbXcli_conn **existing_conn, > uint64_t previous_session_id, > struct smb2_tree **tree, > struct tevent_context *ev, >@@ -429,7 +430,7 @@ NTSTATUS smb2_connect_ext(TALLOC_CTX *mem_ctx, > resolve_ctx, > credentials, > false, /* fallback_to_anonymous */ >- NULL, /* existing_conn */ >+ existing_conn, > previous_session_id, > options, > socket_options, >@@ -473,6 +474,7 @@ NTSTATUS smb2_connect(TALLOC_CTX *mem_ctx, > > status = smb2_connect_ext(mem_ctx, host, ports, share, resolve_ctx, > credentials, >+ NULL, /* existing_conn */ > 0, /* previous_session_id */ > tree, ev, options, socket_options, > gensec_settings); >diff --git a/source4/torture/smb2/util.c b/source4/torture/smb2/util.c >index ecf80d9105b1..233f589c73f6 100644 >--- a/source4/torture/smb2/util.c >+++ b/source4/torture/smb2/util.c >@@ -426,6 +426,7 @@ bool torture_smb2_connection_ext(struct torture_context *tctx, > share, > lpcfg_resolve_context(tctx->lp_ctx), > samba_cmdline_get_creds(), >+ NULL, /* existing_conn */ > previous_session_id, > tree, > tctx->ev, >-- >2.34.1 > > >From cb87d5a9668975907d584cf2a0a1dee0df61238e Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Fri, 4 Aug 2023 17:16:14 +0200 >Subject: [PATCH 4/6] s4:torture/smb2: add smb2.multichannel.bugs.bug_15346 > >This demonstrates the race quite easily against >Samba and works fine against Windows Server 2022. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15346 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> >(cherry picked from commit 4028d6582907cf582730ceec56872d8584ad02e6) >--- > selftest/knownfail.d/samba3.smb2.multichannel | 1 + > source4/torture/smb2/multichannel.c | 315 ++++++++++++++++++ > 2 files changed, 316 insertions(+) > create mode 100644 selftest/knownfail.d/samba3.smb2.multichannel > >diff --git a/selftest/knownfail.d/samba3.smb2.multichannel b/selftest/knownfail.d/samba3.smb2.multichannel >new file mode 100644 >index 000000000000..26b80a39d5be >--- /dev/null >+++ b/selftest/knownfail.d/samba3.smb2.multichannel >@@ -0,0 +1 @@ >+^samba3.smb2.multichannel.bugs.bug_15346 >diff --git a/source4/torture/smb2/multichannel.c b/source4/torture/smb2/multichannel.c >index 801c1ec801f4..cae28ff1bed6 100644 >--- a/source4/torture/smb2/multichannel.c >+++ b/source4/torture/smb2/multichannel.c >@@ -31,6 +31,7 @@ > #include "lib/cmdline/cmdline.h" > #include "libcli/security/security.h" > #include "libcli/resolve/resolve.h" >+#include "lib/socket/socket.h" > #include "lib/param/param.h" > #include "lib/events/events.h" > #include "oplock_break_handler.h" >@@ -2343,6 +2344,315 @@ done: > return ret; > } > >+/* >+ * Test channel merging race >+ * This is a regression test for >+ * https://bugzilla.samba.org/show_bug.cgi?id=15346 >+ */ >+struct test_multichannel_bug_15346_conn; >+ >+struct test_multichannel_bug_15346_state { >+ struct torture_context *tctx; >+ struct test_multichannel_bug_15346_conn *conns; >+ size_t num_conns; >+ size_t num_ready; >+ bool asserted; >+ bool looping; >+}; >+ >+struct test_multichannel_bug_15346_conn { >+ struct test_multichannel_bug_15346_state *state; >+ size_t idx; >+ struct smbXcli_conn *smbXcli; >+ struct tevent_req *nreq; >+ struct tevent_req *ereq; >+}; >+ >+static void test_multichannel_bug_15346_ndone(struct tevent_req *subreq); >+static void test_multichannel_bug_15346_edone(struct tevent_req *subreq); >+ >+static void test_multichannel_bug_15346_ndone(struct tevent_req *subreq) >+{ >+ struct test_multichannel_bug_15346_conn *conn = >+ (struct test_multichannel_bug_15346_conn *) >+ tevent_req_callback_data_void(subreq); >+ struct test_multichannel_bug_15346_state *state = conn->state; >+ struct torture_context *tctx = state->tctx; >+ NTSTATUS status; >+ bool ok = false; >+ >+ SMB_ASSERT(conn->nreq == subreq); >+ conn->nreq = NULL; >+ >+ status = smbXcli_negprot_recv(subreq, NULL, NULL); >+ TALLOC_FREE(subreq); >+ torture_assert_ntstatus_ok_goto(tctx, status, ok, asserted, >+ "smbXcli_negprot_recv failed"); >+ >+ torture_comment(tctx, "conn[%zu]: negprot done\n", conn->idx); >+ >+ conn->ereq = smb2cli_echo_send(conn->smbXcli, >+ tctx->ev, >+ conn->smbXcli, >+ state->num_conns * 2 * 1000); >+ torture_assert_goto(tctx, conn->ereq != NULL, ok, asserted, >+ "smb2cli_echo_send"); >+ tevent_req_set_callback(conn->ereq, >+ test_multichannel_bug_15346_edone, >+ conn); >+ >+ return; >+ >+asserted: >+ SMB_ASSERT(!ok); >+ state->asserted = true; >+ state->looping = false; >+ return; >+} >+ >+static void test_multichannel_bug_15346_edone(struct tevent_req *subreq) >+{ >+ struct test_multichannel_bug_15346_conn *conn = >+ (struct test_multichannel_bug_15346_conn *) >+ tevent_req_callback_data_void(subreq); >+ struct test_multichannel_bug_15346_state *state = conn->state; >+ struct torture_context *tctx = state->tctx; >+ NTSTATUS status; >+ bool ok = false; >+ >+ SMB_ASSERT(conn->ereq == subreq); >+ conn->ereq = NULL; >+ >+ status = smb2cli_echo_recv(subreq); >+ TALLOC_FREE(subreq); >+ torture_assert_ntstatus_ok_goto(tctx, status, ok, asserted, >+ "smb2cli_echo_recv failed"); >+ >+ torture_comment(tctx, "conn[%zu]: echo done\n", conn->idx); >+ >+ state->num_ready += 1; >+ if (state->num_ready < state->num_conns) { >+ return; >+ } >+ >+ state->looping = false; >+ return; >+ >+asserted: >+ SMB_ASSERT(!ok); >+ state->asserted = true; >+ state->looping = false; >+ return; >+} >+ >+static bool test_multichannel_bug_15346(struct torture_context *tctx, >+ struct smb2_tree *tree1) >+{ >+ const char *host = torture_setting_string(tctx, "host", NULL); >+ const char *share = torture_setting_string(tctx, "share", NULL); >+ struct resolve_context *resolve_ctx = lpcfg_resolve_context(tctx->lp_ctx); >+ const char *socket_options = lpcfg_socket_options(tctx->lp_ctx); >+ struct gensec_settings *gsettings = NULL; >+ bool ret = true; >+ NTSTATUS status; >+ struct smb2_transport *transport1 = tree1->session->transport; >+ struct test_multichannel_bug_15346_state *state = NULL; >+ uint32_t server_capabilities; >+ struct smb2_handle root_handle = {{0}}; >+ size_t i; >+ >+ if (smbXcli_conn_protocol(transport1->conn) < PROTOCOL_SMB3_00) { >+ torture_fail(tctx, >+ "SMB 3.X Dialect family required for Multichannel" >+ " tests\n"); >+ } >+ >+ server_capabilities = smb2cli_conn_server_capabilities( >+ tree1->session->transport->conn); >+ if (!(server_capabilities & SMB2_CAP_MULTI_CHANNEL)) { >+ torture_fail(tctx, >+ "Server does not support multichannel."); >+ } >+ >+ torture_comment(tctx, "Testing for BUG 15346\n"); >+ >+ state = talloc_zero(tctx, struct test_multichannel_bug_15346_state); >+ torture_assert_goto(tctx, state != NULL, ret, done, >+ "talloc_zero"); >+ state->tctx = tctx; >+ >+ gsettings = lpcfg_gensec_settings(state, tctx->lp_ctx); >+ torture_assert_goto(tctx, gsettings != NULL, ret, done, >+ "lpcfg_gensec_settings"); >+ >+ /* >+ * 32 is the W2K12R2 and W2K16 limit >+ * add 31 additional connections >+ */ >+ state->num_conns = 31; >+ state->conns = talloc_zero_array(state, >+ struct test_multichannel_bug_15346_conn, >+ state->num_conns); >+ torture_assert_goto(tctx, state->conns != NULL, ret, done, >+ "talloc_zero_array"); >+ >+ /* >+ * First we open additional the tcp connections >+ */ >+ >+ for (i = 0; i < state->num_conns; i++) { >+ struct test_multichannel_bug_15346_conn *conn = &state->conns[i]; >+ struct socket_context *sock = NULL; >+ uint16_t port = 445; >+ struct smbcli_options options = transport1->options; >+ >+ conn->state = state; >+ conn->idx = i; >+ >+ status = socket_connect_multi(state->conns, >+ host, >+ 1, &port, >+ resolve_ctx, >+ tctx->ev, >+ &sock, >+ &port); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "socket_connect_multi failed"); >+ >+ conn->smbXcli = smbXcli_conn_create(state->conns, >+ sock->fd, >+ host, >+ SMB_SIGNING_OFF, >+ 0, >+ &options.client_guid, >+ options.smb2_capabilities, >+ &options.smb3_capabilities); >+ torture_assert_goto(tctx, conn->smbXcli != NULL, ret, done, >+ "smbXcli_conn_create failed"); >+ sock->fd = -1; >+ TALLOC_FREE(sock); >+ } >+ >+ /* >+ * Now prepare the async SMB2 Negotiate requests >+ */ >+ for (i = 0; i < state->num_conns; i++) { >+ struct test_multichannel_bug_15346_conn *conn = &state->conns[i]; >+ >+ conn->nreq = smbXcli_negprot_send(conn->smbXcli, >+ tctx->ev, >+ conn->smbXcli, >+ state->num_conns * 2 * 1000, >+ smbXcli_conn_protocol(transport1->conn), >+ smbXcli_conn_protocol(transport1->conn), >+ 33, /* max_credits */ >+ NULL); >+ torture_assert_goto(tctx, conn->nreq != NULL, ret, done, "smbXcli_negprot_send"); >+ tevent_req_set_callback(conn->nreq, >+ test_multichannel_bug_15346_ndone, >+ conn); >+ } >+ >+ /* >+ * now we loop until all negprot and the first round >+ * of echos are done. >+ */ >+ state->looping = true; >+ while (state->looping) { >+ torture_assert_goto(tctx, tevent_loop_once(tctx->ev) == 0, >+ ret, done, "tevent_loop_once"); >+ } >+ >+ if (state->asserted) { >+ ret = false; >+ goto done; >+ } >+ >+ /* >+ * No we check that the connections are still usable >+ */ >+ for (i = 0; i < state->num_conns; i++) { >+ struct test_multichannel_bug_15346_conn *conn = &state->conns[i]; >+ >+ torture_comment(tctx, "conn[%zu]: checking echo again1\n", conn->idx); >+ >+ status = smb2cli_echo(conn->smbXcli, 1000); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "smb2cli_echo failed"); >+ } >+ >+ status = smb2_util_roothandle(tree1, &root_handle); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "smb2_util_roothandle failed"); >+ >+ /* >+ * No we check that the connections are still usable >+ */ >+ for (i = 0; i < state->num_conns; i++) { >+ struct test_multichannel_bug_15346_conn *conn = &state->conns[i]; >+ struct smbcli_options options = transport1->options; >+ struct smb2_session *session = NULL; >+ struct smb2_tree *tree = NULL; >+ union smb_fileinfo io; >+ >+ torture_comment(tctx, "conn[%zu]: checking session bind\n", conn->idx); >+ >+ /* >+ * Prepare smb2_{tree,session,transport} structures >+ * for the existing connection. >+ */ >+ options.only_negprot = true; >+ status = smb2_connect_ext(state->conns, >+ host, >+ NULL, /* ports */ >+ share, >+ resolve_ctx, >+ samba_cmdline_get_creds(), >+ &conn->smbXcli, >+ 0, /* previous_session_id */ >+ &tree, >+ tctx->ev, >+ &options, >+ socket_options, >+ gsettings); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "smb2_connect_ext failed"); >+ conn->smbXcli = tree->session->transport->conn; >+ >+ session = smb2_session_channel(tree->session->transport, >+ lpcfg_gensec_settings(tree, tctx->lp_ctx), >+ tree, >+ tree1->session); >+ torture_assert_goto(tctx, session != NULL, ret, done, >+ "smb2_session_channel failed"); >+ >+ status = smb2_session_setup_spnego(session, >+ samba_cmdline_get_creds(), >+ 0 /* previous_session_id */); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "smb2_session_setup_spnego failed"); >+ >+ /* >+ * Fix up the bound smb2_tree >+ */ >+ tree->session = session; >+ tree->smbXcli = tree1->smbXcli; >+ >+ ZERO_STRUCT(io); >+ io.generic.level = RAW_FILEINFO_SMB2_ALL_INFORMATION; >+ io.generic.in.file.handle = root_handle; >+ >+ status = smb2_getinfo_file(tree, tree, &io); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "smb2_getinfo_file failed"); >+ } >+ >+ done: >+ talloc_free(state); >+ >+ return ret; >+} >+ > struct torture_suite *torture_smb2_multichannel_init(TALLOC_CTX *ctx) > { > struct torture_suite *suite = torture_suite_create(ctx, "multichannel"); >@@ -2352,10 +2662,13 @@ struct torture_suite *torture_smb2_multichannel_init(TALLOC_CTX *ctx) > "oplocks"); > struct torture_suite *suite_leases = torture_suite_create(ctx, > "leases"); >+ struct torture_suite *suite_bugs = torture_suite_create(ctx, >+ "bugs"); > > torture_suite_add_suite(suite, suite_generic); > torture_suite_add_suite(suite, suite_oplocks); > torture_suite_add_suite(suite, suite_leases); >+ torture_suite_add_suite(suite, suite_bugs); > > torture_suite_add_1smb2_test(suite_generic, "interface_info", > test_multichannel_interface_info); >@@ -2377,6 +2690,8 @@ struct torture_suite *torture_smb2_multichannel_init(TALLOC_CTX *ctx) > test_multichannel_lease_break_test3); > torture_suite_add_1smb2_test(suite_leases, "test4", > test_multichannel_lease_break_test4); >+ torture_suite_add_1smb2_test(suite_bugs, "bug_15346", >+ test_multichannel_bug_15346); > > suite->description = talloc_strdup(suite, "SMB2 Multichannel tests"); > >-- >2.34.1 > > >From 2e33ed66b9617ba8fe43709d296509498cf0e777 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 3 Aug 2023 15:34:29 +0200 >Subject: [PATCH 5/6] s3:smbd: always clear filter_subreq in > smb2srv_client_mc_negprot_next() > >Commit 5d66d5b84f87267243dcd5223210906ce589af91 introduced a >'verify_again:' target, if we ever hit that, we would leak >the existing filter_subreq. > >Moving it just above a possible messaging_filtered_read_send() >will allow us to only clear it if we actually create a new >request. That will help us in the next commits. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15346 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> >(cherry picked from commit 50d61e5300250922bf36bb699306f82dff6a00b9) >--- > source3/smbd/smbXsrv_client.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/source3/smbd/smbXsrv_client.c b/source3/smbd/smbXsrv_client.c >index f5b296f65d2f..577131fc16f2 100644 >--- a/source3/smbd/smbXsrv_client.c >+++ b/source3/smbd/smbXsrv_client.c >@@ -555,7 +555,6 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req) > uint32_t seqnum = 0; > struct server_id last_server_id = { .pid = 0, }; > >- TALLOC_FREE(state->filter_subreq); > SMB_ASSERT(state->db_rec == NULL); > state->db_rec = smbXsrv_client_global_fetch_locked(table->global.db_ctx, > &client_guid, >@@ -636,6 +635,7 @@ verify_again: > SMB_ASSERT(last_server_id.pid == 0); > last_server_id = global->server_id; > >+ TALLOC_FREE(state->filter_subreq); > if (procid_is_local(&global->server_id)) { > subreq = messaging_filtered_read_send(state, > state->ev, >-- >2.34.1 > > >From 04407a824cc95d00ec17d71da476300614a00a5f Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 3 Aug 2023 15:45:45 +0200 >Subject: [PATCH 6/6] s3:smbd: fix multichannel connection passing race > >If a client opens multiple connection with the same >client guid in parallel, our connection passing is likely >to hit a race. > >Assume we have 3 processes: > >smbdA: This process already handles all connections for > a given client guid >smbdB: This just received a new connection with an > SMB2 neprot for the same client guid >smbdC: This also received a new connection with an > SMB2 neprot for the same client guid > >Now both smbdB and smbdC send a MSG_SMBXSRV_CONNECTION_PASS >message to smbdA. These messages contain the socket fd >for each connection. > >While waiting for a MSG_SMBXSRV_CONNECTION_PASSED message >from smbdA, both smbdB and smbdC watch the smbXcli_client.tdb >record for changes (that also verifies smbdA stays alive). > >Once one of them say smbdB received the MSG_SMBXSRV_CONNECTION_PASSED >message, the dbwrap_watch logic will wakeup smbdC in order to >let it recheck the smbXcli_client.tdb record in order to >handle the case where smbdA died or deleted its record. > >Now smbdC rechecks the smbXcli_client.tdb record, but it >was not woken because of a problem with smbdA. It meant >that smbdC sends a MSG_SMBXSRV_CONNECTION_PASS message >including the socket fd again. > >As a result smbdA got the socket fd from smbdC twice (or even more), >and creates two (or more) smbXsrv_connection structures for the >same low level tcp connection. And it also sends more than one >SMB2 negprot response. Depending on the tevent logic, it will >use different smbXsrv_connection structures to process incoming >requests. And this will almost immediately result in errors. > >The typicall error is: > smb2_validate_sequence_number: smb2_validate_sequence_number: bad message_id 2 (sequence id 2) (granted = 1, low = 1, range = 1) > >But other errors would also be possible. > >The detail that leads to the long delays on the client side is >that our smbd_server_connection_terminate_ex() code will close >only the fd of a single smbXsrv_connection, but the refcount >on the socket fd in the kernel is still not 0, so the tcp >connection is still alive... > >Now we remember the server_id of the process that we send >the MSG_SMBXSRV_CONNECTION_PASS message to. And just keep >watching the smbXcli_client.tdb record if the server_id >don't change. As we just need more patience to wait for >the MSG_SMBXSRV_CONNECTION_PASSED message. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15346 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> > >Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org> >Autobuild-Date(master): Tue Aug 8 13:59:58 UTC 2023 on atb-devel-224 > >(cherry picked from commit f348b84fbcf203ab1ba92840cf7aecd55dbf9aa0) >--- > selftest/knownfail.d/samba3.smb2.multichannel | 1 - > source3/smbd/smbXsrv_client.c | 31 +++++++++++++++++++ > 2 files changed, 31 insertions(+), 1 deletion(-) > delete mode 100644 selftest/knownfail.d/samba3.smb2.multichannel > >diff --git a/selftest/knownfail.d/samba3.smb2.multichannel b/selftest/knownfail.d/samba3.smb2.multichannel >deleted file mode 100644 >index 26b80a39d5be..000000000000 >--- a/selftest/knownfail.d/samba3.smb2.multichannel >+++ /dev/null >@@ -1 +0,0 @@ >-^samba3.smb2.multichannel.bugs.bug_15346 >diff --git a/source3/smbd/smbXsrv_client.c b/source3/smbd/smbXsrv_client.c >index 577131fc16f2..928a9d72caa5 100644 >--- a/source3/smbd/smbXsrv_client.c >+++ b/source3/smbd/smbXsrv_client.c >@@ -488,6 +488,7 @@ struct smb2srv_client_mc_negprot_state { > struct tevent_context *ev; > struct smbd_smb2_request *smb2req; > struct db_record *db_rec; >+ struct server_id sent_server_id; > uint64_t watch_instance; > uint32_t last_seqnum; > struct tevent_req *filter_subreq; >@@ -530,6 +531,8 @@ struct tevent_req *smb2srv_client_mc_negprot_send(TALLOC_CTX *mem_ctx, > > tevent_req_set_cleanup_fn(req, smb2srv_client_mc_negprot_cleanup); > >+ server_id_set_disconnected(&state->sent_server_id); >+ > smb2srv_client_mc_negprot_next(req); > > if (!tevent_req_is_in_progress(req)) { >@@ -625,6 +628,30 @@ verify_again: > return; > } > >+ if (server_id_equal(&state->sent_server_id, &global->server_id)) { >+ /* >+ * We hit a race with other concurrent connections, >+ * which have woken us. >+ * >+ * We already sent the pass or drop message to >+ * the process, so we need to wait for a >+ * response and not pass the connection >+ * again! Otherwise the process would >+ * receive the same tcp connection via >+ * more than one file descriptor and >+ * create more than one smbXsrv_connection >+ * structure for the same tcp connection, >+ * which means the client would see more >+ * than one SMB2 negprot response to its >+ * single SMB2 netprot request and we >+ * as server get the session keys and >+ * message id validation wrong >+ */ >+ goto watch_again; >+ } >+ >+ server_id_set_disconnected(&state->sent_server_id); >+ > /* > * If last_server_id is set, we expect > * smbXsrv_client_global_verify_record() >@@ -660,6 +687,7 @@ verify_again: > */ > goto verify_again; > } >+ state->sent_server_id = global->server_id; > if (tevent_req_nterror(req, status)) { > return; > } >@@ -674,11 +702,14 @@ verify_again: > */ > goto verify_again; > } >+ state->sent_server_id = global->server_id; > if (tevent_req_nterror(req, status)) { > return; > } > } > >+watch_again: >+ > /* > * If the record changed, but we are not happy with the change yet, > * we better remove ourself from the waiter list >-- >2.34.1 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=18038">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
asn
:
review+
Actions:
View
Attachments on
bug 15346
:
17846
|
17884
|
17885
|
17886
|
17890
|
17891
|
18037
| 18038 |
18039
|
18041