[SCM] Samba Shared Repository - branch v4-11-test updated

Karolin Seeger kseeger at samba.org
Tue Feb 25 22:25:02 UTC 2020


The branch, v4-11-test has been updated
       via  a95a8c7eaa4 smbd: Separate aio_pthread indicator from normal EINTR
       via  a33656c9df2 lib: Map EINPROGRESS->NT_STATUS_MORE_PROCESSING_REQUIRED
       via  64b2eda07fc test: Show that smbd does not handle EINTR from open() correctly
       via  0232cc46a35 test: Intercept open in vfs_error_inject
       via  ea1e73c2281 wafsamba: Do not use 'rU' as the 'U' is deprecated in Python 3.9
      from  370278fca39 s3: VFS: full_audit. Use system session_info if called from a temporary share definition.

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-11-test


- Log -----------------------------------------------------------------
commit a95a8c7eaa46d5c8c485de714f0a97e307e49f7e
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
    
    (cherry picked from commit aebe427b77b5315eb5d2b05b8c72824ca0389723)
    
    Autobuild-User(v4-11-test): Karolin Seeger <kseeger at samba.org>
    Autobuild-Date(v4-11-test): Tue Feb 25 22:24:54 UTC 2020 on sn-devel-184

commit a33656c9df2cde3ff1cfc6b0427c7dfb2b140cae
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>
    (cherry picked from commit 4a943d842a51674425f0c4019f823ef0a9d09f49)

commit 64b2eda07fcf3ee38a344848297c2a0f8a13748b
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>
    (cherry picked from commit 7bbba73b30f06304e9a2ad48e853d9ec8171dd30)

commit 0232cc46a35a57b4c3ccdb7d4222ec0c9f3fca38
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>
    (cherry picked from commit 305204a241b74c599f4f6a064cac6608afd9c893)

commit ea1e73c2281ea3e7849fd30002c42d858b19b968
Author: Andreas Schneider <asn at samba.org>
Date:   Wed Feb 5 16:58:26 2020 +0100

    wafsamba: Do not use 'rU' as the 'U' is deprecated in Python 3.9
    
    See https://docs.python.org/3.9/whatsnew/3.9.html#changes-in-the-python-api
    
    "open(), io.open(), codecs.open() and fileinput.FileInput no longer accept 'U'
    (“universal newline”) in the file mode. This flag was deprecated since Python
    3.3. In Python 3, the “universal newline” is used by default when a file is
    open in text mode. The newline parameter of open() controls how universal
    newlines works."
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14266
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    
    Autobuild-User(master): Andreas Schneider <asn at cryptomilk.org>
    Autobuild-Date(master): Thu Feb  6 07:30:13 UTC 2020 on sn-devel-184
    
    (cherry picked from commit 52722746a5eb40c309ba59f78bd8e3d897417bdc)

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

Summary of changes:
 buildtools/wafsamba/samba_utils.py      |  2 +-
 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                     | 38 +++++++++++++------
 7 files changed, 122 insertions(+), 13 deletions(-)
 create mode 100755 source3/script/tests/test_open_eintr.sh


Changeset truncated at 500 lines:

diff --git a/buildtools/wafsamba/samba_utils.py b/buildtools/wafsamba/samba_utils.py
index ad97de1859b..be022adc8f5 100644
--- a/buildtools/wafsamba/samba_utils.py
+++ b/buildtools/wafsamba/samba_utils.py
@@ -700,7 +700,7 @@ def PROCESS_SEPARATE_RULE(self, rule):
             cache[node] = True
             self.pre_recurse(node)
             try:
-                function_code = node.read('rU', None)
+                function_code = node.read('r', None)
                 exec(compile(function_code, node.abspath(), 'exec'), self.exec_dict)
             finally:
                 self.post_recurse(node)
diff --git a/source3/lib/errmap_unix.c b/source3/lib/errmap_unix.c
index 9eb30f7b814..20cc714ea9d 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 577180b6b01..5febdd3af32 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 67e9282459c..5c43e6e9458 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -751,6 +751,15 @@ plantestsuite("samba3.blackbox.sharesec", "simpleserver:local",
               [os.path.join(samba3srcdir, "script/tests/test_sharesec.sh"),
                configuration, os.path.join(bindir(), "sharesec"), "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 5524f80af20..871cc72053f 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2960,6 +2960,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;
 
 	if (conn->printer) {
 		/*
@@ -3290,8 +3291,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)) {
-		bool delay;
-
+		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:
 		 *
@@ -3301,14 +3308,21 @@ 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) {
+		bool delay;
 
 		/*
 		 * From here on we assume this is an oplock break triggered
@@ -3359,7 +3373,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(request_time, req);
 		}
 		return fsp_open;


-- 
Samba Shared Repository



More information about the samba-cvs mailing list