[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Wed Jun 24 20:15:04 UTC 2020


The branch, master has been updated
       via  20565373206 s3: smbd: Allow a SHUTDOWN_CLOSE on a file with outstanding aio if there are no client connections alive.
       via  c7a4a7f103a s3: smbd: Make smbXsrv_client_valid_connections() external.
       via  f206d37f6ec s3: selftest: Add samba3.blackbox.aio-outstanding test.
      from  ab50d348d93 gpo: Test samba-tool gpo admxload

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 205653732064ecf76d3198451240af468806ec14
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Jun 16 15:01:49 2020 -0700

    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 at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Wed Jun 24 20:14:15 UTC 2020 on sn-devel-184

commit c7a4a7f103a29b84ebae8519fa7cc25c836308b9
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Jun 16 14:58:54 2020 -0700

    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 at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit f206d37f6ec8143b2051a8fe15783c188344adbe
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Jun 22 13:44:12 2020 -0700

    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 at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 ...orce_close_share.sh => test_aio_outstanding.sh} | 64 ++++++++++------------
 source3/selftest/tests.py                          |  8 +++
 source3/smbd/close.c                               | 26 +++++++++
 source3/smbd/globals.h                             |  1 +
 source3/smbd/smb2_server.c                         |  2 +-
 5 files changed, 64 insertions(+), 37 deletions(-)
 copy source3/script/tests/{test_force_close_share.sh => test_aio_outstanding.sh} (50%)


Changeset truncated at 500 lines:

diff --git a/source3/script/tests/test_force_close_share.sh b/source3/script/tests/test_aio_outstanding.sh
similarity index 50%
copy from source3/script/tests/test_force_close_share.sh
copy to source3/script/tests/test_aio_outstanding.sh
index da78b5a752e..b189343bb24 100755
--- a/source3/script/tests/test_force_close_share.sh
+++ b/source3/script/tests/test_aio_outstanding.sh
@@ -1,38 +1,39 @@
 #!/bin/bash
 #
-# Test smbcontrol close-share command.
-#
-# Copyright (C) 2020 Volker Lendecke
-# Copyright (C) 2020 Jeremy Allison
+# 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 5 ]; then
-    echo Usage: test_share_force_close.sh \
-	 SERVERCONFFILE SMBCLIENT SMBCONTROL IP aio_delay_inject_sharename
+if [ $# -lt 4 ]; then
+    echo Usage: test_aio_outstanding.sh \
+	 SERVERCONFFILE SMBCLIENT IP aio_delay_inject_sharename
 exit 1
 fi
 
 CONF=$1
 SMBCLIENT=$2
-SMBCONTROL=$3
-SERVER=$4
-SHARE=$5
+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
+rm aio_outstanding_testfile
+head -c 20MB /dev/zero >aio_outstanding_testfile
 
 CLI_FORCE_INTERACTIVE=1; export CLI_FORCE_INTERACTIVE
 
@@ -49,52 +50,43 @@ head -n 1 <&101
 head -n 1 <&102
 
 # Ensure we're putting a fresh file.
-echo "del testfile" >&100
-echo "put testfile" >&100
-
-sleep 1
+echo "del aio_outstanding_testfile" >&100
+echo "put aio_outstanding_testfile" >&100
 
-# Close the aio_delay_inject share whilst we have outstanding writes.
+sleep 2
 
-testit "smbcontrol" ${SMBCONTROL} ${CONF} smbd close-share ${SHARE} ||
-    failed=$(expr $failed + 1)
+# Terminate the smbclient write to the aio_delay_inject share whilst
+# we have outstanding writes.
+kill $CLIENT_PID
 
 sleep 1
 
-# If we get one or more NT_STATUS_NETWORK_NAME_DELETED
-# or NT_STATUS_INVALID_HANDLE on stderr from the writes we
-# know the server stayed up and didn't crash when the
-# close-share removed the share.
+# Ensure the panic count didn't change.
 #
 # BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301
 #
-COUNT=$(head -n 2 <&102 |
-	    grep -e NT_STATUS_NETWORK_NAME_DELETED -e NT_STATUS_INVALID_HANDLE |
-	    wc -l)
-
-testit "Verify close-share did cancel the file put" \
-       test $COUNT -ge 1 || failed=$(expr $failed + 1)
 
-kill ${CLIENT_PID}
+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
+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 &
-CLIENT_PID=$!
 
 sleep 1
 
 exec 100>smbclient-stdin  101<smbclient-stdout
 
-echo "del testfile" >&100
+echo "del aio_outstanding_testfile" >&100
+echo "exit" >&100
 
-sleep 1
+sleep 2
 
-kill ${CLIENT_PID}
+rm -f smbclient-stdin smbclient-stdout aio_outstanding_testfile
 
-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..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")])
 
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;
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;


-- 
Samba Shared Repository



More information about the samba-cvs mailing list