The Samba-Bugzilla – Attachment 17568 Details for
Bug 15201
memory leak on temporary of struct imessaging_post_state and struct tevent_immediate on struct imessaging_context (in rpcd_spoolss and maybe others)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
Patches for v4-17-test
bfixes-tmp417.txt (text/plain), 12.10 KB, created by
Stefan Metzmacher
on 2022-10-18 07:36:29 UTC
(
hide
)
Description:
Patches for v4-17-test
Filename:
MIME Type:
Creator:
Stefan Metzmacher
Created:
2022-10-18 07:36:29 UTC
Size:
12.10 KB
patch
obsolete
>From 3e8ca9b7a6b5c729f9ebb77de3b69676791705c0 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Wed, 12 Oct 2022 13:30:32 +0200 >Subject: [PATCH 1/5] smbXsrv_client: ignore NAME_NOT_FOUND from > smb2srv_client_connection_passed > >If we hit a race, when a client disconnects the connection after the initial >SMB2 Negotiate request, before the connection is completely passed to >process serving the given client guid, the temporary smbd which accepted the >new connection may already detected the disconnect and exitted before >the long term smbd servicing the client guid was able to send the >MSG_SMBXSRV_CONNECTION_PASSED message. > >The result was a log message like this: > > smbXsrv_client_connection_pass_loop: smb2srv_client_connection_passed() failed => NT_STATUS_OBJECT_NAME_NOT_FOUND > >and all connections belonging to the client guid were dropped, >because we called exit_server_cleanly(). > >Now we ignore NT_STATUS_OBJECT_NAME_NOT_FOUND from >smb2srv_client_connection_passed() and let the normal >event loop detect the broken connection, so that only >that connection is terminated (not the whole smbd process). > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15200 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 636ec45c93ad040ba70296aa543884c145b3e789) >--- > source3/smbd/smbXsrv_client.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > >diff --git a/source3/smbd/smbXsrv_client.c b/source3/smbd/smbXsrv_client.c >index d7a6fa35bf07..6b9887cdfb11 100644 >--- a/source3/smbd/smbXsrv_client.c >+++ b/source3/smbd/smbXsrv_client.c >@@ -1111,6 +1111,16 @@ static void smbXsrv_client_connection_pass_loop(struct tevent_req *subreq) > } > > status = smb2srv_client_connection_passed(client, pass_info0); >+ if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { >+ /* >+ * We hit a race where, the client dropped the connection >+ * while the socket was passed to us and the origin >+ * process already existed. >+ */ >+ DBG_DEBUG("smb2srv_client_connection_passed() ignore %s\n", >+ nt_errstr(status)); >+ status = NT_STATUS_OK; >+ } > if (!NT_STATUS_IS_OK(status)) { > const char *r = "smb2srv_client_connection_passed() failed"; > DBG_ERR("%s => %s\n", r, nt_errstr(status)); >-- >2.34.1 > > >From 8615f492226fe68a8b2228ab3120bf4e6562bfb7 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Wed, 12 Oct 2022 13:40:26 +0200 >Subject: [PATCH 2/5] smbXsrv_client: fix a debug message in > smbXsrv_client_global_verify_record() > >DBG_WARNING() already adds the function name as prefix. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15200 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit acb3d821deaf06faa16f6428682ecdb02babeb98) >--- > source3/smbd/smbXsrv_client.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > >diff --git a/source3/smbd/smbXsrv_client.c b/source3/smbd/smbXsrv_client.c >index 6b9887cdfb11..1cff66a2e3a7 100644 >--- a/source3/smbd/smbXsrv_client.c >+++ b/source3/smbd/smbXsrv_client.c >@@ -231,8 +231,7 @@ static void smbXsrv_client_global_verify_record(struct db_record *db_rec, > (ndr_pull_flags_fn_t)ndr_pull_smbXsrv_client_globalB); > if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { > NTSTATUS status = ndr_map_error2ntstatus(ndr_err); >- DBG_WARNING("smbXsrv_client_global_verify_record: " >- "key '%s' ndr_pull_struct_blob - %s\n", >+ DBG_WARNING("key '%s' ndr_pull_struct_blob - %s\n", > hex_encode_talloc(frame, key.dptr, key.dsize), > nt_errstr(status)); > TALLOC_FREE(frame); >-- >2.34.1 > > >From da5a3b8274552bae9e818f50eb44c769e69261c1 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Wed, 12 Oct 2022 13:54:41 +0200 >Subject: [PATCH 3/5] smbXsrv_client: call > smb2srv_client_connection_{pass,drop}() before dbwrap_watched_watch_send() > >dbwrap_watched_watch_send() should typically be the last thing to call >before the db record is unlocked, as it's not that easy to undo. > >In future we want to recover from smb2srv_client_connection_{pass,drop}() >returning NT_STATUS_OBJECT_NAME_NOT_FOUND and it would add complexity if >would need to undo dbwrap_watched_watch_send() at that point. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15200 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 56c597bc2b29dc3e555f737ba189f521d0e31e8c) >--- > source3/smbd/smbXsrv_client.c | 31 +++++++++++++++---------------- > 1 file changed, 15 insertions(+), 16 deletions(-) > >diff --git a/source3/smbd/smbXsrv_client.c b/source3/smbd/smbXsrv_client.c >index 1cff66a2e3a7..f0d34bbf4940 100644 >--- a/source3/smbd/smbXsrv_client.c >+++ b/source3/smbd/smbXsrv_client.c >@@ -612,6 +612,20 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req) > tevent_req_set_callback(subreq, smb2srv_client_mc_negprot_done, req); > } > >+ if (procid_is_local(&global->server_id)) { >+ status = smb2srv_client_connection_pass(state->smb2req, >+ global); >+ if (tevent_req_nterror(req, status)) { >+ return; >+ } >+ } else { >+ status = smb2srv_client_connection_drop(state->smb2req, >+ global); >+ if (tevent_req_nterror(req, status)) { >+ return; >+ } >+ } >+ > /* > * If the record changed, but we are not happy with the change yet, > * we better remove ourself from the waiter list >@@ -643,22 +657,7 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req) > } > tevent_req_set_callback(subreq, smb2srv_client_mc_negprot_watched, req); > >- if (procid_is_local(&global->server_id)) { >- status = smb2srv_client_connection_pass(state->smb2req, >- global); >- TALLOC_FREE(global); >- if (tevent_req_nterror(req, status)) { >- return; >- } >- } else { >- status = smb2srv_client_connection_drop(state->smb2req, >- global); >- TALLOC_FREE(global); >- if (tevent_req_nterror(req, status)) { >- return; >- } >- } >- >+ TALLOC_FREE(global); > TALLOC_FREE(state->db_rec); > return; > } >-- >2.34.1 > > >From 853a9fff4e2c2f29e96acec60d97b9bcd72d1a3b Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Wed, 12 Oct 2022 14:15:53 +0200 >Subject: [PATCH 4/5] smbXsrv_client: make sure we only wait for > smb2srv_client_mc_negprot_filter once and only when needed > >This will simplify the following changes... > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15200 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 8c8d8cf01e01c2726d03fa1c81e0ce9992ee736c) >--- > source3/smbd/smbXsrv_client.c | 6 ++++++ > 1 file changed, 6 insertions(+) > >diff --git a/source3/smbd/smbXsrv_client.c b/source3/smbd/smbXsrv_client.c >index f0d34bbf4940..0a958842d341 100644 >--- a/source3/smbd/smbXsrv_client.c >+++ b/source3/smbd/smbXsrv_client.c >@@ -471,6 +471,7 @@ struct smb2srv_client_mc_negprot_state { > struct db_record *db_rec; > uint64_t watch_instance; > uint32_t last_seqnum; >+ struct tevent_req *filter_subreq; > }; > > static void smb2srv_client_mc_negprot_cleanup(struct tevent_req *req, >@@ -534,6 +535,7 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req) > NTSTATUS status; > uint32_t seqnum = 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, >@@ -610,6 +612,7 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req) > return; > } > tevent_req_set_callback(subreq, smb2srv_client_mc_negprot_done, req); >+ state->filter_subreq = subreq; > } > > if (procid_is_local(&global->server_id)) { >@@ -692,6 +695,9 @@ static void smb2srv_client_mc_negprot_done(struct tevent_req *subreq) > NTSTATUS status; > int ret; > >+ SMB_ASSERT(state->filter_subreq == subreq); >+ state->filter_subreq = NULL; >+ > ret = messaging_filtered_read_recv(subreq, state, &rec); > TALLOC_FREE(subreq); > if (ret != 0) { >-- >2.34.1 > > >From 039f1067fc55d02cb7eba7eae35b945c56a09c8f Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Wed, 12 Oct 2022 14:57:18 +0200 >Subject: [PATCH 5/5] smbXsrv_client: handle NAME_NOT_FOUND from > smb2srv_client_connection_{pass,drop}() > >If we get NT_STATUS_OBJECT_NOT_FOUND from smb2srv_client_connection_{pass,drop}() >we should just keep the connection and overwrite the stale record in >smbXsrv_client_global.tdb. It's basically a race with serverid_exists() >and a process that doesn't cleanly teardown. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15200 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 5d66d5b84f87267243dcd5223210906ce589af91) >--- > source3/smbd/smbXsrv_client.c | 49 +++++++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > >diff --git a/source3/smbd/smbXsrv_client.c b/source3/smbd/smbXsrv_client.c >index 0a958842d341..f57bc724910d 100644 >--- a/source3/smbd/smbXsrv_client.c >+++ b/source3/smbd/smbXsrv_client.c >@@ -189,6 +189,7 @@ static void smbXsrv_client_global_verify_record(struct db_record *db_rec, > bool *is_free, > bool *was_free, > TALLOC_CTX *mem_ctx, >+ const struct server_id *dead_server_id, > struct smbXsrv_client_global0 **_g, > uint32_t *pseqnum) > { >@@ -198,6 +199,7 @@ static void smbXsrv_client_global_verify_record(struct db_record *db_rec, > struct smbXsrv_client_globalB global_blob; > enum ndr_err_code ndr_err; > struct smbXsrv_client_global0 *global = NULL; >+ bool dead = false; > bool exists; > TALLOC_CTX *frame = talloc_stackframe(); > >@@ -254,6 +256,22 @@ static void smbXsrv_client_global_verify_record(struct db_record *db_rec, > > global = global_blob.info.info0; > >+ dead = server_id_equal(dead_server_id, &global->server_id); >+ if (dead) { >+ struct server_id_buf tmp; >+ >+ DBG_NOTICE("key '%s' server_id %s is already dead.\n", >+ hex_encode_talloc(frame, key.dptr, key.dsize), >+ server_id_str_buf(global->server_id, &tmp)); >+ if (DEBUGLVL(DBGLVL_NOTICE)) { >+ NDR_PRINT_DEBUG(smbXsrv_client_globalB, &global_blob); >+ } >+ TALLOC_FREE(frame); >+ dbwrap_record_delete(db_rec); >+ *is_free = true; >+ return; >+ } >+ > exists = serverid_exists(&global->server_id); > if (!exists) { > struct server_id_buf tmp; >@@ -534,6 +552,7 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req) > struct tevent_req *subreq = NULL; > NTSTATUS status; > uint32_t seqnum = 0; >+ struct server_id last_server_id = { .pid = 0, }; > > TALLOC_FREE(state->filter_subreq); > SMB_ASSERT(state->db_rec == NULL); >@@ -545,10 +564,14 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req) > return; > } > >+verify_again: >+ TALLOC_FREE(global); >+ > smbXsrv_client_global_verify_record(state->db_rec, > &is_free, > NULL, > state, >+ &last_server_id, > &global, > &seqnum); > if (is_free) { >@@ -602,6 +625,16 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req) > return; > } > >+ /* >+ * If last_server_id is set, we expect >+ * smbXsrv_client_global_verify_record() >+ * to detect the already dead global->server_id >+ * as state->db_rec is still locked and its >+ * value didn't change. >+ */ >+ SMB_ASSERT(last_server_id.pid == 0); >+ last_server_id = global->server_id; >+ > if (procid_is_local(&global->server_id)) { > subreq = messaging_filtered_read_send(state, > state->ev, >@@ -618,12 +651,28 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req) > if (procid_is_local(&global->server_id)) { > status = smb2srv_client_connection_pass(state->smb2req, > global); >+ if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { >+ /* >+ * We remembered last_server_id = global->server_id >+ * above, so we'll treat it as dead in the >+ * next round to smbXsrv_client_global_verify_record(). >+ */ >+ goto verify_again; >+ } > if (tevent_req_nterror(req, status)) { > return; > } > } else { > status = smb2srv_client_connection_drop(state->smb2req, > global); >+ if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { >+ /* >+ * We remembered last_server_id = global->server_id >+ * above, so we'll treat it as dead in the >+ * next round to smbXsrv_client_global_verify_record(). >+ */ >+ goto verify_again; >+ } > if (tevent_req_nterror(req, status)) { > return; > } >-- >2.34.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Actions:
View
Attachments on
bug 15201
:
17568
|
17569
|
17570
|
17571