The Samba-Bugzilla – Attachment 16091 Details for
Bug 14301
smbd panic on force-close share during async io
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am fix for 4.12.next.
bug-14301-4.12.patch (text/plain), 9.04 KB, created by
Jeremy Allison
on 2020-06-25 22:07:26 UTC
(
hide
)
Description:
git-am fix for 4.12.next.
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2020-06-25 22:07:26 UTC
Size:
9.04 KB
patch
obsolete
>From 29e93ed4cab2a3e92b082b5a07d5e3f5fb279a64 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Mon, 22 Jun 2020 13:44:12 -0700 >Subject: [PATCH 1/3] s3: selftest: Add samba3.blackbox.aio-outstanding test. > >Shows smbd panics if connection is terminated (torn down) >by killing the client with outstanding aio requests in the >queue. As we're closing smbd we should cope with this. > >Followup-bugfix for: > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit f206d37f6ec8143b2051a8fe15783c188344adbe) >--- > selftest/knownfail.d/aio_outstanding | 2 + > source3/script/tests/test_aio_outstanding.sh | 92 ++++++++++++++++++++ > source3/selftest/tests.py | 8 ++ > 3 files changed, 102 insertions(+) > create mode 100644 selftest/knownfail.d/aio_outstanding > create mode 100755 source3/script/tests/test_aio_outstanding.sh > >diff --git a/selftest/knownfail.d/aio_outstanding b/selftest/knownfail.d/aio_outstanding >new file mode 100644 >index 00000000000..6426f760cb1 >--- /dev/null >+++ b/selftest/knownfail.d/aio_outstanding >@@ -0,0 +1,2 @@ >+samba3.blackbox.aio-outstanding >+ >diff --git a/source3/script/tests/test_aio_outstanding.sh b/source3/script/tests/test_aio_outstanding.sh >new file mode 100755 >index 00000000000..b189343bb24 >--- /dev/null >+++ b/source3/script/tests/test_aio_outstanding.sh >@@ -0,0 +1,92 @@ >+#!/bin/bash >+# >+# Test terminating an smbclient connection with outstanding >+# aio requests. >+# >+# Note this is designed to be run against >+# the aio_delay_inject share which is preconfigured >+# with 2 second delays on pread/pwrite. >+ >+if [ $# -lt 4 ]; then >+ echo Usage: test_aio_outstanding.sh \ >+ SERVERCONFFILE SMBCLIENT IP aio_delay_inject_sharename >+exit 1 >+fi >+ >+CONF=$1 >+SMBCLIENT=$2 >+SERVER=$3 >+SHARE=$4 >+ >+incdir=$(dirname $0)/../../../testprogs/blackbox >+. $incdir/subunit.sh >+ >+failed=0 >+# >+# Note if we already have any panics in the smbd log. >+# >+panic_count_0=$(grep -c PANIC $SMBD_TEST_LOG) >+ >+# Create the smbclient communication pipes. >+rm -f smbclient-stdin smbclient-stdout smbclient-stderr >+mkfifo smbclient-stdin smbclient-stdout smbclient-stderr >+ >+# Create a large-ish testfile >+rm aio_outstanding_testfile >+head -c 20MB /dev/zero >aio_outstanding_testfile >+ >+CLI_FORCE_INTERACTIVE=1; export CLI_FORCE_INTERACTIVE >+ >+${SMBCLIENT} //${SERVER}/${SHARE} ${CONF} -U${USER}%${PASSWORD} \ >+ < smbclient-stdin > smbclient-stdout 2>smbclient-stderr & >+CLIENT_PID=$! >+ >+sleep 1 >+ >+exec 100>smbclient-stdin 101<smbclient-stdout 102<smbclient-stderr >+ >+# consume the smbclient startup messages >+head -n 1 <&101 >+head -n 1 <&102 >+ >+# Ensure we're putting a fresh file. >+echo "del aio_outstanding_testfile" >&100 >+echo "put aio_outstanding_testfile" >&100 >+ >+sleep 2 >+ >+# Terminate the smbclient write to the aio_delay_inject share whilst >+# we have outstanding writes. >+kill $CLIENT_PID >+ >+sleep 1 >+ >+# Ensure the panic count didn't change. >+# >+# BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 >+# >+ >+panic_count_1=$(grep -c PANIC $SMBD_TEST_LOG) >+ >+# Rerun smbclient to remove the testfile on the server. >+rm -f smbclient-stdin smbclient-stdout smbclient-stderr aio_outstanding_testfile >+mkfifo smbclient-stdin smbclient-stdout >+ >+${SMBCLIENT} //${SERVER}/${SHARE} ${CONF} -U${USER}%${PASSWORD} \ >+ < smbclient-stdin > smbclient-stdout & >+ >+sleep 1 >+ >+exec 100>smbclient-stdin 101<smbclient-stdout >+ >+echo "del aio_outstanding_testfile" >&100 >+echo "exit" >&100 >+ >+sleep 2 >+ >+rm -f smbclient-stdin smbclient-stdout aio_outstanding_testfile >+ >+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 005d6f453b1..dc44160e50d 100755 >--- a/source3/selftest/tests.py >+++ b/source3/selftest/tests.py >@@ -865,6 +865,14 @@ plantestsuite("samba3.blackbox.net_tdb", "simpleserver:local", > smbclient3, '$SERVER', 'tmp', '$USERNAME', '$PASSWORD', > configuration, '$LOCAL_PATH', '$LOCK_DIR']) > >+plantestsuite("samba3.blackbox.aio-outstanding", "simpleserver:local", >+ [os.path.join(samba3srcdir, >+ "script/tests/test_aio_outstanding.sh"), >+ configuration, >+ os.path.join(bindir(), "smbclient"), >+ '$SERVER_IP', >+ "aio_delay_inject"]) >+ > plantestsuite("samba3.blackbox.smbd_error", "simpleserver:local", > [os.path.join(samba3srcdir, "script/tests/test_smbd_error.sh")]) > >-- >2.20.1 > > >From 4bf6e8296b3c7e2d81798a687195780cb4c26e12 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 25 Jun 2020 14:56:03 -0700 >Subject: [PATCH 2/3] s3: smbd: Add smbXsrv_client_valid_connections(). > >Next commit will make use of this. > >Followup-bugfix for: > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/smbd/globals.h | 1 + > source3/smbd/smb2_server.c | 14 ++++++++++++++ > 2 files changed, 15 insertions(+) > >diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h >index 7b26d04ed0f..5bcf0503b4c 100644 >--- a/source3/smbd/globals.h >+++ b/source3/smbd/globals.h >@@ -219,6 +219,7 @@ NTSTATUS smbd_calculate_access_mask(connection_struct *conn, > > void smbd_notify_cancel_by_smbreq(const struct smb_request *smbreq); > >+size_t smbXsrv_client_valid_connections(struct smbXsrv_client *client); > void smbd_server_connection_terminate_ex(struct smbXsrv_connection *xconn, > const char *reason, > const char *location); >diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c >index 88937ac19b0..f0fe14624cd 100644 >--- a/source3/smbd/smb2_server.c >+++ b/source3/smbd/smb2_server.c >@@ -1106,6 +1106,20 @@ static NTSTATUS smbd_smb2_request_setup_out(struct smbd_smb2_request *req) > return NT_STATUS_OK; > } > >+size_t smbXsrv_client_valid_connections(struct smbXsrv_client *client) >+{ >+ struct smbXsrv_connection *xconn = NULL; >+ size_t num_ok = 0; >+ >+ for (xconn = client->connections; xconn != NULL; xconn = xconn->next) { >+ if (NT_STATUS_IS_OK(xconn->transport.status)) { >+ num_ok++; >+ } >+ } >+ >+ return num_ok; >+} >+ > void smbd_server_connection_terminate_ex(struct smbXsrv_connection *xconn, > const char *reason, > const char *location) >-- >2.20.1 > > >From 507a0f50624dd03b2f9320f1e65c6c5f235949b8 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 16 Jun 2020 15:01:49 -0700 >Subject: [PATCH 3/3] s3: smbd: Allow a SHUTDOWN_CLOSE on a file with > outstanding aio if there are no client connections alive. > >The process is exiting now so pthreads will never complete to cause >problems. > >Remove the knownfail.d/aio_outstanding entry. > >Followup-bugfix for: > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> > >Autobuild-User(master): Jeremy Allison <jra@samba.org> >Autobuild-Date(master): Wed Jun 24 20:14:15 UTC 2020 on sn-devel-184 > >(cherry picked from commit 205653732064ecf76d3198451240af468806ec14) >--- > selftest/knownfail.d/aio_outstanding | 2 -- > source3/smbd/close.c | 26 ++++++++++++++++++++++++++ > 2 files changed, 26 insertions(+), 2 deletions(-) > delete mode 100644 selftest/knownfail.d/aio_outstanding > >diff --git a/selftest/knownfail.d/aio_outstanding b/selftest/knownfail.d/aio_outstanding >deleted file mode 100644 >index 6426f760cb1..00000000000 >--- a/selftest/knownfail.d/aio_outstanding >+++ /dev/null >@@ -1,2 +0,0 @@ >-samba3.blackbox.aio-outstanding >- >diff --git a/source3/smbd/close.c b/source3/smbd/close.c >index d5af62a277c..1a6e33b4403 100644 >--- a/source3/smbd/close.c >+++ b/source3/smbd/close.c >@@ -639,12 +639,38 @@ static NTSTATUS ntstatus_keeperror(NTSTATUS s1, NTSTATUS s2) > static void assert_no_pending_aio(struct files_struct *fsp, > enum file_close_type close_type) > { >+ struct smbXsrv_client *client = global_smbXsrv_client; >+ size_t num_connections_alive; > unsigned num_requests = fsp->num_aio_requests; > > if (num_requests == 0) { > return; > } > >+ num_connections_alive = smbXsrv_client_valid_connections(client); >+ >+ if (close_type == SHUTDOWN_CLOSE && num_connections_alive == 0) { >+ /* >+ * fsp->aio_requests and the contents (fsp->aio_requests[x]) >+ * are both independently owned by fsp and are not in a >+ * talloc heirarchy. This allows the fsp->aio_requests array to >+ * be reallocated independently of the array contents so it can >+ * grow on demand. >+ * >+ * This means we must ensure order of deallocation >+ * on a SHUTDOWN_CLOSE by deallocating the fsp->aio_requests[x] >+ * contents first, as their destructors access the >+ * fsp->aio_request array. If we don't deallocate them >+ * first, when fsp is deallocated fsp->aio_requests >+ * could have been deallocated *before* its contents >+ * fsp->aio_requests[x], causing a crash. >+ */ >+ while (fsp->num_aio_requests != 0) { >+ TALLOC_FREE(fsp->aio_requests[0]); >+ } >+ return; >+ } >+ > DBG_ERR("fsp->num_aio_requests=%u\n", num_requests); > smb_panic("can not close with outstanding aio requests"); > return; >-- >2.20.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
slow
:
review+
Actions:
View
Attachments on
bug 14301
:
15825
|
15837
|
15844
|
15847
|
15854
|
15858
|
15861
|
15867
|
15870
|
16043
|
16044
|
16054
|
16055
|
16064
|
16065
|
16069
| 16091