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 <v...@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 <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
Autobuild-User(master): Jeremy Allison <j...@samba.org>
Autobuild-Date(master): Thu Feb 20 22:14:25 UTC 2020 on sn-devel-184
commit 4a943d842a51674425f0c4019f823ef0a9d09f49
Author: Volker Lendecke <v...@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 <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit 7bbba73b30f06304e9a2ad48e853d9ec8171dd30
Author: Volker Lendecke <v...@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 <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit 305204a241b74c599f4f6a064cac6608afd9c893
Author: Volker Lendecke <v...@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 <v...@samba.org>
Reviewed-by: Jeremy Allison <j...@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
