[SCM] Samba Shared Repository - branch master updated
Jeremy Allison
jra at samba.org
Wed Sep 20 02:44:02 UTC 2023
The branch, master has been updated
via 11280f1705c s3: smbd: Ensure we remove any pending aio values for named pipes on forced shutdown.
via 66398dd03c4 s3: torture: Add a new SMB2 test: SMB2-PIPE-READ-ASYNC-DISCONNECT
via ea062c3b0d4 s3: smbd: named pipe writes are async. Use the same logic as for named pipe transacts to avoid crashes on shutdown.
via 3f32bf887d4 s3: smbd: named pipe reads are async. Use the same logic as for named pipe transacts to avoid crashes on shutdown.
via 82e88f70f18 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 05291d2bd40 s3: smbd: Now we have proved hardlink_internals() doesn't use src_dirfsp and dst_dirfsp, remove the parameters.
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 11280f1705c0faa1729f5aeaa1b6a1f79ab5a199
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
commit 66398dd03c46633b474438dddb771caa2d245e64
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>
commit ea062c3b0d4dbb1f0682f808ac893bf36a6fb194
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>
commit 3f32bf887d4425655e81da0b2234cbca3b1d56e6
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>
commit 82e88f70f181300f6f98691f6680839a94470e13
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>
-----------------------------------------------------------------------
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 5fece702372..4b71ba1a1de 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/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 */
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 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;
}
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;
}
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index 071e0d45103..17426458330 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -132,6 +132,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 486fb4c8115..3598e301174 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -15843,6 +15843,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