[SCM] Samba Shared Repository - branch v4-18-test updated

Jule Anger janger at samba.org
Wed Sep 20 21:39:02 UTC 2023


The branch, v4-18-test has been updated
       via  f869013c616 s3: smbd: Ensure we remove any pending aio values for named pipes on forced shutdown.
       via  db1fbcc0263 s3: torture: Add a new SMB2 test: SMB2-PIPE-READ-ASYNC-DISCONNECT
       via  721513a219d s3: smbd: named pipe writes are async. Use the same logic as for named pipe transacts to avoid crashes on shutdown.
       via  b3a881f89ae s3: smbd: named pipe reads are async. Use the same logic as for named pipe transacts to avoid crashes on shutdown.
       via  4baff9de6b2 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.
      from  82d6f8a6ce3 nsswitch/wb_common.c: fix socket fd and memory leaks of global state

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-18-test


- Log -----------------------------------------------------------------
commit f869013c616d706007f11ebfcaa0f8af4cadc230
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Sep 19 14:36:45 2023 -0700

    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 at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Wed Sep 20 02:43:18 UTC 2023 on atb-devel-224
    
    (cherry picked from commit 11280f1705c0faa1729f5aeaa1b6a1f79ab5a199)
    
    Autobuild-User(v4-18-test): Jule Anger <janger at samba.org>
    Autobuild-Date(v4-18-test): Wed Sep 20 21:38:55 UTC 2023 on atb-devel-224

commit db1fbcc0263d8ce3854a65bda5b54b38a9d4d66d
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Sep 19 14:30:26 2023 -0700

    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 at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    (cherry picked from commit 66398dd03c46633b474438dddb771caa2d245e64)

commit 721513a219d8b543a3f6a284d05ddc2c99717afe
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Sep 18 17:37:44 2023 -0700

    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 at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    (cherry picked from commit ea062c3b0d4dbb1f0682f808ac893bf36a6fb194)

commit b3a881f89ae089e3ec8e603e96eda5b1388e0cb0
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Sep 18 17:09:00 2023 -0700

    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 at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    (cherry picked from commit 3f32bf887d4425655e81da0b2234cbca3b1d56e6)

commit 4baff9de6b2dc93d81c25874c78f31f91b16d260
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Sep 18 14:43:23 2023 -0700

    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 at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    (cherry picked from commit 82e88f70f181300f6f98691f6680839a94470e13)

-----------------------------------------------------------------------

Summary of changes:
 ...torture_s3.sh => test_smbtorture_nocrash_s3.sh} |  12 +++
 source3/selftest/tests.py                          |  16 +++
 source3/smbd/close.c                               |   8 ++
 source3/smbd/smb2_aio.c                            |  24 +++++
 source3/smbd/smb2_read.c                           |  13 +++
 source3/smbd/smb2_write.c                          |  13 +++
 source3/torture/proto.h                            |   1 +
 source3/torture/test_smb2.c                        | 117 +++++++++++++++++++++
 source3/torture/torture.c                          |   4 +
 9 files changed, 208 insertions(+)
 copy source3/script/tests/{test_smbtorture_s3.sh => test_smbtorture_nocrash_s3.sh} (62%)


Changeset truncated at 500 lines:

diff --git a/source3/script/tests/test_smbtorture_s3.sh b/source3/script/tests/test_smbtorture_nocrash_s3.sh
similarity index 62%
copy from source3/script/tests/test_smbtorture_s3.sh
copy to source3/script/tests/test_smbtorture_nocrash_s3.sh
index 4376f4a7199..b6ef1391262 100755
--- a/source3/script/tests/test_smbtorture_s3.sh
+++ b/source3/script/tests/test_smbtorture_nocrash_s3.sh
@@ -1,5 +1,7 @@
 #!/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
@@ -20,7 +22,17 @@ 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 ea17ead3eda..1fdcad1089f 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -479,6 +479,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/smbd/close.c b/source3/smbd/close.c
index d1e89325780..b42f4d2db71 100644
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -1634,6 +1634,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 */
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;
diff --git a/source3/smbd/smb2_read.c b/source3/smbd/smb2_read.c
index ff945302f54..5dd6f3de540 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;
 	}
 
diff --git a/source3/smbd/smb2_write.c b/source3/smbd/smb2_write.c
index ff99127b067..9fa87765b8f 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;
 	}
 
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index 5bfe4cc8e02..21d7b3e00a7 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 dc249643aa6..269ade4ef61 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 14a0347882b..1315b328f5f 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,


-- 
Samba Shared Repository



More information about the samba-cvs mailing list