Attachment 13874 Details for Bug 13189 – Patches for 4.7

[patch] Patches for 4.7

patches-4-7 (text/plain), 12.53 KB, created by Christof Schmitt on 2017-12-18 22:16:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Christof Schmitt

Created: 2017-12-18 22:16:52 UTC

Size: 12.53 KB

patch

obsolete

>From dd696b18e7e553d952e8dba98de60f221e5ed8bb Mon Sep 17 00:00:00 2001
>From: Christof Schmitt <cs@samba.org>
>Date: Fri, 8 Dec 2017 15:29:07 -0700
>Subject: [PATCH 1/5] vfs_error_inject: Add new module
>
>This module allow injecting errors in vfs calls. It only implements one
>case (return ESTALE from chdir), but the idea is to extend this to more
>vfs functions and more errors when needed.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13189
>
>Signed-off-by: Christof Schmitt <cs@samba.org>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 24623d53256c2424563709dedc19af1a106ccc73)
>---
> source3/modules/vfs_error_inject.c | 100 +++++++++++++++++++++++++++++++++++++
> source3/modules/wscript_build      |   7 +++
> source3/wscript                    |   1 +
> 3 files changed, 108 insertions(+)
> create mode 100644 source3/modules/vfs_error_inject.c
>
>diff --git a/source3/modules/vfs_error_inject.c b/source3/modules/vfs_error_inject.c
>new file mode 100644
>index 0000000..bb5477a
>--- /dev/null
>+++ b/source3/modules/vfs_error_inject.c
>@@ -0,0 +1,100 @@
>+/*
>+ *  Unix SMB/CIFS implementation.
>+ *  Samba VFS module for error injection in VFS calls
>+ *  Copyright (C) Christof Schmitt 2017
>+ *
>+ *  This program is free software; you can redistribute it and/or modify
>+ *  it under the terms of the GNU General Public License as published by
>+ *  the Free Software Foundation; either version 3 of the License, or
>+ *  (at your option) any later version.
>+ *
>+ *  This program is distributed in the hope that it will be useful,
>+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+ *  GNU General Public License for more details.
>+ *
>+ *  You should have received a copy of the GNU General Public License
>+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>+ */
>+
>+#include "includes.h"
>+#include "smbd/smbd.h"
>+
>+#undef DBGC_CLASS
>+#define DBGC_CLASS DBGC_VFS
>+
>+struct unix_error_map {
>+	const char *err_str;
>+	int error;
>+} unix_error_map_array[] = {
>+	{	"ESTALE",	ESTALE	},
>+};
>+
>+static int find_unix_error_from_string(const char *err_str)
>+{
>+	int i;
>+
>+	for (i = 0; i < ARRAY_SIZE(unix_error_map_array); i++) {
>+		struct unix_error_map *m = &unix_error_map_array[i];
>+
>+		if (strequal(err_str, m->err_str)) {
>+			return m->error;
>+		}
>+	}
>+
>+	return 0;
>+}
>+
>+static int inject_unix_error(const char *vfs_func, vfs_handle_struct *handle)
>+{
>+	const char *err_str;
>+
>+	err_str = lp_parm_const_string(SNUM(handle->conn),
>+				       "error_inject", vfs_func, NULL);
>+
>+	if (err_str != NULL) {
>+		int error;
>+
>+		error = find_unix_error_from_string(err_str);
>+		if (error != 0) {
>+			DBG_WARNING("Returning error %s for VFS function %s\n",
>+				    err_str, vfs_func);
>+			return error;
>+		}
>+
>+		if (strequal(err_str, "panic")) {
>+			DBG_ERR("Panic in VFS function %s\n", vfs_func);
>+			smb_panic("error_inject");
>+		}
>+
>+		DBG_ERR("Unknown error inject %s requested "
>+			"for vfs function %s\n", err_str, vfs_func);
>+	}
>+
>+	return 0;
>+}
>+
>+static int vfs_error_inject_chdir(vfs_handle_struct *handle,
>+				  const struct smb_filename *smb_fname)
>+{
>+	int error;
>+
>+	error = inject_unix_error("chdir", handle);
>+	if (error != 0) {
>+		errno = error;
>+		return -1;
>+	}
>+
>+	return SMB_VFS_NEXT_CHDIR(handle, smb_fname);
>+}
>+
>+static struct vfs_fn_pointers vfs_error_inject_fns = {
>+	.chdir_fn = vfs_error_inject_chdir,
>+};
>+
>+static_decl_vfs;
>+NTSTATUS vfs_error_inject_init(TALLOC_CTX *ctx)
>+{
>+	return smb_register_vfs(SMB_VFS_INTERFACE_VERSION, "error_inject",
>+				&vfs_error_inject_fns);
>+}
>diff --git a/source3/modules/wscript_build b/source3/modules/wscript_build
>index 24eeee6..a6a01f9 100644
>--- a/source3/modules/wscript_build
>+++ b/source3/modules/wscript_build
>@@ -509,3 +509,10 @@ bld.SAMBA3_MODULE('vfs_fake_dfq',
>                  init_function='',
>                  internal_module=bld.SAMBA3_IS_STATIC_MODULE('vfs_fake_dfq'),
>                  enabled=bld.SAMBA3_IS_ENABLED_MODULE('vfs_fake_dfq'))
>+
>+bld.SAMBA3_MODULE('vfs_error_inject',
>+                 subsystem='vfs',
>+                 source='vfs_error_inject.c',
>+                 init_function='',
>+                 internal_module=bld.SAMBA3_IS_STATIC_MODULE('vfs_error_inject'),
>+                 enabled=bld.SAMBA3_IS_ENABLED_MODULE('vfs_error_inject'))
>diff --git a/source3/wscript b/source3/wscript
>index f3b6d33..6ddb49a 100644
>--- a/source3/wscript
>+++ b/source3/wscript
>@@ -1703,6 +1703,7 @@ main() {
> 
>     if Options.options.enable_selftest or Options.options.developer:
>         default_shared_modules.extend(TO_LIST('vfs_fake_acls vfs_nfs4acl_xattr'))
>+        default_shared_modules.extend(TO_LIST('vfs_error_inject'))
> 
>     if conf.CONFIG_SET('AD_DC_BUILD_IS_ENABLED'):
>         default_static_modules.extend(TO_LIST('pdb_samba_dsdb auth_samba4 vfs_dfs_samba4'))
>-- 
>1.8.3.1
>
>
>From 5eb14ee142dee37949878f9ffce68337a2cafdab Mon Sep 17 00:00:00 2001
>From: Christof Schmitt <cs@samba.org>
>Date: Wed, 13 Dec 2017 11:34:05 -0700
>Subject: [PATCH 2/5] selftest: Add share for error injection testing
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13189
>
>Signed-off-by: Christof Schmitt <cs@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 8b6402f3e5ff98c2701e626e47246b2400f76e5f)
>---
> selftest/target/Samba3.pm | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
>index 78dc105..4178ed2 100755
>--- a/selftest/target/Samba3.pm
>+++ b/selftest/target/Samba3.pm
>@@ -2033,6 +2033,10 @@ sub provision($$$$$$$$$)
> [compound_find]
> 	copy = tmp
> 	smbd:find async delay usec = 10000
>+[error_inject]
>+	copy = tmp
>+	vfs objects = error_inject
>+	include = $libdir/error_inject.conf
> 	";
> 	close(CONF);
> 
>-- 
>1.8.3.1
>
>
>From 1f294fe49573dc74f2a6e4590d33d368a50deb68 Mon Sep 17 00:00:00 2001
>From: Christof Schmitt <cs@samba.org>
>Date: Wed, 13 Dec 2017 12:47:31 -0700
>Subject: [PATCH 3/5] selftest: Make location of log file available in tests
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13189
>
>Signed-off-by: Christof Schmitt <cs@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit b0e1fc74fdacecb86f46b47e527b3fdf1906d27b)
>---
> selftest/selftest.pl | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/selftest/selftest.pl b/selftest/selftest.pl
>index b3ef658..db65500 100755
>--- a/selftest/selftest.pl
>+++ b/selftest/selftest.pl
>@@ -870,6 +870,7 @@ my @exported_envvars = (
> 	"RESOLV_CONF",
> 	"UNACCEPTABLE_PASSWORD",
> 	"LOCK_DIR",
>+	"SMBD_TEST_LOG",
> 
> 	# nss_wrapper
> 	"NSS_WRAPPER_PASSWD",
>-- 
>1.8.3.1
>
>
>From f7dea854f5fee578caeb886faea98ee18fc1d97c Mon Sep 17 00:00:00 2001
>From: Christof Schmitt <cs@samba.org>
>Date: Wed, 13 Dec 2017 12:58:18 -0700
>Subject: [PATCH 4/5] selftest: Add test for failing chdir call in smbd
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13189
>
>Signed-off-by: Christof Schmitt <cs@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 0d3000be2af8f8c4a37892d95ae694ad834d7b3a)
>---
> selftest/knownfail                      |  1 +
> source3/script/tests/test_smbd_error.sh | 56 +++++++++++++++++++++++++++++++++
> source3/selftest/tests.py               |  3 ++
> 3 files changed, 60 insertions(+)
> create mode 100755 source3/script/tests/test_smbd_error.sh
>
>diff --git a/selftest/knownfail b/selftest/knownfail
>index dd23c7d..d5e4a18 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -345,3 +345,4 @@
> ^samba.tests.ntlmauth.python$ktest$.ntlmauth.NtlmAuthTests.test_ntlm_connection$ktest$
> # Disabling NTLM means you can't use samr to change the password
> ^samba.tests.ntlmauth.python$ktest$.ntlmauth.NtlmAuthTests.test_samr_change_password$ktest$
>+^samba3.blackbox.smbd_error.check_panic_2
>diff --git a/source3/script/tests/test_smbd_error.sh b/source3/script/tests/test_smbd_error.sh
>new file mode 100755
>index 0000000..e9af47a
>--- /dev/null
>+++ b/source3/script/tests/test_smbd_error.sh
>@@ -0,0 +1,56 @@
>+#!/bin/sh
>+#
>+# Test smbd with failing chdir system call.
>+#
>+# Verify that smbd does not panic when the chdir system call is
>+# returning an error.  ensure that the output format for ACL entries
>+#
>+# Copyright (C) 2017 Christof Schmitt
>+
>+. $(dirname $0)/../../../testprogs/blackbox/subunit.sh
>+failed=0
>+error_inject_conf=$(dirname $SMB_CONF_PATH)/error_inject.conf
>+
>+panic_count_0=$(grep -c PANIC $SMBD_TEST_LOG)
>+
>+#
>+# Verify that a panic in smbd will result in a PANIC message in the log
>+#
>+
>+# As a panic is expected here, also overwrite the default "panic
>+# action" in selftest to not start a debugger
>+echo 'error_inject:chdir = panic' > $error_inject_conf
>+echo '[global]' >> $error_inject_conf
>+echo 'panic action = ""' >> $error_inject_conf
>+
>+testit_expect_failure "smbclient" $VALGRIND \
>+		      $BINDIR/smbclient //$SERVER_IP/error_inject \
>+		      -U$USERNAME%$PASSWORD  -c dir ||
>+	failed=$(expr $failed + 1)
>+
>+rm $error_inject_conf
>+
>+panic_count_1=$(grep -c PANIC $SMBD_TEST_LOG)
>+
>+testit "check_panic_1" test $(expr $panic_count_0 + 1) -eq $panic_count_1 ||
>+	failed=$(expr $failed + 1)
>+
>+#
>+# Verify that a failing chdir vfs call does not result in a smbd panic
>+#
>+
>+echo 'error_inject:chdir = ESTALE' > $error_inject_conf
>+
>+testit_expect_failure "smbclient" $VALGRIND \
>+		      $BINDIR/smbclient //$SERVER_IP/error_inject \
>+		      -U$USERNAME%$PASSWORD  -c dir ||
>+	failed=$(expr $failed + 1)
>+
>+panic_count_2=$(grep -c PANIC $SMBD_TEST_LOG)
>+
>+testit "check_panic_2" test $panic_count_1 -eq $panic_count_2 ||
>+	failed=$(expr $failed + 1)
>+
>+rm $error_inject_conf
>+
>+testok $0 $failed
>diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>index c20ff17..108d0c4 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -572,6 +572,9 @@ plantestsuite("samba3.blackbox.net_tdb", "simpleserver:local",
>                 smbclient3, '$SERVER', 'tmp', '$USERNAME', '$PASSWORD',
>                 configuration, '$LOCAL_PATH', '$LOCK_DIR' ])
> 
>+plantestsuite("samba3.blackbox.smbd_error", "simpleserver:local",
>+              [ os.path.join(samba3srcdir, "script/tests/test_smbd_error.sh") ])
>+
> plantestsuite("samba3.blackbox.net_cache_samlogon", "ad_member:local",
>               [ os.path.join(samba3srcdir, "script/tests/test_net_cache_samlogon.sh"),
>                 '$SERVER', 'tmp', '$DC_USERNAME', '$DC_PASSWORD'])
>-- 
>1.8.3.1
>
>
>From ea3d1b509b86191838dafe7c81c4336bd759f86f Mon Sep 17 00:00:00 2001
>From: Christof Schmitt <cs@samba.org>
>Date: Wed, 13 Dec 2017 11:34:23 -0700
>Subject: [PATCH 5/5] smbd: Fix coredump on failing chdir during logoff
>
>server_exit does an internal tree disconnect which requires a chdir to
>the share directory. In case the file system encountered a problem and
>the chdir call returns an error, this triggers a SERVER_EXIT_ABNORMAL
>which in turn results in a panic and a coredump. As the log already
>indicates the problem (chdir returned an error), avoid the
>SERVER_EXIT_ABNORMAL in this case and not trigger a coredump.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13189
>
>Signed-off-by: Christof Schmitt <cs@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Jeremy Allison <jra@samba.org>
>Autobuild-Date(master): Sat Dec 16 01:56:06 CET 2017 on sn-devel-144
>
>(cherry picked from commit 7fa91fc4791d076c609eaf119753e38dd3c50a1c)
>---
> selftest/knownfail         | 1 -
> source3/smbd/server_exit.c | 4 ----
> 2 files changed, 5 deletions(-)
>
>diff --git a/selftest/knownfail b/selftest/knownfail
>index d5e4a18..dd23c7d 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -345,4 +345,3 @@
> ^samba.tests.ntlmauth.python$ktest$.ntlmauth.NtlmAuthTests.test_ntlm_connection$ktest$
> # Disabling NTLM means you can't use samr to change the password
> ^samba.tests.ntlmauth.python$ktest$.ntlmauth.NtlmAuthTests.test_samr_change_password$ktest$
>-^samba3.blackbox.smbd_error.check_panic_2
>diff --git a/source3/smbd/server_exit.c b/source3/smbd/server_exit.c
>index 74ddd70..ab96ebf 100644
>--- a/source3/smbd/server_exit.c
>+++ b/source3/smbd/server_exit.c
>@@ -150,8 +150,6 @@ static void exit_server_common(enum server_exit_reason how,
> 			DEBUG(0, ("exit_server_common: "
> 				  "smb1srv_tcon_disconnect_all() failed (%s) - "
> 				  "triggering cleanup\n", nt_errstr(status)));
>-			how = SERVER_EXIT_ABNORMAL;
>-			reason = "smb1srv_tcon_disconnect_all failed";
> 		}
> 
> 		status = smbXsrv_session_logoff_all(xconn);
>@@ -161,8 +159,6 @@ static void exit_server_common(enum server_exit_reason how,
> 			DEBUG(0, ("exit_server_common: "
> 				  "smbXsrv_session_logoff_all() failed (%s) - "
> 				  "triggering cleanup\n", nt_errstr(status)));
>-			how = SERVER_EXIT_ABNORMAL;
>-			reason = "smbXsrv_session_logoff_all failed";
> 		}
> 	}
> 
>-- 
>1.8.3.1
>

Flags:

jra: review+

Actions: View

Attachments on bug 13189: 13873 | 13874