Attachment 13873 Details for Bug 13189 – Patches for 4.6

[patch] Patches for 4.6

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

(hide)

Description:

Filename:

MIME Type:

Creator: Christof Schmitt

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

Size: 12.55 KB

patch

obsolete

>From 52c9353b4a9c257b207b7db76814fde6da045a58 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 | 99 ++++++++++++++++++++++++++++++++++++++
> source3/modules/wscript_build      |  7 +++
> source3/wscript                    |  1 +
> 3 files changed, 107 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..3196a2f
>--- /dev/null
>+++ b/source3/modules/vfs_error_inject.c
>@@ -0,0 +1,99 @@
>+/*
>+ *  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 char *path)
>+{
>+	int error;
>+
>+	error = inject_unix_error("chdir", handle);
>+	if (error != 0) {
>+		errno = error;
>+		return -1;
>+	}
>+
>+	return SMB_VFS_NEXT_CHDIR(handle, path);
>+}
>+
>+static struct vfs_fn_pointers vfs_error_inject_fns = {
>+	.chdir_fn = vfs_error_inject_chdir,
>+};
>+
>+static_decl_vfs;
>+NTSTATUS vfs_error_inject_init(void)
>+{
>+	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 86ff3eb..ae28d76 100644
>--- a/source3/modules/wscript_build
>+++ b/source3/modules/wscript_build
>@@ -505,3 +505,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 b705406..33eb599 100644
>--- a/source3/wscript
>+++ b/source3/wscript
>@@ -1694,6 +1694,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 d5ce0c8d1297b816ce973acd245fb4c3a5035884 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 dbfad1c..77716e9 100755
>--- a/selftest/target/Samba3.pm
>+++ b/selftest/target/Samba3.pm
>@@ -2010,6 +2010,10 @@ sub provision($$$$$$$$)
> 	copy = tmp
> 	kernel oplocks = yes
> 	vfs objects = streams_xattr xattr_tdb
>+[error_inject]
>+	copy = tmp
>+	vfs objects = error_inject
>+	include = $libdir/error_inject.conf
> 	";
> 	close(CONF);
> 
>-- 
>1.8.3.1
>
>
>From 8e5e80b7263331a7547f274febedff6c194ead40 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 c54ea68..c4a5464 100755
>--- a/selftest/selftest.pl
>+++ b/selftest/selftest.pl
>@@ -843,6 +843,7 @@ my @exported_envvars = (
> 	"DNS_FORWARDER2",
> 	"RESOLV_CONF",
> 	"UNACCEPTABLE_PASSWORD",
>+	"SMBD_TEST_LOG",
> 
> 	# nss_wrapper
> 	"NSS_WRAPPER_PASSWD",
>-- 
>1.8.3.1
>
>
>From b41ec7ac87274d5d9d005588982f23cdec3fcd4d 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 6e1d058..e259ccc 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -308,3 +308,4 @@
> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_rank_none.*
> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_security_descriptor.*
> ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion$nt4_dc$
>+^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 5dfdce7..5e74650 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -513,6 +513,9 @@ 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.smbd_error", "simpleserver:local",
>+              [ os.path.join(samba3srcdir, "script/tests/test_smbd_error.sh") ])
>+
> plantestsuite("samba3.blackbox.net_dom_join_fail_dc", "nt4_dc",
>               [os.path.join(samba3srcdir, "script/tests/test_net_dom_join_fail_dc.sh"),
>                "$USERNAME", "$PASSWORD", "$SERVER", "$PREFIX/net_dom_join_fail_dc",
>-- 
>1.8.3.1
>
>
>From f0c20b46850bc0f1ac2d56ab5c8805744e5242bf 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 e259ccc..6e1d058 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -308,4 +308,3 @@
> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_rank_none.*
> ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_security_descriptor.*
> ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion$nt4_dc$
>-^samba3.blackbox.smbd_error.check_panic_2
>diff --git a/source3/smbd/server_exit.c b/source3/smbd/server_exit.c
>index bf50394..04a2932 100644
>--- a/source3/smbd/server_exit.c
>+++ b/source3/smbd/server_exit.c
>@@ -149,8 +149,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);
>@@ -160,8 +158,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