From 1cd6d09fc24ab766a6296de2896d517ad42a0c40 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 | 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 cfca26359c3..cfdfaa98c84 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -966,6 +966,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 640179ba46e4d64178feea3c363dc1cb9a61b6f3 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 7c9a0c8f4b175f715cbf801a0455c6717f2ad701 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                 | 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 3169c8d5487..68154a61ab5 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