Attachment 16064 Details for Bug 14301 – git-am supplemental fix for master.

[patch] git-am supplemental fix for master.

14301-extra.master (text/plain), 8.00 KB, created by Jeremy Allison on 2020-06-22 22:10:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2020-06-22 22:10:51 UTC

Size: 8.00 KB

patch

obsolete

>From 011cc775f87eb82167db2afb62b8e1b544d24c6a 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>
>---
> selftest/knownfail.d/aio_outstanding         |  2 +
> source3/script/tests/test_aio_outstanding.sh | 94 ++++++++++++++++++++
> source3/selftest/tests.py                    |  8 ++
> 3 files changed, 104 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..6a947e15449
>--- /dev/null
>+++ b/source3/script/tests/test_aio_outstanding.sh
>@@ -0,0 +1,94 @@
>+#!/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 testfile
>+head -c 20MB /dev/zero >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 testfile" >&100
>+echo "put 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 testfile
>+mkfifo smbclient-stdin smbclient-stdout
>+
>+${SMBCLIENT} //${SERVER}/${SHARE} ${CONF} -U${USER}%${PASSWORD} \
>+	     < smbclient-stdin > smbclient-stdout &
>+CLIENT_PID=$!
>+
>+sleep 1
>+
>+exec 100>smbclient-stdin  101<smbclient-stdout
>+
>+echo "del testfile" >&100
>+
>+sleep 1
>+
>+kill ${CLIENT_PID}
>+
>+rm -f smbclient-stdin smbclient-stdout 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 cfca26359c3..e7794fac71c 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -943,6 +943,14 @@ plantestsuite("samba3.blackbox.force-close-share", "simpleserver:local",
>                '$SERVER_IP',
>                "aio_delay_inject"])
> 
>+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.open-eintr", "simpleserver:local",
>               [os.path.join(samba3srcdir,
>                             "script/tests/test_open_eintr.sh"),
>-- 
>2.20.1
>
>
>From 97df2cabef71266e2dea90d71110b3f01bdb84dd Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 16 Jun 2020 14:58:54 -0700
>Subject: [PATCH 2/3] s3: smbd: Make smbXsrv_client_valid_connections()
> external.
>
>We will need to this ensure our client connections are
>terminated in close_file before exiting with outstanding
>aio.
>
>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 | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
>index d3b6ac2ffe6..2a963439bef 100644
>--- a/source3/smbd/globals.h
>+++ b/source3/smbd/globals.h
>@@ -222,6 +222,7 @@ void smbd_notify_cancel_by_smbreq(const struct smb_request *smbreq);
> 
> void smbXsrv_connection_disconnect_transport(struct smbXsrv_connection *xconn,
> 					     NTSTATUS status);
>+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 0a5083b5b8f..b58d3fbf097 100644
>--- a/source3/smbd/smb2_server.c
>+++ b/source3/smbd/smb2_server.c
>@@ -1121,7 +1121,7 @@ void smbXsrv_connection_disconnect_transport(struct smbXsrv_connection *xconn,
> 	DO_PROFILE_INC(disconnect);
> }
> 
>-static size_t smbXsrv_client_valid_connections(struct smbXsrv_client *client)
>+size_t smbXsrv_client_valid_connections(struct smbXsrv_client *client)
> {
> 	struct smbXsrv_connection *xconn = NULL;
> 	size_t num_ok = 0;
>-- 
>2.20.1
>
>
>From 4a1bf9ed57e9231a03d56f4ef15d6b12c497eda4 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>
>---
> selftest/knownfail.d/aio_outstanding |  2 --
> source3/smbd/close.c                 | 17 +++++++++++++++++
> 2 files changed, 17 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 3169c8d5487..e1491f8e6d0 100644
>--- a/source3/smbd/close.c
>+++ b/source3/smbd/close.c
>@@ -639,12 +639,29 @@ 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) {
>+		/*
>+		 * Ensure we deallocate here, as fsp->aio_requests[0]
>+		 * is owned by fsp, not fsp->aio_requests.
>+		 * fsp_aio_requests[x] gets orphaned if we don't
>+		 * delete here.
>+		 */
>+		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
>

Actions: View

Attachments on bug 14301: 15825 | 15837 | 15844 | 15847 | 15854 | 15858 | 15861 | 15867 | 15870 | 16043 | 16044 | 16054 | 16055 | 16064 | 16065 | 16069 | 16091