From 52c9353b4a9c257b207b7db76814fde6da045a58 Mon Sep 17 00:00:00 2001 From: Christof Schmitt 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 Signed-off-by: Andrew Bartlett Reviewed-by: Jeremy Allison (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 . + */ + +#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 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 Reviewed-by: Jeremy Allison (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 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 Reviewed-by: Jeremy Allison (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 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 Reviewed-by: Jeremy Allison (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 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 Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison 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