From 89df1f7df29db8c2e2ef8179a875a95ca192510d 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 4c1034c923962cf469471bc7d2e64c30a8b288e0 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 | 63 +++++++++++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 3 deletions(-)

diff --git a/source3/lib/server_id_watch.c b/source3/lib/server_id_watch.c
index 50b35f27b3e2..9ef2e9ef6a83 100644
--- a/source3/lib/server_id_watch.c
+++ b/source3/lib/server_id_watch.c
@@ -17,17 +17,19 @@
  * 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"
+#include "lib/util/util_file.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 +49,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 +94,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 9c9786e15ff029daf37bd6968f6d1881965188eb 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 9ef2e9ef6a83..77b1952f9eb3 100644
--- a/source3/lib/server_id_watch.c
+++ b/source3/lib/server_id_watch.c
@@ -101,6 +101,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;
@@ -109,6 +110,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 6e7799bb00676a36a9dfdece5a02d5be798b4371 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 95034b147a8a..27c3e08b7def 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2929,8 +2929,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 4a9d0a6be074dacdc78c9b59fa3bdb8561d7c372 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 27c3e08b7def..57ee0a278e60 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"
@@ -2432,6 +2433,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;
@@ -2443,8 +2448,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,
@@ -2457,6 +2476,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;
@@ -2596,9 +2617,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,
@@ -2607,7 +2675,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,
@@ -2653,6 +2722,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;
 
@@ -2674,6 +2759,7 @@ static NTSTATUS delay_for_oplock(files_struct *fsp,
 	}
 
 	if (state.delay) {
+		*blocker_debug_state = state.blocker_debug_state;
 		return NT_STATUS_RETRY;
 	}
 
@@ -2787,7 +2873,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;
@@ -2828,7 +2915,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;
 	}
@@ -2863,7 +2951,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;
@@ -2907,6 +2996,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");
@@ -3189,7 +3280,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.
@@ -3213,7 +3305,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);
 }
 
 /****************************************************************************
@@ -3275,6 +3367,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;
 
@@ -3297,9 +3390,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 b9bd7ccbbcd93a6f53ec02962132201d10a767f4 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 736642733593..451a0682f1e3 100644
--- a/source3/smbd/smb2_process.c
+++ b/source3/smbd/smb2_process.c
@@ -1797,10 +1797,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;
@@ -1811,6 +1807,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 0b50ed09766787ae0372e75f3e22381d45f1957f 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 451a0682f1e3..d994a0ffd4b8 100644
--- a/source3/smbd/smb2_process.c
+++ b/source3/smbd/smb2_process.c
@@ -1706,8 +1706,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)
 {
@@ -1743,6 +1771,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)
 {
@@ -1751,6 +1803,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
@@ -1763,6 +1816,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()) {
 			/*
@@ -1810,7 +1864,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;
@@ -2055,6 +2115,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 472fb13e99bfab547b038fe60ca60f5f961ab556 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 2dee36ba583bd1b01376c64eb63cf57e39f25da8 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 cc2fe8979e8a..f97acafe44ea 100644
--- a/selftest/skip
+++ b/selftest/skip
@@ -148,3 +148,4 @@ bench # don't run benchmarks in our selftest
 ^samba4.smb2.mkdir.*\(ad_dc_ntvfs\)$ # Ignore ad_dc_ntvfs since this is a new test
 ^samba.tests.reparsepoints.*
 ^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 929294a2d477d0ce8a0e76b00e19d1ccc3ad9a7e 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 8357d1b84318a3e0cda4f4c4ff06a32b697ac83f 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 5f55ff658bb6..0dac04450fe2 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 8634dc8a0b712004763352aa0ad9bbc9cdd6c09c 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 0dac04450fe2..dfb87dd3775c 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