Attachment 18112 Details for Bug 15423 – git-am fix for 4.19.next, 4.18.next.

[patch] git-am fix for 4.19.next, 4.18.next.

bug-15423-4.19.patch (text/plain), 14.77 KB, created by Jeremy Allison on 2023-09-20 17:09:36 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2023-09-20 17:09:36 UTC

Size: 14.77 KB

patch

obsolete

>From a88f0e7f38670966bf1553fa5036d650b74c666a Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Mon, 18 Sep 2023 14:43:23 -0700
>Subject: [PATCH 1/5] s3: smbd: Add some DEVELOPER-only code to panic if the
> destructor for an aio_lnk is called and the associated fsp doesn't exist.
>
>Make this DEVELOPER-only as it walks the entire open
>file list on every file close (with associated aio).
>
>This helps catch really subtle problems with orphaned
>aio lnk structs.
>
>Reproducer test case to follow.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15423
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit 82e88f70f181300f6f98691f6680839a94470e13)
>---
> source3/smbd/smb2_aio.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
>diff --git a/source3/smbd/smb2_aio.c b/source3/smbd/smb2_aio.c
>index 8c01c76a3e9..88aa68d218f 100644
>--- a/source3/smbd/smb2_aio.c
>+++ b/source3/smbd/smb2_aio.c
>@@ -64,6 +64,9 @@ struct aio_extra *create_aio_extra(TALLOC_CTX *mem_ctx,
> }
> 
> struct aio_req_fsp_link {
>+#ifdef DEVELOPER
>+	struct smbd_server_connection *sconn;
>+#endif
> 	files_struct *fsp;
> 	struct tevent_req *req;
> };
>@@ -74,6 +77,24 @@ static int aio_del_req_from_fsp(struct aio_req_fsp_link *lnk)
> 	files_struct *fsp = lnk->fsp;
> 	struct tevent_req *req = lnk->req;
> 
>+#ifdef DEVELOPER
>+	struct files_struct *ifsp = NULL;
>+	bool found = false;
>+
>+	/*
>+	 * When this is called, lnk->fsp must still exist
>+	 * on the files list for this connection. Panic if not.
>+	 */
>+	for (ifsp = lnk->sconn->files; ifsp; ifsp = ifsp->next) {
>+		if (ifsp == fsp) {
>+			found = true;
>+		}
>+	}
>+	if (!found) {
>+		smb_panic("orphaned lnk on fsp aio list.\n");
>+	}
>+#endif
>+
> 	for (i=0; i<fsp->num_aio_requests; i++) {
> 		if (fsp->aio_requests[i] == req) {
> 			break;
>@@ -130,6 +151,9 @@ bool aio_add_req_to_fsp(files_struct *fsp, struct tevent_req *req)
> 
> 	lnk->fsp = fsp;
> 	lnk->req = req;
>+#ifdef DEVELOPER
>+	lnk->sconn = fsp->conn->sconn;
>+#endif
> 	talloc_set_destructor(lnk, aio_del_req_from_fsp);
> 
> 	return true;
>-- 
>2.39.3
>
>
>From f6e8e599a58d17dede3fc09a20258b0cd263df76 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Mon, 18 Sep 2023 17:09:00 -0700
>Subject: [PATCH 2/5] s3: smbd: named pipe reads are async. Use the same logic
> as for named pipe transacts to avoid crashes on shutdown.
>
>Noticed by Metze.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15423
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit 3f32bf887d4425655e81da0b2234cbca3b1d56e6)
>---
> source3/smbd/smb2_read.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
>diff --git a/source3/smbd/smb2_read.c b/source3/smbd/smb2_read.c
>index 0101c42cf76..bac78e4c77a 100644
>--- a/source3/smbd/smb2_read.c
>+++ b/source3/smbd/smb2_read.c
>@@ -494,6 +494,7 @@ static struct tevent_req *smbd_smb2_read_send(TALLOC_CTX *mem_ctx,
> 
> 	if (IS_IPC(smbreq->conn)) {
> 		struct tevent_req *subreq = NULL;
>+		bool ok;
> 
> 		state->out_data = data_blob_talloc(state, NULL, in_length);
> 		if (in_length > 0 && tevent_req_nomem(state->out_data.data, req)) {
>@@ -515,6 +516,18 @@ static struct tevent_req *smbd_smb2_read_send(TALLOC_CTX *mem_ctx,
> 		tevent_req_set_callback(subreq,
> 					smbd_smb2_read_pipe_done,
> 					req);
>+
>+		/*
>+		 * Make sure we mark the fsp as having outstanding async
>+		 * activity so we don't crash on shutdown close.
>+		 */
>+
>+		ok = aio_add_req_to_fsp(fsp, req);
>+		if (!ok) {
>+			tevent_req_nterror(req, NT_STATUS_NO_MEMORY);
>+			return tevent_req_post(req, ev);
>+		}
>+
> 		return req;
> 	}
> 
>-- 
>2.39.3
>
>
>From b1c55e1432ad42af21e756787b3ec6a235ff7f47 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Mon, 18 Sep 2023 17:37:44 -0700
>Subject: [PATCH 3/5] s3: smbd: named pipe writes are async. Use the same logic
> as for named pipe transacts to avoid crashes on shutdown.
>
>Noticed by Metze.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15423
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit ea062c3b0d4dbb1f0682f808ac893bf36a6fb194)
>---
> source3/smbd/smb2_write.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
>diff --git a/source3/smbd/smb2_write.c b/source3/smbd/smb2_write.c
>index 269c489642e..a7af20173a1 100644
>--- a/source3/smbd/smb2_write.c
>+++ b/source3/smbd/smb2_write.c
>@@ -307,6 +307,7 @@ static struct tevent_req *smbd_smb2_write_send(TALLOC_CTX *mem_ctx,
> 
> 	if (IS_IPC(smbreq->conn)) {
> 		struct tevent_req *subreq = NULL;
>+                bool ok;
> 
> 		if (!fsp_is_np(fsp)) {
> 			tevent_req_nterror(req, NT_STATUS_FILE_CLOSED);
>@@ -323,6 +324,18 @@ static struct tevent_req *smbd_smb2_write_send(TALLOC_CTX *mem_ctx,
> 		tevent_req_set_callback(subreq,
> 					smbd_smb2_write_pipe_done,
> 					req);
>+
>+		/*
>+		 * Make sure we mark the fsp as having outstanding async
>+		 * activity so we don't crash on shutdown close.
>+		 */
>+
>+		ok = aio_add_req_to_fsp(fsp, req);
>+		if (!ok) {
>+			tevent_req_nterror(req, NT_STATUS_NO_MEMORY);
>+			return tevent_req_post(req, ev);
>+		}
>+
> 		return req;
> 	}
> 
>-- 
>2.39.3
>
>
>From 5365769e354a90a4d17cfef96026094974c1296b Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 19 Sep 2023 14:30:26 -0700
>Subject: [PATCH 4/5] s3: torture: Add a new SMB2 test:
> SMB2-PIPE-READ-ASYNC-DISCONNECT
>
>Shows the server crashes if we open a named pipe, do an async read
>and then disconnect.
>
>Adds knownfail:
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15423
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit 66398dd03c46633b474438dddb771caa2d245e64)
>---
> .../smb2_pipe_read_async_disconnect           |   1 +
> .../tests/test_smbtorture_nocrash_s3.sh       |  38 ++++++
> source3/selftest/tests.py                     |  16 +++
> source3/torture/proto.h                       |   1 +
> source3/torture/test_smb2.c                   | 117 ++++++++++++++++++
> source3/torture/torture.c                     |   4 +
> 6 files changed, 177 insertions(+)
> create mode 100644 selftest/knownfail.d/smb2_pipe_read_async_disconnect
> create mode 100755 source3/script/tests/test_smbtorture_nocrash_s3.sh
>
>diff --git a/selftest/knownfail.d/smb2_pipe_read_async_disconnect b/selftest/knownfail.d/smb2_pipe_read_async_disconnect
>new file mode 100644
>index 00000000000..342a308adec
>--- /dev/null
>+++ b/selftest/knownfail.d/smb2_pipe_read_async_disconnect
>@@ -0,0 +1 @@
>+^samba3.smbtorture_s3.plain.SMB2-PIPE-READ-ASYNC-DISCONNECT.check_panic$fileserver$
>diff --git a/source3/script/tests/test_smbtorture_nocrash_s3.sh b/source3/script/tests/test_smbtorture_nocrash_s3.sh
>new file mode 100755
>index 00000000000..b6ef1391262
>--- /dev/null
>+++ b/source3/script/tests/test_smbtorture_nocrash_s3.sh
>@@ -0,0 +1,38 @@
>+#!/bin/sh
>+
>+. $(dirname $0)/../../../testprogs/blackbox/subunit.sh
>+
>+# this runs the file serving tests that are expected to pass with samba3
>+
>+if [ $# -lt 5 ]; then
>+	cat <<EOF
>+Usage: test_smbtorture_s3.sh TEST UNC USERNAME PASSWORD SMBTORTURE <smbtorture args>
>+EOF
>+	exit 1
>+fi
>+
>+t="$1"
>+unc="$2"
>+username="$3"
>+password="$4"
>+SMBTORTURE="$5"
>+shift 5
>+ADDARGS="$*"
>+
>+incdir=$(dirname $0)/../../../testprogs/blackbox
>+. $incdir/subunit.sh
>+
>+panic_count_0=$(grep -c PANIC $SMBD_TEST_LOG)
>+
>+echo "$panic_count_0" >/tmp/look
>+
>+failed=0
>+testit "smbtorture" $VALGRIND $SMBTORTURE $unc -U"$username"%"$password" $ADDARGS $t || failed=$(expr $failed + 1)
>+
>+panic_count_1=$(grep -c PANIC $SMBD_TEST_LOG)
>+
>+echo "$panic_count_1" >>/tmp/look
>+
>+testit "check_panic" test $panic_count_0 -eq $panic_count_1 || failed=$(expr $failed + 1)
>+
>+testok $0 $failed
>diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>index 783cb486012..8986a73f03a 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -481,6 +481,22 @@ plantestsuite("samba3.smbtorture_s3.plain.%s" % "SMB2-DEL-ON-CLOSE-NONWRITE-DELE
>                 "",
>                 "-l $LOCAL_PATH"])
> 
>+#
>+# Test doing an async read + disconnect on a pipe doesn't crash the server.
>+# BUG: https://bugzilla.samba.org/show_bug.cgi?id=15423
>+#
>+plantestsuite("samba3.smbtorture_s3.plain.%s" % "SMB2-PIPE-READ-ASYNC-DISCONNECT",
>+                "fileserver",
>+                [os.path.join(samba3srcdir,
>+                              "script/tests/test_smbtorture_nocrash_s3.sh"),
>+                'SMB2-PIPE-READ-ASYNC-DISCONNECT',
>+                '//$SERVER_IP/tmp',
>+                '$USERNAME',
>+                '$PASSWORD',
>+                smbtorture3,
>+                "",
>+                "-l $LOCAL_PATH"])
>+
> shares = [
>     "vfs_aio_pthread_async_dosmode_default1",
>     "vfs_aio_pthread_async_dosmode_default2"
>diff --git a/source3/torture/proto.h b/source3/torture/proto.h
>index 93d0727e936..0206cf15c9a 100644
>--- a/source3/torture/proto.h
>+++ b/source3/torture/proto.h
>@@ -124,6 +124,7 @@ bool run_smb2_dfs_paths(int dummy);
> bool run_smb2_non_dfs_share(int dummy);
> bool run_smb2_dfs_share_non_dfs_path(int dummy);
> bool run_smb2_dfs_filename_leading_backslash(int dummy);
>+bool run_smb2_pipe_read_async_disconnect(int dummy);
> bool run_smb1_dfs_paths(int dummy);
> bool run_smb1_dfs_search_paths(int dummy);
> bool run_smb1_dfs_operations(int dummy);
>diff --git a/source3/torture/test_smb2.c b/source3/torture/test_smb2.c
>index 3cb6d13789a..672c38e2b55 100644
>--- a/source3/torture/test_smb2.c
>+++ b/source3/torture/test_smb2.c
>@@ -5136,3 +5136,120 @@ bool run_smb2_dfs_filename_leading_backslash(int dummy)
> 	(void)smb2_dfs_delete(cli, dfs_filename_slash);
> 	return retval;
> }
>+
>+/*
>+ * Ensure a named pipe async read followed by a disconnect
>+ * doesn't crash the server (server crash checked for in
>+ * containing test script:
>+ * source3/script/tests/test_smbtorture_nocrash_s3.sh)
>+ * BUG: https://bugzilla.samba.org/show_bug.cgi?id=15423
>+ */
>+
>+bool run_smb2_pipe_read_async_disconnect(int dummy)
>+{
>+	struct cli_state *cli = NULL;
>+	NTSTATUS status;
>+	uint64_t fid_persistent = 0;
>+	uint64_t fid_volatile = 0;
>+	struct tevent_context *ev;
>+	struct tevent_req *req;
>+	bool retval = false;
>+
>+	printf("Starting SMB2-PIPE-READ-ASYNC-DISCONNECT\n");
>+
>+	if (!torture_init_connection(&cli)) {
>+		return false;
>+	}
>+
>+	status = smbXcli_negprot(cli->conn,
>+				cli->timeout,
>+				PROTOCOL_SMB2_02,
>+				PROTOCOL_SMB3_11);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		printf("smbXcli_negprot returned %s\n", nt_errstr(status));
>+		return false;
>+	}
>+
>+	status = cli_session_setup_creds(cli, torture_creds);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		printf("cli_session_setup returned %s\n", nt_errstr(status));
>+		return false;
>+	}
>+
>+	status = cli_tree_connect_creds(cli, "IPC$", "IPC", torture_creds);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		printf("cli_tree_connect to IPC$ returned %s\n",
>+			nt_errstr(status));
>+		return false;
>+	}
>+
>+	/* Open the SAMR pipe. */
>+	status = smb2cli_create(cli->conn,
>+				cli->timeout,
>+				cli->smb2.session,
>+				cli->smb2.tcon,
>+				"SAMR",
>+				SMB2_OPLOCK_LEVEL_NONE, /* oplock_level, */
>+				SMB2_IMPERSONATION_IMPERSONATION, /* impersonation_level, */
>+				SEC_STD_SYNCHRONIZE|
>+					SEC_FILE_READ_DATA|
>+					SEC_FILE_WRITE_DATA, /* desired_access, */
>+				FILE_ATTRIBUTE_NORMAL, /* file_attributes, */
>+				FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access, */
>+				FILE_OPEN, /* create_disposition, */
>+				0, /* create_options, */
>+				NULL, /* smb2_create_blobs *blobs */
>+				&fid_persistent,
>+				&fid_volatile,
>+				NULL, /* struct smb_create_returns * */
>+				talloc_tos(), /* mem_ctx. */
>+				NULL, /* struct smb2_create_blobs * */
>+				NULL); /* psymlink */
>+	if (!NT_STATUS_IS_OK(status)) {
>+		printf("%s:%d smb2cli_create on SAMR returned %s\n",
>+			__FILE__,
>+			__LINE__,
>+			nt_errstr(status));
>+		goto err;
>+	}
>+
>+	ev = samba_tevent_context_init(talloc_tos());
>+	if (ev == NULL) {
>+		goto err;
>+	}
>+
>+	/* Start an async read. */
>+	req = smb2cli_read_send(talloc_tos(),
>+				ev,
>+				cli->conn,
>+				cli->timeout,
>+				cli->smb2.session,
>+				cli->smb2.tcon,
>+				16*1024,
>+				0, /* offset */
>+				fid_persistent,
>+				fid_volatile,
>+				0, /* minimum_count */
>+				0); /* remaining_bytes */
>+	if (req == NULL) {
>+		goto err;
>+	}
>+
>+	/* Force disconnect. */
>+	smbXcli_conn_disconnect(cli->conn, NT_STATUS_LOCAL_DISCONNECT);
>+	fid_volatile = 0;
>+	retval = true;
>+
>+  err:
>+
>+	if (fid_volatile != 0) {
>+		smb2cli_close(cli->conn,
>+			      cli->timeout,
>+			      cli->smb2.session,
>+			      cli->smb2.tcon,
>+			      0, /* flags */
>+			      fid_persistent,
>+			      fid_volatile);
>+	}
>+	return retval;
>+}
>diff --git a/source3/torture/torture.c b/source3/torture/torture.c
>index 2a8b8dec22f..9a1420c9e3a 100644
>--- a/source3/torture/torture.c
>+++ b/source3/torture/torture.c
>@@ -15763,6 +15763,10 @@ static struct {
> 		.name  = "SMB2-DFS-FILENAME-LEADING-BACKSLASH",
> 		.fn    = run_smb2_dfs_filename_leading_backslash,
> 	},
>+	{
>+		.name  = "SMB2-PIPE-READ-ASYNC-DISCONNECT",
>+		.fn    = run_smb2_pipe_read_async_disconnect,
>+	},
> 	{
> 		.name  = "SMB1-TRUNCATED-SESSSETUP",
> 		.fn    = run_smb1_truncated_sesssetup,
>-- 
>2.39.3
>
>
>From 9747b2e85da0c1613d3a98db23676d1df353eeaf Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 19 Sep 2023 14:36:45 -0700
>Subject: [PATCH 5/5] s3: smbd: Ensure we remove any pending aio values for
> named pipes on forced shutdown.
>
>Matches file and directory closes.
>
>Remove knownfail.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15423
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>
>Autobuild-User(master): Jeremy Allison <jra@samba.org>
>Autobuild-Date(master): Wed Sep 20 02:43:18 UTC 2023 on atb-devel-224
>
>(cherry picked from commit 11280f1705c0faa1729f5aeaa1b6a1f79ab5a199)
>---
> selftest/knownfail.d/smb2_pipe_read_async_disconnect | 1 -
> source3/smbd/close.c                                 | 8 ++++++++
> 2 files changed, 8 insertions(+), 1 deletion(-)
> delete mode 100644 selftest/knownfail.d/smb2_pipe_read_async_disconnect
>
>diff --git a/selftest/knownfail.d/smb2_pipe_read_async_disconnect b/selftest/knownfail.d/smb2_pipe_read_async_disconnect
>deleted file mode 100644
>index 342a308adec..00000000000
>--- a/selftest/knownfail.d/smb2_pipe_read_async_disconnect
>+++ /dev/null
>@@ -1 +0,0 @@
>-^samba3.smbtorture_s3.plain.SMB2-PIPE-READ-ASYNC-DISCONNECT.check_panic$fileserver$
>diff --git a/source3/smbd/close.c b/source3/smbd/close.c
>index 2b180e0c718..af5e78daa10 100644
>--- a/source3/smbd/close.c
>+++ b/source3/smbd/close.c
>@@ -1630,6 +1630,14 @@ NTSTATUS close_file_smb(struct smb_request *req,
> 	SMB_ASSERT(fsp->stream_fsp == NULL);
> 
> 	if (fsp->fake_file_handle != NULL) {
>+		/*
>+		 * Named pipes are opened as fake files and
>+		 * can have pending aio requests. Ensure
>+		 * we clear out all pending aio on force
>+		 * shutdown of named pipes also.
>+		 * BUG: https://bugzilla.samba.org/show_bug.cgi?id=15423
>+		 */
>+		assert_no_pending_aio(fsp, close_type);
> 		status = close_fake_file(req, fsp);
> 	} else if (fsp->print_file != NULL) {
> 		/* FIXME: return spool errors */
>-- 
>2.39.3
>

Flags:

slow: review+

Actions: View

Attachments on bug 15423: 17988 | 18112