[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Thu Feb 20 22:15:02 UTC 2020


The branch, master has been updated
       via  aebe427b77b smbd: Separate aio_pthread indicator from normal EINTR
       via  4a943d842a5 lib: Map EINPROGRESS->NT_STATUS_MORE_PROCESSING_REQUIRED
       via  7bbba73b30f test: Show that smbd does not handle EINTR from open() correctly
       via  305204a241b test: Intercept open in vfs_error_inject
      from  8441471d5dc autobuild: samba-ctdb does not need an AD DC

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


- Log -----------------------------------------------------------------
commit aebe427b77b5315eb5d2b05b8c72824ca0389723
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Feb 20 14:13:35 2020 +0100

    smbd: Separate aio_pthread indicator from normal EINTR
    
    According to Posix and the Linux open(2) manpage, the open-syscall can
    return EINTR. If that happens, core smbd saw this as an indication
    that aio_pthread's open function was doing its job. With a real EINTR
    without aio_pthread this meant we ended up in a server_exit after 20
    seconds, because there was nobody to do the retry.
    
    EINTR is mapped to NT_STATUS_RETRY. Handle this by just retrying after
    a second.
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=14285
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Thu Feb 20 22:14:25 UTC 2020 on sn-devel-184

commit 4a943d842a51674425f0c4019f823ef0a9d09f49
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Feb 20 10:25:16 2020 +0100

    lib: Map EINPROGRESS->NT_STATUS_MORE_PROCESSING_REQUIRED
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=14285
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit 7bbba73b30f06304e9a2ad48e853d9ec8171dd30
Author: Volker Lendecke <vl at samba.org>
Date:   Wed Feb 19 15:25:38 2020 +0100

    test: Show that smbd does not handle EINTR from open() correctly
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=14285
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit 305204a241b74c599f4f6a064cac6608afd9c893
Author: Volker Lendecke <vl at samba.org>
Date:   Wed Feb 19 14:44:11 2020 +0100

    test: Intercept open in vfs_error_inject
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=14285
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

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

Summary of changes:
 source3/lib/errmap_unix.c               |  1 +
 source3/modules/vfs_aio_pthread.c       |  2 +-
 source3/modules/vfs_error_inject.c      | 17 +++++++++
 source3/script/tests/test_open_eintr.sh | 66 +++++++++++++++++++++++++++++++++
 source3/selftest/tests.py               |  9 +++++
 source3/smbd/open.c                     | 34 ++++++++++++-----
 6 files changed, 119 insertions(+), 10 deletions(-)
 create mode 100755 source3/script/tests/test_open_eintr.sh


Changeset truncated at 500 lines:

diff --git a/source3/lib/errmap_unix.c b/source3/lib/errmap_unix.c
index 5a6dd245023..33b48cadcb5 100644
--- a/source3/lib/errmap_unix.c
+++ b/source3/lib/errmap_unix.c
@@ -118,6 +118,7 @@ static const struct {
 #ifdef EOVERFLOW
 	{ EOVERFLOW,      NT_STATUS_ALLOTTED_SPACE_EXCEEDED },
 #endif
+	{ EINPROGRESS,	NT_STATUS_MORE_PROCESSING_REQUIRED },
 };
 
 /*********************************************************************
diff --git a/source3/modules/vfs_aio_pthread.c b/source3/modules/vfs_aio_pthread.c
index a7d97223dbd..d13ce2fdc63 100644
--- a/source3/modules/vfs_aio_pthread.c
+++ b/source3/modules/vfs_aio_pthread.c
@@ -318,7 +318,7 @@ static int open_async(const files_struct *fsp,
 		opd->fname));
 
 	/* Cause the calling code to reschedule us. */
-	errno = EINTR; /* Maps to NT_STATUS_RETRY. */
+	errno = EINPROGRESS; /* Maps to NT_STATUS_MORE_PROCESSING_REQUIRED. */
 	return -1;
 }
 
diff --git a/source3/modules/vfs_error_inject.c b/source3/modules/vfs_error_inject.c
index c8c3ea4701f..c7a99370b2a 100644
--- a/source3/modules/vfs_error_inject.c
+++ b/source3/modules/vfs_error_inject.c
@@ -29,6 +29,7 @@ struct unix_error_map {
 } unix_error_map_array[] = {
 	{	"ESTALE",	ESTALE	},
 	{	"EBADF",	EBADF	},
+	{	"EINTR",	EINTR	},
 };
 
 static int find_unix_error_from_string(const char *err_str)
@@ -106,9 +107,25 @@ static ssize_t vfs_error_inject_pwrite(vfs_handle_struct *handle,
 	return SMB_VFS_NEXT_PWRITE(handle, fsp, data, n, offset);
 }
 
+static int vfs_error_inject_open(
+	struct vfs_handle_struct *handle,
+	struct smb_filename *smb_fname,
+	files_struct *fsp,
+	int flags,
+	mode_t mode)
+{
+	int error = inject_unix_error("open", handle);
+	if (error != 0) {
+		errno = error;
+		return -1;
+	}
+	return SMB_VFS_NEXT_OPEN(handle, smb_fname, fsp, flags, mode);
+}
+
 static struct vfs_fn_pointers vfs_error_inject_fns = {
 	.chdir_fn = vfs_error_inject_chdir,
 	.pwrite_fn = vfs_error_inject_pwrite,
+	.open_fn = vfs_error_inject_open,
 };
 
 static_decl_vfs;
diff --git a/source3/script/tests/test_open_eintr.sh b/source3/script/tests/test_open_eintr.sh
new file mode 100755
index 00000000000..d08f7fcfc23
--- /dev/null
+++ b/source3/script/tests/test_open_eintr.sh
@@ -0,0 +1,66 @@
+#!/bin/bash
+#
+# Test smbd handling when open returns EINTR
+#
+# Copyright (C) 2020 Volker Lendecke
+
+if [ $# -lt 5 ]; then
+    echo Usage: test_open_eintr.sh \
+	 --configfile=SERVERCONFFILE SMBCLIENT SMBCONTROL SERVER SHARE
+exit 1
+fi
+
+CONF=$1; shift 1
+SMBCLIENT=$1; shift 1
+SMBCONTROL=$1; shift 1
+SERVER=$1; shift 1
+SHARE=$1; shift 1
+
+error_inject_conf=$(dirname ${SERVERCONFFILE})/error_inject.conf
+> ${error_inject_conf}
+
+incdir=$(dirname $0)/../../../testprogs/blackbox
+. $incdir/subunit.sh
+
+failed=0
+
+rm -f smbclient-stdin smbclient-stdout smbclient-stderr
+mkfifo smbclient-stdin smbclient-stdout smbclient-stderr
+
+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
+
+echo "error_inject:open = EINTR" > ${error_inject_conf}
+${SMBCONTROL} ${CONF} 0 reload-config
+
+sleep 1
+> ${error_inject_conf}
+
+echo 'get badnames/blank.txt -' >&100
+
+sleep 1
+
+> ${error_inject_conf}
+${SMBCONTROL} ${CONF} 0 reload-config
+
+head -n 1 <&102 | grep 'getting file' > /dev/null
+GREP_RET=$?
+
+kill ${CLIENT_PID}
+rm -f smbclient-stdin smbclient-stdout smbclient-stderr
+
+testit "Verify that we could get the file" \
+       test $GREP_RET -eq 0 || failed=$(expr $failed + 1)
+
+testok $0 $failed
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index d486e40c5a7..6060cec49c3 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -814,6 +814,15 @@ plantestsuite("samba3.blackbox.close-denied-share", "simpleserver:local",
                '$SERVER_IP',
                "tmp"])
 
+plantestsuite("samba3.blackbox.open-eintr", "simpleserver:local",
+              [os.path.join(samba3srcdir,
+                            "script/tests/test_open_eintr.sh"),
+               configuration,
+               os.path.join(bindir(), "smbclient"),
+               os.path.join(bindir(), "smbcontrol"),
+               '$SERVER_IP',
+               "error_inject"])
+
 plantestsuite("samba3.blackbox.net_tdb", "simpleserver:local",
               [os.path.join(samba3srcdir, "script/tests/test_net_tdb.sh"),
                smbclient3, '$SERVER', 'tmp', '$USERNAME', '$PASSWORD',
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 709f963c277..6e65ff8d53d 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -3301,6 +3301,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	SMB_STRUCT_STAT saved_stat = smb_fname->st;
 	struct timespec old_write_time;
 	struct file_id id;
+	bool setup_poll = false;
 	bool ok;
 
 	if (conn->printer) {
@@ -3645,6 +3646,14 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 			     open_access_mask, &new_file_created);
 
 	if (NT_STATUS_EQUAL(fsp_open, NT_STATUS_NETWORK_BUSY)) {
+		if (file_existed && S_ISFIFO(fsp->fsp_name->st.st_ex_mode)) {
+			DEBUG(10, ("FIFO busy\n"));
+			return NT_STATUS_NETWORK_BUSY;
+		}
+		if (req == NULL) {
+			DEBUG(10, ("Internal open busy\n"));
+			return NT_STATUS_NETWORK_BUSY;
+		}
 		/*
 		 * This handles the kernel oplock case:
 		 *
@@ -3654,15 +3663,20 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 		 * "Samba locking.tdb oplocks" are handled below after acquiring
 		 * the sharemode lock with get_share_mode_lock().
 		 */
-		if (file_existed && S_ISFIFO(fsp->fsp_name->st.st_ex_mode)) {
-			DEBUG(10, ("FIFO busy\n"));
-			return NT_STATUS_NETWORK_BUSY;
-		}
-		if (req == NULL) {
-			DEBUG(10, ("Internal open busy\n"));
-			return NT_STATUS_NETWORK_BUSY;
-		}
+		setup_poll = true;
+	}
+
+	if (NT_STATUS_EQUAL(fsp_open, NT_STATUS_RETRY)) {
+		/*
+		 * EINTR from the open(2) syscall. Just setup a retry
+		 * in a bit. We can't use the sys_write() tight retry
+		 * loop here, as we might have to actually deal with
+		 * lease-break signals to avoid a deadlock.
+		 */
+		setup_poll = true;
+	}
 
+	if (setup_poll) {
 		/*
 		 * From here on we assume this is an oplock break triggered
 		 */
@@ -3693,7 +3707,9 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	}
 
 	if (!NT_STATUS_IS_OK(fsp_open)) {
-		if (NT_STATUS_EQUAL(fsp_open, NT_STATUS_RETRY)) {
+		bool wait_for_aio = NT_STATUS_EQUAL(
+			fsp_open, NT_STATUS_MORE_PROCESSING_REQUIRED);
+		if (wait_for_aio) {
 			schedule_async_open(req);
 		}
 		return fsp_open;


-- 
Samba Shared Repository



More information about the samba-cvs mailing list