Attachment 15817 Details for Bug 14285 – patch for 4.12

[patch] patch for 4.12

14285-4.12.txt (text/plain), 10.11 KB, created by Volker Lendecke on 2020-02-24 12:52:09 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Volker Lendecke

Created: 2020-02-24 12:52:09 UTC

Size: 10.11 KB

patch

obsolete

>From 4862bec97ff3b5d72f104c998af8aaaa9e1ce856 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Wed, 19 Feb 2020 14:44:11 +0100
>Subject: [PATCH 1/4] test: Intercept open in vfs_error_inject
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=14285
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 305204a241b74c599f4f6a064cac6608afd9c893)
>---
> source3/modules/vfs_error_inject.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
>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;
>-- 
>2.20.1
>
>
>From 5efcf86392ddf1409c4fe9104b824a31d0372839 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Wed, 19 Feb 2020 15:25:38 +0100
>Subject: [PATCH 2/4] 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@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 7bbba73b30f06304e9a2ad48e853d9ec8171dd30)
>---
> selftest/knownfail.d/open_eintr         |  1 +
> source3/script/tests/test_open_eintr.sh | 66 +++++++++++++++++++++++++
> source3/selftest/tests.py               |  9 ++++
> 3 files changed, 76 insertions(+)
> create mode 100644 selftest/knownfail.d/open_eintr
> create mode 100755 source3/script/tests/test_open_eintr.sh
>
>diff --git a/selftest/knownfail.d/open_eintr b/selftest/knownfail.d/open_eintr
>new file mode 100644
>index 00000000000..e5b42ed6bcd
>--- /dev/null
>+++ b/selftest/knownfail.d/open_eintr
>@@ -0,0 +1 @@
>+samba3.blackbox.open-eintr.*$simpleserver:local$
>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 b6751248996..dddcb03230c 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -802,6 +802,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',
>-- 
>2.20.1
>
>
>From 09b502ee1c60f68b0e32f7bf0393f342b2d8ed78 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Thu, 20 Feb 2020 10:25:16 +0100
>Subject: [PATCH 3/4] lib: Map EINPROGRESS->NT_STATUS_MORE_PROCESSING_REQUIRED
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=14285
>Signed-off-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 4a943d842a51674425f0c4019f823ef0a9d09f49)
>---
> source3/lib/errmap_unix.c | 1 +
> 1 file changed, 1 insertion(+)
>
>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 },
> };
> 
> /*********************************************************************
>-- 
>2.20.1
>
>
>From bfb3a309385e45bf39e551f936c8a839ad3cb437 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Thu, 20 Feb 2020 14:13:35 +0100
>Subject: [PATCH 4/4] 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@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Jeremy Allison <jra@samba.org>
>Autobuild-Date(master): Thu Feb 20 22:14:25 UTC 2020 on sn-devel-184
>
>(cherry picked from commit aebe427b77b5315eb5d2b05b8c72824ca0389723)
>---
> selftest/knownfail.d/open_eintr   |  1 -
> source3/modules/vfs_aio_pthread.c |  2 +-
> source3/smbd/open.c               | 34 +++++++++++++++++++++++--------
> 3 files changed, 26 insertions(+), 11 deletions(-)
> delete mode 100644 selftest/knownfail.d/open_eintr
>
>diff --git a/selftest/knownfail.d/open_eintr b/selftest/knownfail.d/open_eintr
>deleted file mode 100644
>index e5b42ed6bcd..00000000000
>--- a/selftest/knownfail.d/open_eintr
>+++ /dev/null
>@@ -1 +0,0 @@
>-samba3.blackbox.open-eintr.*$simpleserver:local$
>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/smbd/open.c b/source3/smbd/open.c
>index e64e22581e0..98770358cf1 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;
>-- 
>2.20.1
>

Flags:

jra: review+
vl: ci-passed+

Actions: View

Attachments on bug 14285: 15817 | 15818 | 15819