The Samba-Bugzilla – Attachment 17569 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-16-test (require patches from bug #15159)
bfixes-tmp416.txt (text/plain), 11.97 KB, created by
Stefan Metzmacher
on 2022-10-18 07:41:08 UTC
(
hide
)
Description:
Patches for v4-16-test (require patches from bug #15159)
Filename:
MIME Type:
Creator:
Stefan Metzmacher
Created:
2022-10-18 07:41:08 UTC
Size:
11.97 KB
patch
obsolete
>From c34eed537e5cf77c88f281adeaec3fb68999e592 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 1ee4410c1cf5..c948b50d75b3 100644 >--- a/source3/smbd/smbXsrv_client.c >+++ b/source3/smbd/smbXsrv_client.c >@@ -1063,6 +1063,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 5d071b2e295720f9126708bddbcc881da3a72a6b 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 c948b50d75b3..648ca84e8c79 100644 >--- a/source3/smbd/smbXsrv_client.c >+++ b/source3/smbd/smbXsrv_client.c >@@ -227,8 +227,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 f5cdb16261298c2082b04d678616e38999376dbe 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> >(similar to commit 56c597bc2b29dc3e555f737ba189f521d0e31e8c) >--- > source3/smbd/smbXsrv_client.c | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) > >diff --git a/source3/smbd/smbXsrv_client.c b/source3/smbd/smbXsrv_client.c >index 648ca84e8c79..c671c99b6711 100644 >--- a/source3/smbd/smbXsrv_client.c >+++ b/source3/smbd/smbXsrv_client.c >@@ -592,31 +592,30 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req) > tevent_req_set_callback(subreq, smb2srv_client_mc_negprot_done, req); > } > >- subreq = dbwrap_watched_watch_send(state, >- state->ev, >- state->db_rec, >- global->server_id); >- if (tevent_req_nomem(subreq, req)) { >- return; >- } >- 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; > } > } > >+ subreq = dbwrap_watched_watch_send(state, >+ state->ev, >+ state->db_rec, >+ global->server_id); >+ if (tevent_req_nomem(subreq, req)) { >+ return; >+ } >+ tevent_req_set_callback(subreq, smb2srv_client_mc_negprot_watched, req); >+ >+ TALLOC_FREE(global); > TALLOC_FREE(state->db_rec); > return; > } >-- >2.34.1 > > >From 51cdbb8900cd80e48dfab0d987f9a584ca23a928 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 c671c99b6711..08dfd8668566 100644 >--- a/source3/smbd/smbXsrv_client.c >+++ b/source3/smbd/smbXsrv_client.c >@@ -462,6 +462,7 @@ struct smb2srv_client_mc_negprot_state { > struct tevent_context *ev; > struct smbd_smb2_request *smb2req; > struct db_record *db_rec; >+ struct tevent_req *filter_subreq; > }; > > static void smb2srv_client_mc_negprot_cleanup(struct tevent_req *req, >@@ -519,6 +520,7 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req) > struct tevent_req *subreq = NULL; > NTSTATUS status; > >+ 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, >@@ -590,6 +592,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)) { >@@ -650,6 +653,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 64f1454ce8d714bfced723f3c84b7176bd9e8f27 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 08dfd8668566..28e5035d21dc 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) > { > TDB_DATA key; >@@ -197,6 +198,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(); > >@@ -250,6 +252,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; >@@ -519,6 +537,7 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req) > bool is_free = false; > struct tevent_req *subreq = NULL; > NTSTATUS status; >+ struct server_id last_server_id = { .pid = 0, }; > > TALLOC_FREE(state->filter_subreq); > SMB_ASSERT(state->db_rec == NULL); >@@ -530,10 +549,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); > if (is_free) { > /* >@@ -582,6 +605,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, >@@ -598,12 +631,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