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