The Samba-Bugzilla – Attachment 18459 Details for
Bug 15624
DH reconnect error handling can lead to stale sharemode entries
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
Patches for v4-19-test
bfixes-tmp419.txt (text/plain), 45.61 KB, created by
Stefan Metzmacher
on 2024-10-02 13:48:53 UTC
(
hide
)
Description:
Patches for v4-19-test
Filename:
MIME Type:
Creator:
Stefan Metzmacher
Created:
2024-10-02 13:48:53 UTC
Size:
45.61 KB
patch
obsolete
>From 32b70009804a4343dd04a9f8ada5161ab02937bf Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 25 Apr 2024 15:24:57 +0200 >Subject: [PATCH 01/12] s3/lib: add next helper variable in server_id_watch_* > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624 > >Pair-Programmed-With: Stefan Metzmacher <metze@samba.org> > >Signed-off-by: Ralph Boehme <slow@samba.org> >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Guenther Deschner <gd@samba.org> >(cherry picked from commit d76edcd48437715c7541b5b1e6a56245c25f460b) >--- > source3/lib/server_id_watch.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > >diff --git a/source3/lib/server_id_watch.c b/source3/lib/server_id_watch.c >index f0189e0e896a..50b35f27b3e2 100644 >--- a/source3/lib/server_id_watch.c >+++ b/source3/lib/server_id_watch.c >@@ -27,6 +27,7 @@ > struct server_id_watch_state { > struct tevent_context *ev; > struct server_id pid; >+ struct timeval start; > }; > > static void server_id_watch_waited(struct tevent_req *subreq); >@@ -37,6 +38,7 @@ struct tevent_req *server_id_watch_send(TALLOC_CTX *mem_ctx, > { > struct tevent_req *req, *subreq; > struct server_id_watch_state *state; >+ struct timeval next; > > req = tevent_req_create(mem_ctx, &state, struct server_id_watch_state); > if (req == NULL) { >@@ -44,14 +46,15 @@ struct tevent_req *server_id_watch_send(TALLOC_CTX *mem_ctx, > } > state->ev = ev; > state->pid = pid; >+ state->start = tevent_timeval_current(); > > if (!serverid_exists(&state->pid)) { > tevent_req_done(req); > return tevent_req_post(req, ev); > } > >- subreq = tevent_wakeup_send( >- state, ev, tevent_timeval_current_ofs(0, 500000)); >+ next = tevent_timeval_add(&state->start, 0, 500000); >+ subreq = tevent_wakeup_send(state, ev, next); > if (tevent_req_nomem(subreq, req)) { > return tevent_req_post(req, ev); > } >@@ -66,6 +69,8 @@ static void server_id_watch_waited(struct tevent_req *subreq) > subreq, struct tevent_req); > struct server_id_watch_state *state = tevent_req_data( > req, struct server_id_watch_state); >+ struct timeval now; >+ struct timeval next; > bool ok; > > ok = tevent_wakeup_recv(subreq); >@@ -80,8 +85,9 @@ static void server_id_watch_waited(struct tevent_req *subreq) > return; > } > >- subreq = tevent_wakeup_send( >- state, state->ev, tevent_timeval_current_ofs(0, 500000)); >+ now = tevent_timeval_current(); >+ next = tevent_timeval_add(&now, 0, 500000); >+ subreq = tevent_wakeup_send(state, state->ev, next); > if (tevent_req_nomem(subreq, req)) { > return; > } >-- >2.34.1 > > >From 00e75c123ee53fe20bf15c81352ed58c98923a75 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 4 Apr 2024 12:31:05 +0200 >Subject: [PATCH 02/12] s3/lib: add option "serverid watch:debug = yes" to > print kernel stack of hanging process > >We only do if sys_have_proc_fds() returns true, so it's most likely >linux... > >Enabled by default with log level 10... > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624 > >Pair-Programmed-With: Stefan Metzmacher <metze@samba.org> > >Signed-off-by: Ralph Boehme <slow@samba.org> >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Guenther Deschner <gd@samba.org> >(cherry picked from commit 5c57e840527432c4b1a7ec94894939022a9e9622) >--- > source3/lib/server_id_watch.c | 62 +++++++++++++++++++++++++++++++++-- > 1 file changed, 59 insertions(+), 3 deletions(-) > >diff --git a/source3/lib/server_id_watch.c b/source3/lib/server_id_watch.c >index 50b35f27b3e2..c372ec8c4317 100644 >--- a/source3/lib/server_id_watch.c >+++ b/source3/lib/server_id_watch.c >@@ -17,17 +17,18 @@ > * along with this program. If not, see <http://www.gnu.org/licenses/>. > */ > >-#include "replace.h" >-#include <tevent.h> >-#include <talloc.h> >+#include "includes.h" > #include "serverid.h" > #include "server_id_watch.h" >+#include "lib/util/server_id.h" > #include "lib/util/tevent_unix.h" > > struct server_id_watch_state { > struct tevent_context *ev; > struct server_id pid; > struct timeval start; >+ struct timeval warn; >+ bool debug; > }; > > static void server_id_watch_waited(struct tevent_req *subreq); >@@ -47,6 +48,12 @@ struct tevent_req *server_id_watch_send(TALLOC_CTX *mem_ctx, > state->ev = ev; > state->pid = pid; > state->start = tevent_timeval_current(); >+ state->warn = tevent_timeval_add(&state->start, 10, 0); >+ >+ state->debug = lp_parm_bool(GLOBAL_SECTION_SNUM, >+ "serverid watch", >+ "debug", >+ CHECK_DEBUGLVL(DBGLVL_DEBUG)); > > if (!serverid_exists(&state->pid)) { > tevent_req_done(req); >@@ -86,6 +93,55 @@ static void server_id_watch_waited(struct tevent_req *subreq) > } > > now = tevent_timeval_current(); >+ >+ if (!state->debug) { >+ goto next; >+ } >+ >+ if (timeval_compare(&state->warn, &now) == -1) { >+ double duration = timeval_elapsed2(&state->start, &now); >+ char proc_path[64] = { 0, }; >+ char *kstack = NULL; >+ struct server_id_buf buf; >+ const char *pid = server_id_str_buf(state->pid, &buf); >+ int ret; >+ >+ state->warn = tevent_timeval_add(&now, 10, 0); >+ >+ if (!procid_is_local(&state->pid) || !sys_have_proc_fds()) { >+ DBG_ERR("Process %s hanging for %f seconds?\n", >+ pid, duration); >+ goto next; >+ } >+ >+ ret = snprintf(proc_path, >+ ARRAY_SIZE(proc_path), >+ "/proc/%" PRIu64 "/stack", >+ state->pid.pid); >+ if (ret < 0) { >+ DBG_ERR("Process %s hanging for %f seconds?\n" >+ "snprintf failed\n", >+ pid, duration); >+ goto next; >+ } >+ >+ become_root(); >+ kstack = file_load(proc_path, NULL, 0, state); >+ unbecome_root(); >+ if (kstack == NULL) { >+ DBG_ERR("Process %s hanging for %f seconds?\n" >+ "file_load [%s] failed\n", >+ pid, duration, proc_path); >+ goto next; >+ } >+ >+ DBG_ERR("Process %s hanging for %f seconds?\n" >+ "%s:\n%s", >+ pid, duration, proc_path, kstack); >+ TALLOC_FREE(kstack); >+ } >+ >+next: > next = tevent_timeval_add(&now, 0, 500000); > subreq = tevent_wakeup_send(state, state->ev, next); > if (tevent_req_nomem(subreq, req)) { >-- >2.34.1 > > >From a199dbd6b9681d1e883e99298f9d7e4ef6c310b4 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 25 Apr 2024 15:17:08 +0200 >Subject: [PATCH 03/12] s3/lib: add option "serverid watch:debug script" > >This takes just PID and NODE:PID on a cluster. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624 > >Pair-Programmed-With: Stefan Metzmacher <metze@samba.org> > >Signed-off-by: Ralph Boehme <slow@samba.org> >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Guenther Deschner <gd@samba.org> >(cherry picked from commit 7add7dbf1aee13b4d9ab70d1a5312c8ff30d9e00) >--- > source3/lib/server_id_watch.c | 52 +++++++++++++++++++++++++++++++++++ > 1 file changed, 52 insertions(+) > >diff --git a/source3/lib/server_id_watch.c b/source3/lib/server_id_watch.c >index c372ec8c4317..8ddf9c6b1c89 100644 >--- a/source3/lib/server_id_watch.c >+++ b/source3/lib/server_id_watch.c >@@ -100,6 +100,7 @@ static void server_id_watch_waited(struct tevent_req *subreq) > > if (timeval_compare(&state->warn, &now) == -1) { > double duration = timeval_elapsed2(&state->start, &now); >+ const char *cmd = NULL; > char proc_path[64] = { 0, }; > char *kstack = NULL; > struct server_id_buf buf; >@@ -108,6 +109,57 @@ static void server_id_watch_waited(struct tevent_req *subreq) > > state->warn = tevent_timeval_add(&now, 10, 0); > >+ cmd = lp_parm_const_string(GLOBAL_SECTION_SNUM, >+ "serverid watch", >+ "debug script", >+ NULL); >+ if (cmd != NULL) { >+ char *cmdstr = NULL; >+ char *output = NULL; >+ int fd; >+ >+ /* >+ * Note in a cluster setup pid will be >+ * a NOTE:PID like '1:3978365' >+ * >+ * Without clustering it is just '3978365' >+ */ >+ cmdstr = talloc_asprintf(state, "%s %s", cmd, pid); >+ if (cmdstr == NULL) { >+ DBG_ERR("Process %s hanging for %f seconds?\n" >+ "talloc_asprintf failed\n", >+ pid, duration); >+ goto next; >+ } >+ >+ become_root(); >+ ret = smbrun(cmdstr, &fd, NULL); >+ unbecome_root(); >+ if (ret != 0) { >+ DBG_ERR("Process %s hanging for %f seconds?\n" >+ "smbrun('%s') failed\n", >+ pid, duration, cmdstr); >+ TALLOC_FREE(cmdstr); >+ goto next; >+ } >+ >+ output = fd_load(fd, NULL, 0, state); >+ close(fd); >+ if (output == NULL) { >+ DBG_ERR("Process %s hanging for %f seconds?\n" >+ "fd_load() of smbrun('%s') failed\n", >+ pid, duration, cmdstr); >+ TALLOC_FREE(cmdstr); >+ goto next; >+ } >+ DBG_ERR("Process %s hanging for %f seconds?\n" >+ "%s returned:\n%s", >+ pid, duration, cmdstr, output); >+ TALLOC_FREE(cmdstr); >+ TALLOC_FREE(output); >+ goto next; >+ } >+ > if (!procid_is_local(&state->pid) || !sys_have_proc_fds()) { > DBG_ERR("Process %s hanging for %f seconds?\n", > pid, duration); >-- >2.34.1 > > >From 971dca69d67aca58ca7a4e2e84a0b53e3acd7c91 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Fri, 5 Apr 2024 12:15:28 +0200 >Subject: [PATCH 04/12] smbd: log share_mode_watch_recv() errors as errors > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624 > >Pair-Programmed-With: Stefan Metzmacher <metze@samba.org> > >Signed-off-by: Ralph Boehme <slow@samba.org> >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Guenther Deschner <gd@samba.org> >(cherry picked from commit b45e78871aadca6ae33475bee890736838f44219) >--- > source3/smbd/open.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > >diff --git a/source3/smbd/open.c b/source3/smbd/open.c >index 36c890dc9d55..e868ba4d4d6a 100644 >--- a/source3/smbd/open.c >+++ b/source3/smbd/open.c >@@ -2972,8 +2972,9 @@ static void defer_open_done(struct tevent_req *req) > status = share_mode_watch_recv(req, NULL, NULL); > TALLOC_FREE(req); > if (!NT_STATUS_IS_OK(status)) { >- DEBUG(5, ("dbwrap_watched_watch_recv returned %s\n", >- nt_errstr(status))); >+ DBG_ERR("share_mode_watch_recv() returned %s, " >+ "rescheduling mid %" PRIu64 "\n", >+ nt_errstr(status), state->mid); > /* > * Even if it failed, retry anyway. TODO: We need a way to > * tell a re-scheduled open about that error. >-- >2.34.1 > > >From d3d559b8680a9850ac47f69ba3ba762ebbf1bdb7 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 4 Apr 2024 19:18:19 +0200 >Subject: [PATCH 05/12] smbd: add option "smbd lease break:debug hung procs" > >By enabling this a process sending a lease break message to another process >holding a lease will start watching that process and if that process didn't >process the lease break within 10 seconds (cf server_id_watch_waited()), we log >a kernel stack backtrace of that process. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624 > >Pair-Programmed-With: Stefan Metzmacher <metze@samba.org> > >Signed-off-by: Ralph Boehme <slow@samba.org> >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Guenther Deschner <gd@samba.org> >(cherry picked from commit d8613d7ee23c4e990285a387eb9ac2eeefff9749) >--- > source3/smbd/open.c | 110 ++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 102 insertions(+), 8 deletions(-) > >diff --git a/source3/smbd/open.c b/source3/smbd/open.c >index e868ba4d4d6a..750875018a11 100644 >--- a/source3/smbd/open.c >+++ b/source3/smbd/open.c >@@ -38,6 +38,7 @@ > #include "serverid.h" > #include "messages.h" > #include "source3/lib/dbwrap/dbwrap_watch.h" >+#include "source3/lib/server_id_watch.h" > #include "locking/leases_db.h" > #include "librpc/gen_ndr/ndr_leases_db.h" > #include "lib/util/time_basic.h" >@@ -2475,6 +2476,10 @@ static int map_lease_type_to_oplock(uint32_t lease_type) > return result; > } > >+struct blocker_debug_state { >+ size_t num_blockers; >+}; >+ > struct delay_for_oplock_state { > struct files_struct *fsp; > const struct smb2_lease *lease; >@@ -2486,8 +2491,22 @@ struct delay_for_oplock_state { > bool have_other_lease; > uint32_t total_lease_types; > bool delay; >+ struct blocker_debug_state *blocker_debug_state; > }; > >+static int blocker_debug_state_destructor(struct blocker_debug_state *state) >+{ >+ if (state->num_blockers == 0) { >+ return 0; >+ } >+ >+ DBG_DEBUG("blocker_debug_state [%p] num_blockers [%zu]\n", >+ state, state->num_blockers); >+ return 0; >+} >+ >+static void delay_for_oplock_fn_watch_done(struct tevent_req *subreq); >+ > static bool delay_for_oplock_fn( > struct share_mode_entry *e, > bool *modified, >@@ -2500,6 +2519,8 @@ static bool delay_for_oplock_fn( > uint32_t e_lease_type = SMB2_LEASE_NONE; > uint32_t break_to; > bool lease_is_breaking = false; >+ struct tevent_req *subreq = NULL; >+ struct server_id_buf idbuf = {}; > > if (e_is_lease) { > NTSTATUS status; >@@ -2639,9 +2660,56 @@ static bool delay_for_oplock_fn( > state->delay = true; > } > >+ if (!state->delay) { >+ return false; >+ } >+ >+ if (state->blocker_debug_state == NULL) { >+ return false; >+ } >+ >+ subreq = server_id_watch_send(state->blocker_debug_state, >+ fsp->conn->sconn->ev_ctx, >+ e->pid); >+ if (subreq == NULL) { >+ DBG_ERR("server_id_watch_send(%s) returned NULL\n", >+ server_id_str_buf(e->pid, &idbuf)); >+ return false; >+ } >+ >+ tevent_req_set_callback(subreq, >+ delay_for_oplock_fn_watch_done, >+ state->blocker_debug_state); >+ >+ state->blocker_debug_state->num_blockers++; >+ >+ DBG_DEBUG("Starting to watch pid [%s] state [%p] num_blockers [%zu]\n", >+ server_id_str_buf(e->pid, &idbuf), >+ state->blocker_debug_state, >+ state->blocker_debug_state->num_blockers); >+ > return false; > }; > >+static void delay_for_oplock_fn_watch_done(struct tevent_req *subreq) >+{ >+ struct blocker_debug_state *blocker_debug_state = tevent_req_callback_data( >+ subreq, struct blocker_debug_state); >+ struct server_id pid = {}; >+ struct server_id_buf idbuf = {}; >+ int ret; >+ >+ ret = server_id_watch_recv(subreq, &pid); >+ if (ret != 0) { >+ DBG_ERR("server_id_watch_recv failed %s\n", strerror(ret)); >+ return; >+ } >+ >+ DBG_DEBUG("state [%p] server_id_watch_recv() returned pid [%s] exited\n", >+ blocker_debug_state, >+ server_id_str_buf(pid, &idbuf)); >+} >+ > static NTSTATUS delay_for_oplock(files_struct *fsp, > int oplock_request, > const struct smb2_lease *lease, >@@ -2650,7 +2718,8 @@ static NTSTATUS delay_for_oplock(files_struct *fsp, > uint32_t create_disposition, > bool first_open_attempt, > int *poplock_type, >- uint32_t *pgranted) >+ uint32_t *pgranted, >+ struct blocker_debug_state **blocker_debug_state) > { > struct delay_for_oplock_state state = { > .fsp = fsp, >@@ -2696,6 +2765,22 @@ static NTSTATUS delay_for_oplock(files_struct *fsp, > goto grant; > } > >+ if (lp_parm_bool(GLOBAL_SECTION_SNUM, >+ "smbd lease break", >+ "debug hung procs", >+ false)) >+ { >+ state.blocker_debug_state = talloc_zero(fsp, >+ struct blocker_debug_state); >+ if (state.blocker_debug_state == NULL) { >+ return NT_STATUS_NO_MEMORY; >+ } >+ talloc_steal(talloc_tos(), state.blocker_debug_state); >+ >+ talloc_set_destructor(state.blocker_debug_state, >+ blocker_debug_state_destructor); >+ } >+ > state.delay_mask = have_sharing_violation ? > SMB2_LEASE_HANDLE : SMB2_LEASE_WRITE; > >@@ -2717,6 +2802,7 @@ static NTSTATUS delay_for_oplock(files_struct *fsp, > } > > if (state.delay) { >+ *blocker_debug_state = state.blocker_debug_state; > return NT_STATUS_RETRY; > } > >@@ -2830,7 +2916,8 @@ static NTSTATUS handle_share_mode_lease( > const struct smb2_lease *lease, > bool first_open_attempt, > int *poplock_type, >- uint32_t *pgranted) >+ uint32_t *pgranted, >+ struct blocker_debug_state **blocker_debug_state) > { > bool sharing_violation = false; > NTSTATUS status; >@@ -2871,7 +2958,8 @@ static NTSTATUS handle_share_mode_lease( > create_disposition, > first_open_attempt, > poplock_type, >- pgranted); >+ pgranted, >+ blocker_debug_state); > if (!NT_STATUS_IS_OK(status)) { > return status; > } >@@ -2906,7 +2994,8 @@ static void defer_open_done(struct tevent_req *req); > static void defer_open(struct share_mode_lock *lck, > struct timeval timeout, > struct smb_request *req, >- struct file_id id) >+ struct file_id id, >+ struct blocker_debug_state **blocker_debug_state) > { > struct deferred_open_record *open_rec = NULL; > struct timeval abs_timeout; >@@ -2950,6 +3039,8 @@ static void defer_open(struct share_mode_lock *lck, > } > tevent_req_set_callback(watch_req, defer_open_done, watch_state); > >+ talloc_move(watch_req, blocker_debug_state); >+ > ok = tevent_req_set_endtime(watch_req, req->sconn->ev_ctx, abs_timeout); > if (!ok) { > exit_server("tevent_req_set_endtime failed"); >@@ -3232,7 +3323,8 @@ static bool open_match_attributes(connection_struct *conn, > > static void schedule_defer_open(struct share_mode_lock *lck, > struct file_id id, >- struct smb_request *req) >+ struct smb_request *req, >+ struct blocker_debug_state **blocker_debug_state) > { > /* This is a relative time, added to the absolute > request_time value to get the absolute timeout time. >@@ -3256,7 +3348,7 @@ static void schedule_defer_open(struct share_mode_lock *lck, > return; > } > >- defer_open(lck, timeout, req, id); >+ defer_open(lck, timeout, req, id, blocker_debug_state); > } > > /**************************************************************************** >@@ -3318,6 +3410,7 @@ static NTSTATUS check_and_store_share_mode( > int oplock_type = NO_OPLOCK; > uint32_t granted_lease = 0; > const struct smb2_lease_key *lease_key = NULL; >+ struct blocker_debug_state *blocker_debug_state = NULL; > bool delete_on_close; > bool ok; > >@@ -3340,9 +3433,10 @@ static NTSTATUS check_and_store_share_mode( > lease, > first_open_attempt, > &oplock_type, >- &granted_lease); >+ &granted_lease, >+ &blocker_debug_state); > if (NT_STATUS_EQUAL(status, NT_STATUS_RETRY)) { >- schedule_defer_open(lck, fsp->file_id, req); >+ schedule_defer_open(lck, fsp->file_id, req, &blocker_debug_state); > return NT_STATUS_SHARING_VIOLATION; > } > if (!NT_STATUS_IS_OK(status)) { >-- >2.34.1 > > >From 697d60235dc2ec9088aa4da8e8a3b23fd820354f Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 20 Mar 2024 14:27:27 +0100 >Subject: [PATCH 06/12] smbd: move trace_state variable behind tv variable > >Next commit adds timestamp variables to trace_state that want to be initialized >with the current time, so moving behind tv we can then just reuse tv for that. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Guenther Deschner <gd@samba.org> >(cherry picked from commit 679e12aee2f0c283a6f9b9c6008c549a6ca9633e) >--- > source3/smbd/smb2_process.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > >diff --git a/source3/smbd/smb2_process.c b/source3/smbd/smb2_process.c >index fbbe4ef39924..188eaa148394 100644 >--- a/source3/smbd/smb2_process.c >+++ b/source3/smbd/smb2_process.c >@@ -1783,10 +1783,6 @@ void smbd_process(struct tevent_context *ev_ctx, > int sock_fd, > bool interactive) > { >- struct smbd_tevent_trace_state trace_state = { >- .ev = ev_ctx, >- .frame = talloc_stackframe(), >- }; > const struct loadparm_substitution *lp_sub = > loadparm_s3_global_substitution(); > struct smbXsrv_client *client = NULL; >@@ -1797,6 +1793,10 @@ void smbd_process(struct tevent_context *ev_ctx, > int ret; > NTSTATUS status; > struct timeval tv = timeval_current(); >+ struct smbd_tevent_trace_state trace_state = { >+ .ev = ev_ctx, >+ .frame = talloc_stackframe(), >+ }; > NTTIME now = timeval_to_nttime(&tv); > char *chroot_dir = NULL; > int rc; >-- >2.34.1 > > >From 935db1d23d270f695cf09e2743b046ae575a2900 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 20 Mar 2024 14:28:43 +0100 >Subject: [PATCH 07/12] smbd: add option "smbd:debug events" for tevent > handling duration threshold warnings > >Can be used to enable printing an error message if tevent event handlers ran >longer then three seconds. Also logs a message with a loglevel of 3 if there >were no events at hall. > >Enabled by default with 'log level = 10' or >'smbd profiling level = on'... > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624 > >Pair-Programmed-With: Stefan Metzmacher <metze@samba.org> > >Signed-off-by: Ralph Boehme <slow@samba.org> >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Guenther Deschner <gd@samba.org> >(cherry picked from commit 90d776cb18395ed804f0ab4fd13ef571fc0ad827) >--- > source3/smbd/smb2_process.c | 64 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 64 insertions(+) > >diff --git a/source3/smbd/smb2_process.c b/source3/smbd/smb2_process.c >index 188eaa148394..dbe91132f7f1 100644 >--- a/source3/smbd/smb2_process.c >+++ b/source3/smbd/smb2_process.c >@@ -1692,8 +1692,36 @@ struct smbd_tevent_trace_state { > struct tevent_context *ev; > TALLOC_CTX *frame; > SMBPROFILE_BASIC_ASYNC_STATE(profile_idle); >+ struct timeval before_wait_tv; >+ struct timeval after_wait_tv; > }; > >+static inline void smbd_tevent_trace_callback_before_wait( >+ struct smbd_tevent_trace_state *state) >+{ >+ struct timeval now = timeval_current(); >+ struct timeval diff; >+ >+ diff = tevent_timeval_until(&state->after_wait_tv, &now); >+ if (diff.tv_sec > 3) { >+ DBG_ERR("Handling event took %ld seconds!\n", (long)diff.tv_sec); >+ } >+ state->before_wait_tv = now; >+} >+ >+static inline void smbd_tevent_trace_callback_after_wait( >+ struct smbd_tevent_trace_state *state) >+{ >+ struct timeval now = timeval_current(); >+ struct timeval diff; >+ >+ diff = tevent_timeval_until(&state->before_wait_tv, &now); >+ if (diff.tv_sec > 30) { >+ DBG_NOTICE("No event for %ld seconds!\n", (long)diff.tv_sec); >+ } >+ state->after_wait_tv = now; >+} >+ > static inline void smbd_tevent_trace_callback_before_loop_once( > struct smbd_tevent_trace_state *state) > { >@@ -1729,6 +1757,30 @@ static void smbd_tevent_trace_callback(enum tevent_trace_point point, > errno = 0; > } > >+static void smbd_tevent_trace_callback_debug(enum tevent_trace_point point, >+ void *private_data) >+{ >+ struct smbd_tevent_trace_state *state = >+ (struct smbd_tevent_trace_state *)private_data; >+ >+ switch (point) { >+ case TEVENT_TRACE_BEFORE_WAIT: >+ smbd_tevent_trace_callback_before_wait(state); >+ break; >+ case TEVENT_TRACE_AFTER_WAIT: >+ smbd_tevent_trace_callback_after_wait(state); >+ break; >+ case TEVENT_TRACE_BEFORE_LOOP_ONCE: >+ smbd_tevent_trace_callback_before_loop_once(state); >+ break; >+ case TEVENT_TRACE_AFTER_LOOP_ONCE: >+ smbd_tevent_trace_callback_after_loop_once(state); >+ break; >+ } >+ >+ errno = 0; >+} >+ > static void smbd_tevent_trace_callback_profile(enum tevent_trace_point point, > void *private_data) > { >@@ -1737,6 +1789,7 @@ static void smbd_tevent_trace_callback_profile(enum tevent_trace_point point, > > switch (point) { > case TEVENT_TRACE_BEFORE_WAIT: >+ smbd_tevent_trace_callback_before_wait(state); > if (!smbprofile_dump_pending()) { > /* > * If there's no dump pending >@@ -1749,6 +1802,7 @@ static void smbd_tevent_trace_callback_profile(enum tevent_trace_point point, > SMBPROFILE_BASIC_ASYNC_START(idle, profile_p, state->profile_idle); > break; > case TEVENT_TRACE_AFTER_WAIT: >+ smbd_tevent_trace_callback_after_wait(state); > SMBPROFILE_BASIC_ASYNC_END(state->profile_idle); > if (!smbprofile_dump_pending()) { > /* >@@ -1796,7 +1850,13 @@ void smbd_process(struct tevent_context *ev_ctx, > struct smbd_tevent_trace_state trace_state = { > .ev = ev_ctx, > .frame = talloc_stackframe(), >+ .before_wait_tv = tv, >+ .after_wait_tv = tv, > }; >+ bool debug = lp_parm_bool(GLOBAL_SECTION_SNUM, >+ "smbd", >+ "debug events", >+ CHECK_DEBUGLVL(DBGLVL_DEBUG)); > NTTIME now = timeval_to_nttime(&tv); > char *chroot_dir = NULL; > int rc; >@@ -2041,6 +2101,10 @@ void smbd_process(struct tevent_context *ev_ctx, > tevent_set_trace_callback(ev_ctx, > smbd_tevent_trace_callback_profile, > &trace_state); >+ } else if (debug) { >+ tevent_set_trace_callback(ev_ctx, >+ smbd_tevent_trace_callback_debug, >+ &trace_state); > } else { > tevent_set_trace_callback(ev_ctx, > smbd_tevent_trace_callback, >-- >2.34.1 > > >From e97bbc1948f36c94c575678900be33fd8dd636f5 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 26 Aug 2024 14:11:02 +0200 >Subject: [PATCH 08/12] vfs_error_inject: add 'error_inject:durable_reconnect = > st_ex_nlink' > >This allows to simulate durable reconnect failures because the stat >information of the file changed. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Guenther Deschner <gd@samba.org> >(cherry picked from commit 692ed832dfff61ad1c9b646b5c8d6f85f25efb99) >--- > source3/modules/vfs_error_inject.c | 76 ++++++++++++++++++++++++++++++ > 1 file changed, 76 insertions(+) > >diff --git a/source3/modules/vfs_error_inject.c b/source3/modules/vfs_error_inject.c >index 529504fd8d5e..dcf0de0a2d96 100644 >--- a/source3/modules/vfs_error_inject.c >+++ b/source3/modules/vfs_error_inject.c >@@ -19,6 +19,7 @@ > > #include "includes.h" > #include "smbd/smbd.h" >+#include "librpc/gen_ndr/ndr_open_files.h" > > #undef DBGC_CLASS > #define DBGC_CLASS DBGC_VFS >@@ -204,11 +205,86 @@ static int vfs_error_inject_unlinkat(struct vfs_handle_struct *handle, > return -1; > } > >+static NTSTATUS vfs_error_inject_durable_reconnect(struct vfs_handle_struct *handle, >+ struct smb_request *smb1req, >+ struct smbXsrv_open *op, >+ const DATA_BLOB old_cookie, >+ TALLOC_CTX *mem_ctx, >+ struct files_struct **fsp, >+ DATA_BLOB *new_cookie) >+{ >+ const char *vfs_func = "durable_reconnect"; >+ const char *err_str = NULL; >+ NTSTATUS status; >+ enum ndr_err_code ndr_err; >+ struct vfs_default_durable_cookie cookie; >+ DATA_BLOB modified_cookie = data_blob_null; >+ >+ err_str = lp_parm_const_string(SNUM(handle->conn), >+ "error_inject", >+ vfs_func, >+ NULL); >+ if (err_str == NULL) { >+ return SMB_VFS_NEXT_DURABLE_RECONNECT(handle, >+ smb1req, >+ op, >+ old_cookie, >+ mem_ctx, >+ fsp, >+ new_cookie); >+ } >+ >+ ndr_err = ndr_pull_struct_blob(&old_cookie, talloc_tos(), &cookie, >+ (ndr_pull_flags_fn_t)ndr_pull_vfs_default_durable_cookie); >+ if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { >+ status = ndr_map_error2ntstatus(ndr_err); >+ return status; >+ } >+ >+ if (strcmp(cookie.magic, VFS_DEFAULT_DURABLE_COOKIE_MAGIC) != 0) { >+ return NT_STATUS_INVALID_PARAMETER; >+ } >+ >+ if (cookie.version != VFS_DEFAULT_DURABLE_COOKIE_VERSION) { >+ return NT_STATUS_INVALID_PARAMETER; >+ } >+ >+ if (strequal(err_str, "st_ex_nlink")) { >+ cookie.stat_info.st_ex_nlink += 1; >+ } else { >+ DBG_ERR("Unknown error inject %s requested " >+ "for vfs function %s\n", err_str, vfs_func); >+ return SMB_VFS_NEXT_DURABLE_RECONNECT(handle, >+ smb1req, >+ op, >+ old_cookie, >+ mem_ctx, >+ fsp, >+ new_cookie); >+ } >+ >+ ndr_err = ndr_push_struct_blob(&modified_cookie, talloc_tos(), &cookie, >+ (ndr_push_flags_fn_t)ndr_push_vfs_default_durable_cookie); >+ if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { >+ status = ndr_map_error2ntstatus(ndr_err); >+ return status; >+ } >+ >+ return SMB_VFS_NEXT_DURABLE_RECONNECT(handle, >+ smb1req, >+ op, >+ modified_cookie, >+ mem_ctx, >+ fsp, >+ new_cookie); >+} >+ > static struct vfs_fn_pointers vfs_error_inject_fns = { > .chdir_fn = vfs_error_inject_chdir, > .pwrite_fn = vfs_error_inject_pwrite, > .openat_fn = vfs_error_inject_openat, > .unlinkat_fn = vfs_error_inject_unlinkat, >+ .durable_reconnect_fn = vfs_error_inject_durable_reconnect, > }; > > static_decl_vfs; >-- >2.34.1 > > >From 72ae2336b5681332c72d85608e0a9737025fdc8f Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 26 Aug 2024 14:42:02 +0200 >Subject: [PATCH 09/12] s4:torture/smb2: add > smb2.durable-v2-regressions.durable_v2_reconnect_bug15624 > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Guenther Deschner <gd@samba.org> >(cherry picked from commit ef4ef04e7f83b1029446ff8b5fc5fdf4ab33edbd) >--- > selftest/skip | 1 + > source4/torture/smb2/durable_v2_open.c | 118 +++++++++++++++++++++++++ > source4/torture/smb2/smb2.c | 2 + > 3 files changed, 121 insertions(+) > >diff --git a/selftest/skip b/selftest/skip >index e808367c00db..056c54ea2875 100644 >--- a/selftest/skip >+++ b/selftest/skip >@@ -149,3 +149,4 @@ bench # don't run benchmarks in our selftest > ^samba.tests.reparsepoints.* > ^samba.tests.smb2symlink.* > ^samba3.blackbox.open-eintr.* >+smb2.durable-v2-regressions # Only used in blackbox tests >diff --git a/source4/torture/smb2/durable_v2_open.c b/source4/torture/smb2/durable_v2_open.c >index 9b9af11124c2..7447dd287a48 100644 >--- a/source4/torture/smb2/durable_v2_open.c >+++ b/source4/torture/smb2/durable_v2_open.c >@@ -2355,6 +2355,112 @@ done: > return ret; > } > >+/** >+ * basic test for doing a durable open >+ * tcp disconnect, reconnect, do a durable reopen (succeeds) >+ */ >+static bool test_durable_v2_reconnect_bug15624(struct torture_context *tctx, >+ struct smb2_tree *tree, >+ struct smb2_tree *tree2) >+{ >+ NTSTATUS status; >+ TALLOC_CTX *mem_ctx = talloc_new(tctx); >+ char fname[256]; >+ struct smb2_handle _h; >+ struct smb2_handle *h = NULL; >+ struct smb2_create io; >+ struct GUID create_guid = GUID_random(); >+ struct smbcli_options options; >+ uint64_t previous_session_id; >+ uint8_t b = 0; >+ bool ret = true; >+ bool ok; >+ >+ if (!torture_setting_bool(tctx, "bug15624", false)) { >+ torture_comment(tctx, >+ "share requires:\n" >+ "'vfs objects = error_inject'\n" >+ "'error_inject:durable_reconnect=st_ex_nlink'\n" >+ "test requires:\n" >+ "'--option=torture:bug15624=yes'\n"); >+ torture_skip(tctx, "'--option=torture:bug15624=yes' missing"); >+ } >+ >+ options = tree->session->transport->options; >+ previous_session_id = smb2cli_session_current_id(tree->session->smbXcli); >+ >+ /* Choose a random name in case the state is left a little funky. */ >+ snprintf(fname, >+ sizeof(fname), >+ "durable_v2_reconnect_bug15624_%s.dat", >+ generate_random_str(tctx, 8)); >+ >+ smb2_util_unlink(tree, fname); >+ >+ smb2_oplock_create_share(&io, fname, >+ smb2_util_share_access(""), >+ smb2_util_oplock_level("b")); >+ io.in.durable_open = false; >+ io.in.durable_open_v2 = true; >+ io.in.persistent_open = false; >+ io.in.create_guid = create_guid; >+ io.in.timeout = 0; >+ >+ status = smb2_create(tree, mem_ctx, &io); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ _h = io.out.file.handle; >+ h = &_h; >+ CHECK_VAL(io.out.oplock_level, smb2_util_oplock_level("b")); >+ CHECK_VAL(io.out.durable_open_v2, true); >+ >+ status = smb2_util_write(tree, *h, &b, 0, 1); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ /* disconnect, leaving the durable open */ >+ TALLOC_FREE(tree); >+ h = NULL; >+ >+ ok = torture_smb2_connection_ext(tctx, previous_session_id, >+ &options, &tree); >+ torture_assert_goto(tctx, ok, ret, done, "couldn't reconnect, bailing\n"); >+ >+ ZERO_STRUCT(io); >+ io.in.fname = fname; >+ io.in.durable_open_v2 = false; >+ io.in.durable_handle_v2 = &_h; >+ io.in.create_guid = create_guid; >+ >+ /* >+ * This assumes 'error_inject:durable_reconnect = st_ex_nlink' >+ * will cause the durable reconnect to fail... >+ * in order to have a regression test for the dead lock. >+ */ >+ status = smb2_create(tree, mem_ctx, &io); >+ CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_NOT_FOUND); >+ >+ /* >+ * With the regression this will fail with >+ * a timeout... >+ */ >+ status = smb2_util_unlink(tree2, fname); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+done: >+ if (h != NULL) { >+ smb2_util_close(tree, *h); >+ } >+ TALLOC_FREE(tree); >+ >+ smb2_util_unlink(tree2, fname); >+ >+ TALLOC_FREE(tree2); >+ >+ talloc_free(mem_ctx); >+ >+ return ret; >+} >+ > struct torture_suite *torture_smb2_durable_v2_delay_init(TALLOC_CTX *ctx) > { > struct torture_suite *suite = >@@ -2369,3 +2475,15 @@ struct torture_suite *torture_smb2_durable_v2_delay_init(TALLOC_CTX *ctx) > > return suite; > } >+ >+struct torture_suite *torture_smb2_durable_v2_regressions_init(TALLOC_CTX *ctx) >+{ >+ struct torture_suite *suite = >+ torture_suite_create(ctx, "durable-v2-regressions"); >+ >+ torture_suite_add_2smb2_test(suite, >+ "durable_v2_reconnect_bug15624", >+ test_durable_v2_reconnect_bug15624); >+ >+ return suite; >+} >diff --git a/source4/torture/smb2/smb2.c b/source4/torture/smb2/smb2.c >index 5b6477e47bc3..9cf7f5da78b5 100644 >--- a/source4/torture/smb2/smb2.c >+++ b/source4/torture/smb2/smb2.c >@@ -170,6 +170,8 @@ NTSTATUS torture_smb2_init(TALLOC_CTX *ctx) > torture_smb2_durable_v2_open_init(suite)); > torture_suite_add_suite(suite, > torture_smb2_durable_v2_delay_init(suite)); >+ torture_suite_add_suite(suite, >+ torture_smb2_durable_v2_regressions_init(suite)); > torture_suite_add_suite(suite, torture_smb2_dir_init(suite)); > torture_suite_add_suite(suite, torture_smb2_lease_init(suite)); > torture_suite_add_suite(suite, torture_smb2_compound_init(suite)); >-- >2.34.1 > > >From e0eca3d14c84fbc8bae0fdd1b8a6f980e20bcd7c Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 26 Aug 2024 14:42:12 +0200 >Subject: [PATCH 10/12] s3:tests: let test_durable_handle_reconnect.sh run > smb2.durable-v2-regressions.durable_v2_reconnect_bug15624 > >This demonstrates the dead lock after a durable reconnect failed >because the stat info changed, the file can't be accessed anymore >as we leak the incomplete share mode entry in a still running >process. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Guenther Deschner <gd@samba.org> >(cherry picked from commit 14875448ca06a3a28800343a3a326f1a66bccec0) >--- > .../samba3.blackbox.durable_v2_delay | 1 + > .../tests/test_durable_handle_reconnect.sh | 18 ++++++++++++++++++ > 2 files changed, 19 insertions(+) > create mode 100644 selftest/knownfail.d/samba3.blackbox.durable_v2_delay > >diff --git a/selftest/knownfail.d/samba3.blackbox.durable_v2_delay b/selftest/knownfail.d/samba3.blackbox.durable_v2_delay >new file mode 100644 >index 000000000000..88e299607970 >--- /dev/null >+++ b/selftest/knownfail.d/samba3.blackbox.durable_v2_delay >@@ -0,0 +1 @@ >+^samba3.blackbox.durable_v2_delay.durable-v2-regressions.durable_v2_reconnect_bug15624 >diff --git a/source3/script/tests/test_durable_handle_reconnect.sh b/source3/script/tests/test_durable_handle_reconnect.sh >index 0ab32974824f..fd5c156956f3 100755 >--- a/source3/script/tests/test_durable_handle_reconnect.sh >+++ b/source3/script/tests/test_durable_handle_reconnect.sh >@@ -33,4 +33,22 @@ testit "durable_v2_delay.durable_v2_reconnect_delay_msec" $VALGRIND \ > > rm $delay_inject_conf > >+error_inject_conf=$(dirname $SMB_CONF_PATH)/error_inject.conf >+ >+cat > $error_inject_conf << _EOF >+ kernel share modes = no >+ kernel oplocks = no >+ posix locking = no >+ error_inject:durable_reconnect = st_ex_nlink >+_EOF >+ >+testit "durable-v2-regressions.durable_v2_reconnect_bug15624" \ >+ $VALGRIND $BINDIR/smbtorture //$SERVER_IP/error_inject \ >+ -U$USERNAME%$PASSWORD \ >+ --option=torture:bug15624=yes \ >+ smb2.durable-v2-regressions.durable_v2_reconnect_bug15624 || >+ failed=$(expr $failed + 1) >+ >+rm $error_inject_conf >+ > testok $0 $failed >-- >2.34.1 > > >From c51ab84619b9556f7ead2b7c1ff3388477477cb4 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Tue, 9 Apr 2024 14:52:44 +0200 >Subject: [PATCH 11/12] smbd: consolidate DH reconnect failure code > >No change in behaviour, except that we now >also call fd_close() if vfs_default_durable_cookie() >failed. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624 > >Pair-Programmed-With: Stefan Metzmacher <metze@samba.org> > >Signed-off-by: Ralph Boehme <slow@samba.org> >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Guenther Deschner <gd@samba.org> >(cherry picked from commit a91457f97c98fcec1ed062514c364271af1df669) >--- > source3/smbd/durable.c | 142 ++++++++++++++--------------------------- > 1 file changed, 47 insertions(+), 95 deletions(-) > >diff --git a/source3/smbd/durable.c b/source3/smbd/durable.c >index b21c223b2e44..50075ddd3f7f 100644 >--- a/source3/smbd/durable.c >+++ b/source3/smbd/durable.c >@@ -624,22 +624,22 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > ok = share_mode_forall_entries(lck, durable_reconnect_fn, &e); > if (!ok) { > DBG_WARNING("share_mode_forall_entries failed\n"); >- TALLOC_FREE(lck); >- return NT_STATUS_INTERNAL_DB_ERROR; >+ status = NT_STATUS_INTERNAL_DB_ERROR; >+ goto fail; > } > > if (e.pid.pid == 0) { > DBG_WARNING("Did not find a unique valid share mode entry\n"); >- TALLOC_FREE(lck); >- return NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ goto fail; > } > > if (!server_id_is_disconnected(&e.pid)) { > DEBUG(5, ("vfs_default_durable_reconnect: denying durable " > "reconnect for handle that was not marked " > "disconnected (e.g. smbd or cluster node died)\n")); >- TALLOC_FREE(lck); >- return NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ goto fail; > } > > if (e.share_file_id != op->global->open_persistent_id) { >@@ -648,8 +648,8 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > "(e.g. another client had opened the file)\n", > e.share_file_id, > op->global->open_persistent_id); >- TALLOC_FREE(lck); >- return NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ goto fail; > } > > if ((e.access_mask & (FILE_WRITE_DATA|FILE_APPEND_DATA)) && >@@ -658,8 +658,8 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > DEBUG(5, ("vfs_default_durable_reconnect: denying durable " > "share[%s] is not writeable anymore\n", > lp_servicename(talloc_tos(), lp_sub, SNUM(conn)))); >- TALLOC_FREE(lck); >- return NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ goto fail; > } > > /* >@@ -670,8 +670,7 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > if (!NT_STATUS_IS_OK(status)) { > DEBUG(0, ("vfs_default_durable_reconnect: failed to create " > "new fsp: %s\n", nt_errstr(status))); >- TALLOC_FREE(lck); >- return status; >+ goto fail; > } > > fh_set_private_options(fsp->fh, e.private_options); >@@ -714,9 +713,8 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > */ > if (!GUID_equal(fsp_client_guid(fsp), > &e.client_guid)) { >- TALLOC_FREE(lck); >- file_free(smb1req, fsp); >- return NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ goto fail; > } > > status = leases_db_get( >@@ -730,9 +728,7 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > &lease_version, /* lease_version */ > &epoch); /* epoch */ > if (!NT_STATUS_IS_OK(status)) { >- TALLOC_FREE(lck); >- file_free(smb1req, fsp); >- return status; >+ goto fail; > } > > fsp->lease = find_fsp_lease( >@@ -742,9 +738,8 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > lease_version, > epoch); > if (fsp->lease == NULL) { >- TALLOC_FREE(lck); >- file_free(smb1req, fsp); >- return NT_STATUS_NO_MEMORY; >+ status = NT_STATUS_NO_MEMORY; >+ goto fail; > } > } > >@@ -760,12 +755,10 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > > status = fsp_set_smb_fname(fsp, smb_fname); > if (!NT_STATUS_IS_OK(status)) { >- TALLOC_FREE(lck); >- file_free(smb1req, fsp); > DEBUG(0, ("vfs_default_durable_reconnect: " > "fsp_set_smb_fname failed: %s\n", > nt_errstr(status))); >- return status; >+ goto fail; > } > > op->compat = fsp; >@@ -780,11 +773,8 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > fh_get_gen_id(fsp->fh)); > if (!ok) { > DBG_DEBUG("Could not set new share_mode_entry values\n"); >- TALLOC_FREE(lck); >- op->compat = NULL; >- fsp->op = NULL; >- file_free(smb1req, fsp); >- return NT_STATUS_INTERNAL_ERROR; >+ status = NT_STATUS_INTERNAL_ERROR; >+ goto fail; > } > > ok = brl_reconnect_disconnected(fsp); >@@ -793,11 +783,7 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > DEBUG(1, ("vfs_default_durable_reconnect: " > "failed to reopen brlocks: %s\n", > nt_errstr(status))); >- TALLOC_FREE(lck); >- op->compat = NULL; >- fsp->op = NULL; >- file_free(smb1req, fsp); >- return status; >+ goto fail; > } > > /* >@@ -813,13 +799,9 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > > status = fd_openat(conn->cwd_fsp, fsp->fsp_name, fsp, &how); > if (!NT_STATUS_IS_OK(status)) { >- TALLOC_FREE(lck); > DEBUG(1, ("vfs_default_durable_reconnect: failed to open " > "file: %s\n", nt_errstr(status))); >- op->compat = NULL; >- fsp->op = NULL; >- file_free(smb1req, fsp); >- return status; >+ goto fail; > } > > /* >@@ -833,48 +815,22 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > > ret = SMB_VFS_FSTAT(fsp, &fsp->fsp_name->st); > if (ret == -1) { >- NTSTATUS close_status; > status = map_nt_error_from_unix_common(errno); > DEBUG(1, ("Unable to fstat stream: %s => %s\n", > smb_fname_str_dbg(smb_fname), > nt_errstr(status))); >- close_status = fd_close(fsp); >- if (!NT_STATUS_IS_OK(close_status)) { >- DBG_ERR("fd_close failed (%s) - leaking file " >- "descriptor\n", nt_errstr(close_status)); >- } >- TALLOC_FREE(lck); >- op->compat = NULL; >- fsp->op = NULL; >- file_free(smb1req, fsp); >- return status; >+ goto fail; > } > > if (!S_ISREG(fsp->fsp_name->st.st_ex_mode)) { >- NTSTATUS close_status = fd_close(fsp); >- if (!NT_STATUS_IS_OK(close_status)) { >- DBG_ERR("fd_close failed (%s) - leaking file " >- "descriptor\n", nt_errstr(close_status)); >- } >- TALLOC_FREE(lck); >- op->compat = NULL; >- fsp->op = NULL; >- file_free(smb1req, fsp); >- return NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ goto fail; > } > > file_id = vfs_file_id_from_sbuf(conn, &fsp->fsp_name->st); > if (!file_id_equal(&cookie.id, &file_id)) { >- NTSTATUS close_status = fd_close(fsp); >- if (!NT_STATUS_IS_OK(close_status)) { >- DBG_ERR("fd_close failed (%s) - leaking file " >- "descriptor\n", nt_errstr(close_status)); >- } >- TALLOC_FREE(lck); >- op->compat = NULL; >- fsp->op = NULL; >- file_free(smb1req, fsp); >- return NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ goto fail; > } > > (void)fdos_mode(fsp); >@@ -883,42 +839,21 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > &fsp->fsp_name->st, > fsp_str_dbg(fsp)); > if (!ok) { >- NTSTATUS close_status = fd_close(fsp); >- if (!NT_STATUS_IS_OK(close_status)) { >- DBG_ERR("fd_close failed (%s) - leaking file " >- "descriptor\n", nt_errstr(close_status)); >- } >- TALLOC_FREE(lck); >- op->compat = NULL; >- fsp->op = NULL; >- file_free(smb1req, fsp); >- return NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ status = NT_STATUS_OBJECT_NAME_NOT_FOUND; >+ goto fail; > } > > status = set_file_oplock(fsp); > if (!NT_STATUS_IS_OK(status)) { >- NTSTATUS close_status = fd_close(fsp); >- if (!NT_STATUS_IS_OK(close_status)) { >- DBG_ERR("fd_close failed (%s) - leaking file " >- "descriptor\n", nt_errstr(close_status)); >- } >- TALLOC_FREE(lck); >- op->compat = NULL; >- fsp->op = NULL; >- file_free(smb1req, fsp); >- return status; >+ goto fail; > } > > status = vfs_default_durable_cookie(fsp, mem_ctx, &new_cookie_blob); > if (!NT_STATUS_IS_OK(status)) { >- TALLOC_FREE(lck); > DEBUG(1, ("vfs_default_durable_reconnect: " > "vfs_default_durable_cookie - %s\n", > nt_errstr(status))); >- op->compat = NULL; >- fsp->op = NULL; >- file_free(smb1req, fsp); >- return status; >+ goto fail; > } > > smb1req->chain_fsp = fsp; >@@ -935,4 +870,21 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > *new_cookie = new_cookie_blob; > > return NT_STATUS_OK; >+ >+fail: >+ if (fsp != NULL && fsp_get_pathref_fd(fsp) != -1) { >+ NTSTATUS close_status; >+ close_status = fd_close(fsp); >+ if (!NT_STATUS_IS_OK(close_status)) { >+ DBG_ERR("fd_close failed (%s), leaking fd\n", >+ nt_errstr(close_status)); >+ } >+ } >+ TALLOC_FREE(lck); >+ if (fsp != NULL) { >+ op->compat = NULL; >+ fsp->op = NULL; >+ file_free(smb1req, fsp); >+ } >+ return status; > } >-- >2.34.1 > > >From 5e564c19289b0914e76339f53aba75dfb191de68 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Tue, 9 Apr 2024 14:53:32 +0200 >Subject: [PATCH 12/12] smbd: remove just created sharemode entry in the error > codepaths >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >Without this we leave stale sharemode entries around that can lead to all sorts >of havoc. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15624 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Guenther Deschner <gd@samba.org> > >Autobuild-User(master): Günther Deschner <gd@samba.org> >Autobuild-Date(master): Thu Sep 19 19:36:19 UTC 2024 on atb-devel-224 > >(cherry picked from commit 2ff3b9bc0d254a63a913ff9084de3d794fee27d0) >--- > selftest/knownfail.d/samba3.blackbox.durable_v2_delay | 1 - > source3/smbd/durable.c | 8 ++++++++ > 2 files changed, 8 insertions(+), 1 deletion(-) > delete mode 100644 selftest/knownfail.d/samba3.blackbox.durable_v2_delay > >diff --git a/selftest/knownfail.d/samba3.blackbox.durable_v2_delay b/selftest/knownfail.d/samba3.blackbox.durable_v2_delay >deleted file mode 100644 >index 88e299607970..000000000000 >--- a/selftest/knownfail.d/samba3.blackbox.durable_v2_delay >+++ /dev/null >@@ -1 +0,0 @@ >-^samba3.blackbox.durable_v2_delay.durable-v2-regressions.durable_v2_reconnect_bug15624 >diff --git a/source3/smbd/durable.c b/source3/smbd/durable.c >index 50075ddd3f7f..98d0d403e305 100644 >--- a/source3/smbd/durable.c >+++ b/source3/smbd/durable.c >@@ -538,6 +538,7 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > enum ndr_err_code ndr_err; > struct vfs_default_durable_cookie cookie; > DATA_BLOB new_cookie_blob = data_blob_null; >+ bool have_share_mode_entry = false; > > *result = NULL; > *new_cookie = data_blob_null; >@@ -776,6 +777,7 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > status = NT_STATUS_INTERNAL_ERROR; > goto fail; > } >+ have_share_mode_entry = true; > > ok = brl_reconnect_disconnected(fsp); > if (!ok) { >@@ -872,6 +874,12 @@ NTSTATUS vfs_default_durable_reconnect(struct connection_struct *conn, > return NT_STATUS_OK; > > fail: >+ if (fsp != NULL && have_share_mode_entry) { >+ /* >+ * Something is screwed up, delete the sharemode entry. >+ */ >+ del_share_mode(lck, fsp); >+ } > if (fsp != NULL && fsp_get_pathref_fd(fsp) != -1) { > NTSTATUS close_status; > close_status = fd_close(fsp); >-- >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=18459">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
slow
:
review+
metze
:
review?
(
gd
)
Actions:
View
Attachments on
bug 15624
:
18452
|
18453
|
18454
|
18458
| 18459