The Samba-Bugzilla – Attachment 18041 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]
Patches for v4-17-test (+ 1 additional backport)
bfixes-tmp417.txt (text/plain), 39.51 KB, created by
Stefan Metzmacher
on 2023-08-11 15:14:18 UTC
(
hide
)
Description:
Patches for v4-17-test (+ 1 additional backport)
Filename:
MIME Type:
Creator:
Stefan Metzmacher
Created:
2023-08-11 15:14:18 UTC
Size:
39.51 KB
patch
obsolete
>From 79d69f53de29440def5d169ea90e423380b2f5c1 Mon Sep 17 00:00:00 2001 >From: Volker Lendecke <vl@samba.org> >Date: Thu, 25 Aug 2022 09:54:52 +0200 >Subject: [PATCH 1/7] smbXcli: Pass negotiate contexts through > smbXcli_negprot_send/recv > >We already don't allow setting max_credits in the sync wrapper, so >omit the contexts there as well. > >Signed-off-by: Volker Lendecke <vl@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> > >Autobuild-User(master): Jeremy Allison <jra@samba.org> >Autobuild-Date(master): Fri Aug 26 19:54:03 UTC 2022 on sn-devel-184 > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15346 > >(cherry picked from commit 4ddd277c0b77c502ed6b11e07c92c91f24ac9c15) >--- > libcli/smb/smbXcli_base.c | 80 +++++++++++++++++---- > libcli/smb/smbXcli_base.h | 9 ++- > source3/libsmb/cliconnect.c | 16 +++-- > source3/torture/torture.c | 11 ++- > source4/libcli/raw/rawnegotiate.c | 5 +- > source4/libcli/smb2/connect.c | 5 +- > source4/libcli/smb_composite/connect_nego.c | 5 +- > 7 files changed, 101 insertions(+), 30 deletions(-) > >diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c >index c28850ee57de..7db0600dd48b 100644 >--- a/libcli/smb/smbXcli_base.c >+++ b/libcli/smb/smbXcli_base.c >@@ -4220,6 +4220,8 @@ static const struct { > struct smbXcli_negprot_state { > struct smbXcli_conn *conn; > struct tevent_context *ev; >+ struct smb2_negotiate_contexts *in_ctx; >+ struct smb2_negotiate_contexts *out_ctx; > uint32_t timeout_msec; > > struct { >@@ -4242,7 +4244,8 @@ struct tevent_req *smbXcli_negprot_send(TALLOC_CTX *mem_ctx, > uint32_t timeout_msec, > enum protocol_types min_protocol, > enum protocol_types max_protocol, >- uint16_t max_credits) >+ uint16_t max_credits, >+ struct smb2_negotiate_contexts *in_ctx) > { > struct tevent_req *req, *subreq; > struct smbXcli_negprot_state *state; >@@ -4254,6 +4257,7 @@ struct tevent_req *smbXcli_negprot_send(TALLOC_CTX *mem_ctx, > } > state->conn = conn; > state->ev = ev; >+ state->in_ctx = in_ctx; > state->timeout_msec = timeout_msec; > > if (min_protocol == PROTOCOL_NONE) { >@@ -4934,6 +4938,25 @@ static struct tevent_req *smbXcli_negprot_smb2_subreq(struct smbXcli_negprot_sta > return NULL; > } > >+ if (state->in_ctx != NULL) { >+ struct smb2_negotiate_contexts *ctxs = state->in_ctx; >+ >+ for (i=0; i<ctxs->num_contexts; i++) { >+ struct smb2_negotiate_context *ctx = >+ &ctxs->contexts[i]; >+ >+ status = smb2_negotiate_context_add( >+ state, >+ &c, >+ ctx->type, >+ ctx->data.data, >+ ctx->data.length); >+ if (!NT_STATUS_IS_OK(status)) { >+ return NULL; >+ } >+ } >+ } >+ > status = smb2_negotiate_context_push(state, &b, c); > if (!NT_STATUS_IS_OK(status)) { > return NULL; >@@ -4988,7 +5011,6 @@ static void smbXcli_negprot_smb2_done(struct tevent_req *subreq) > uint8_t *body; > size_t i; > uint16_t dialect_revision; >- struct smb2_negotiate_contexts c = { .num_contexts = 0, }; > uint32_t negotiate_context_offset = 0; > uint16_t negotiate_context_count = 0; > DATA_BLOB negotiate_context_blob = data_blob_null; >@@ -5195,10 +5217,15 @@ static void smbXcli_negprot_smb2_done(struct tevent_req *subreq) > negotiate_context_blob.data += ctx_ofs; > negotiate_context_blob.length -= ctx_ofs; > >- status = smb2_negotiate_context_parse(state, >+ state->out_ctx = talloc_zero(state, struct smb2_negotiate_contexts); >+ if (tevent_req_nomem(state->out_ctx, req)) { >+ return; >+ } >+ >+ status = smb2_negotiate_context_parse(state->out_ctx, > negotiate_context_blob, > negotiate_context_count, >- &c); >+ state->out_ctx); > if (NT_STATUS_EQUAL(status, NT_STATUS_INVALID_PARAMETER)) { > status = NT_STATUS_INVALID_NETWORK_RESPONSE; > } >@@ -5206,8 +5233,8 @@ static void smbXcli_negprot_smb2_done(struct tevent_req *subreq) > return; > } > >- preauth = smb2_negotiate_context_find(&c, >- SMB2_PREAUTH_INTEGRITY_CAPABILITIES); >+ preauth = smb2_negotiate_context_find( >+ state->out_ctx, SMB2_PREAUTH_INTEGRITY_CAPABILITIES); > if (preauth == NULL) { > tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE); > return; >@@ -5237,7 +5264,8 @@ static void smbXcli_negprot_smb2_done(struct tevent_req *subreq) > return; > } > >- sign_algo = smb2_negotiate_context_find(&c, SMB2_SIGNING_CAPABILITIES); >+ sign_algo = smb2_negotiate_context_find( >+ state->out_ctx, SMB2_SIGNING_CAPABILITIES); > if (sign_algo != NULL) { > const struct smb3_signing_capabilities *client_sign_algos = > &state->conn->smb2.client.smb3_capabilities.signing; >@@ -5296,7 +5324,8 @@ static void smbXcli_negprot_smb2_done(struct tevent_req *subreq) > conn->smb2.server.sign_algo = sign_algo_selected; > } > >- cipher = smb2_negotiate_context_find(&c, SMB2_ENCRYPTION_CAPABILITIES); >+ cipher = smb2_negotiate_context_find( >+ state->out_ctx, SMB2_ENCRYPTION_CAPABILITIES); > if (cipher != NULL) { > const struct smb3_encryption_capabilities *client_ciphers = > &state->conn->smb2.client.smb3_capabilities.encryption; >@@ -5516,9 +5545,26 @@ static NTSTATUS smbXcli_negprot_dispatch_incoming(struct smbXcli_conn *conn, > return NT_STATUS_INVALID_NETWORK_RESPONSE; > } > >-NTSTATUS smbXcli_negprot_recv(struct tevent_req *req) >+NTSTATUS smbXcli_negprot_recv( >+ struct tevent_req *req, >+ TALLOC_CTX *mem_ctx, >+ struct smb2_negotiate_contexts **out_ctx) > { >- return tevent_req_simple_recv_ntstatus(req); >+ struct smbXcli_negprot_state *state = tevent_req_data( >+ req, struct smbXcli_negprot_state); >+ NTSTATUS status; >+ >+ if (tevent_req_is_nterror(req, &status)) { >+ tevent_req_received(req); >+ return status; >+ } >+ >+ if (out_ctx != NULL) { >+ *out_ctx = talloc_move(mem_ctx, &state->out_ctx); >+ } >+ >+ tevent_req_received(req); >+ return NT_STATUS_OK; > } > > NTSTATUS smbXcli_negprot(struct smbXcli_conn *conn, >@@ -5543,9 +5589,15 @@ NTSTATUS smbXcli_negprot(struct smbXcli_conn *conn, > if (ev == NULL) { > goto fail; > } >- req = smbXcli_negprot_send(frame, ev, conn, timeout_msec, >- min_protocol, max_protocol, >- WINDOWS_CLIENT_PURE_SMB2_NEGPROT_INITIAL_CREDIT_ASK); >+ req = smbXcli_negprot_send( >+ frame, >+ ev, >+ conn, >+ timeout_msec, >+ min_protocol, >+ max_protocol, >+ WINDOWS_CLIENT_PURE_SMB2_NEGPROT_INITIAL_CREDIT_ASK, >+ NULL); > if (req == NULL) { > goto fail; > } >@@ -5553,7 +5605,7 @@ NTSTATUS smbXcli_negprot(struct smbXcli_conn *conn, > if (!ok) { > goto fail; > } >- status = smbXcli_negprot_recv(req); >+ status = smbXcli_negprot_recv(req, NULL, NULL); > fail: > TALLOC_FREE(frame); > return status; >diff --git a/libcli/smb/smbXcli_base.h b/libcli/smb/smbXcli_base.h >index 805a62ce3422..8e4fb81818fc 100644 >--- a/libcli/smb/smbXcli_base.h >+++ b/libcli/smb/smbXcli_base.h >@@ -457,14 +457,19 @@ NTSTATUS smb2cli_req_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, > NTSTATUS smb2cli_req_get_sent_iov(struct tevent_req *req, > struct iovec *sent_iov); > >+struct smb2_negotiate_contexts; > struct tevent_req *smbXcli_negprot_send(TALLOC_CTX *mem_ctx, > struct tevent_context *ev, > struct smbXcli_conn *conn, > uint32_t timeout_msec, > enum protocol_types min_protocol, > enum protocol_types max_protocol, >- uint16_t max_credits); >-NTSTATUS smbXcli_negprot_recv(struct tevent_req *req); >+ uint16_t max_credits, >+ struct smb2_negotiate_contexts *in_ctx); >+NTSTATUS smbXcli_negprot_recv( >+ struct tevent_req *req, >+ TALLOC_CTX *mem_ctx, >+ struct smb2_negotiate_contexts **out_ctx); > NTSTATUS smbXcli_negprot(struct smbXcli_conn *conn, > uint32_t timeout_msec, > enum protocol_types min_protocol, >diff --git a/source3/libsmb/cliconnect.c b/source3/libsmb/cliconnect.c >index d5b48d347552..d2d048026e74 100644 >--- a/source3/libsmb/cliconnect.c >+++ b/source3/libsmb/cliconnect.c >@@ -2850,11 +2850,15 @@ static void cli_start_connection_connected(struct tevent_req *subreq) > return; > } > >- subreq = smbXcli_negprot_send(state, state->ev, state->cli->conn, >- state->cli->timeout, >- state->min_protocol, >- state->max_protocol, >- WINDOWS_CLIENT_PURE_SMB2_NEGPROT_INITIAL_CREDIT_ASK); >+ subreq = smbXcli_negprot_send( >+ state, >+ state->ev, >+ state->cli->conn, >+ state->cli->timeout, >+ state->min_protocol, >+ state->max_protocol, >+ WINDOWS_CLIENT_PURE_SMB2_NEGPROT_INITIAL_CREDIT_ASK, >+ NULL); > if (tevent_req_nomem(subreq, req)) { > return; > } >@@ -2869,7 +2873,7 @@ static void cli_start_connection_done(struct tevent_req *subreq) > req, struct cli_start_connection_state); > NTSTATUS status; > >- status = smbXcli_negprot_recv(subreq); >+ status = smbXcli_negprot_recv(subreq, NULL, NULL); > TALLOC_FREE(subreq); > if (tevent_req_nterror(req, status)) { > return; >diff --git a/source3/torture/torture.c b/source3/torture/torture.c >index cd32156ae427..fc139a4b808a 100644 >--- a/source3/torture/torture.c >+++ b/source3/torture/torture.c >@@ -3953,8 +3953,15 @@ static bool run_negprot_nowait(int dummy) > for (i=0;i<50000;i++) { > struct tevent_req *req; > >- req = smbXcli_negprot_send(ev, ev, cli->conn, cli->timeout, >- PROTOCOL_CORE, PROTOCOL_NT1, 0); >+ req = smbXcli_negprot_send( >+ ev, >+ ev, >+ cli->conn, >+ cli->timeout, >+ PROTOCOL_CORE, >+ PROTOCOL_NT1, >+ 0, >+ NULL); > if (req == NULL) { > TALLOC_FREE(ev); > return false; >diff --git a/source4/libcli/raw/rawnegotiate.c b/source4/libcli/raw/rawnegotiate.c >index 51c6f0f9ecbe..6d1b73619329 100644 >--- a/source4/libcli/raw/rawnegotiate.c >+++ b/source4/libcli/raw/rawnegotiate.c >@@ -106,7 +106,8 @@ struct tevent_req *smb_raw_negotiate_send(TALLOC_CTX *mem_ctx, > timeout_msec, > minprotocol, > maxprotocol, >- transport->options.max_credits); >+ transport->options.max_credits, >+ NULL); > if (tevent_req_nomem(subreq, req)) { > return tevent_req_post(req, ev); > } >@@ -125,7 +126,7 @@ static void smb_raw_negotiate_done(struct tevent_req *subreq) > struct smb_raw_negotiate_state); > NTSTATUS status; > >- status = smbXcli_negprot_recv(subreq); >+ status = smbXcli_negprot_recv(subreq, NULL, NULL); > TALLOC_FREE(subreq); > if (tevent_req_nterror(req, status)) { > return; >diff --git a/source4/libcli/smb2/connect.c b/source4/libcli/smb2/connect.c >index 9540704491ea..1f68d90538b2 100644 >--- a/source4/libcli/smb2/connect.c >+++ b/source4/libcli/smb2/connect.c >@@ -187,7 +187,8 @@ static void smb2_connect_socket_done(struct composite_context *creq) > state->transport->conn, timeout_msec, > min_protocol, > state->transport->options.max_protocol, >- state->transport->options.max_credits); >+ state->transport->options.max_credits, >+ NULL); > if (tevent_req_nomem(subreq, req)) { > return; > } >@@ -203,7 +204,7 @@ static void smb2_connect_negprot_done(struct tevent_req *subreq) > struct tevent_req); > NTSTATUS status; > >- status = smbXcli_negprot_recv(subreq); >+ status = smbXcli_negprot_recv(subreq, NULL, NULL); > TALLOC_FREE(subreq); > if (tevent_req_nterror(req, status)) { > return; >diff --git a/source4/libcli/smb_composite/connect_nego.c b/source4/libcli/smb_composite/connect_nego.c >index 3bd5dbc59e8e..7224dfa87946 100644 >--- a/source4/libcli/smb_composite/connect_nego.c >+++ b/source4/libcli/smb_composite/connect_nego.c >@@ -167,7 +167,8 @@ static void smb_connect_nego_connect_done(struct composite_context *creq) > timeout_msec, > state->options.min_protocol, > state->options.max_protocol, >- state->options.max_credits); >+ state->options.max_credits, >+ NULL); > if (tevent_req_nomem(subreq, req)) { > return; > } >@@ -181,7 +182,7 @@ static void smb_connect_nego_nego_done(struct tevent_req *subreq) > struct tevent_req); > NTSTATUS status; > >- status = smbXcli_negprot_recv(subreq); >+ status = smbXcli_negprot_recv(subreq, NULL, NULL); > TALLOC_FREE(subreq); > if (tevent_req_nterror(req, status)) { > return; >-- >2.34.1 > > >From ab9107cf83377b88683c8ece776622477f645fa2 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/7] 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 8528dc2423d6ab1c9716100849cd777c422aa8e0 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 7 Aug 2023 11:03:41 +0200 >Subject: [PATCH 3/7] 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 fdaa169d78f469578c8a082d8a726e19717ab76f Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 7 Aug 2023 12:22:43 +0200 >Subject: [PATCH 4/7] 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 68beb2e190940f1c65d32ef72361ea6bf9bb30eb Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Fri, 4 Aug 2023 17:16:14 +0200 >Subject: [PATCH 5/7] 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 24c736d6701a..7c3a60c00162 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" >@@ -2345,6 +2346,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"); >@@ -2354,10 +2664,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); >@@ -2379,6 +2692,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 ac73fbbc6d8e82a26c13b36c5470005157c43391 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 3 Aug 2023 15:34:29 +0200 >Subject: [PATCH 6/7] 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 f57bc724910d..4d3fe30f812b 100644 >--- a/source3/smbd/smbXsrv_client.c >+++ b/source3/smbd/smbXsrv_client.c >@@ -554,7 +554,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, >@@ -635,6 +634,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 9ffe950dc76aee455d42981733769dacf2166246 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 3 Aug 2023 15:45:45 +0200 >Subject: [PATCH 7/7] 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 4d3fe30f812b..90322c5e60ab 100644 >--- a/source3/smbd/smbXsrv_client.c >+++ b/source3/smbd/smbXsrv_client.c >@@ -487,6 +487,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; >@@ -529,6 +530,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)) { >@@ -624,6 +627,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() >@@ -659,6 +686,7 @@ verify_again: > */ > goto verify_again; > } >+ state->sent_server_id = global->server_id; > if (tevent_req_nterror(req, status)) { > return; > } >@@ -673,11 +701,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=18041">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