From dd696b18e7e553d952e8dba98de60f221e5ed8bb 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 | 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 . + */ + +#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 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 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 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 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 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 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 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 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