[PATCHES][BUG 13189] Fix coredump on failing chdir during logoff
Christof Schmitt
cs at samba.org
Wed Dec 13 20:24:05 UTC 2017
smbd coredumps when a chdir call fails during the server shutdown. This
should be avoided as the failing call is already logged and the coredump
does not provide additional information.
Christof
-------------- next part --------------
From b5a325a5e04fa445f56778d94d734b13d22062fe Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Wed, 13 Dec 2017 11:34:23 -0700
Subject: [PATCH 1/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 at samba.org>
---
source3/smbd/server_exit.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/source3/smbd/server_exit.c b/source3/smbd/server_exit.c
index a9ef37f..dbeb247 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
From d1527eebf055f8ae7d19d68cd2bdb6951eab9511 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Fri, 8 Dec 2017 15:29:07 -0700
Subject: [PATCH 2/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 at samba.org>
---
source3/modules/vfs_error_inject.c | 94 ++++++++++++++++++++++++++++++++++++++
source3/modules/wscript_build | 7 +++
source3/wscript | 1 +
3 files changed, 102 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..b4919f5
--- /dev/null
+++ b/source3/modules/vfs_error_inject.c
@@ -0,0 +1,94 @@
+/*
+ * 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;
+ }
+
+ 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,
+};
+
+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 4e4fdb9..079302c 100644
--- a/source3/modules/wscript_build
+++ b/source3/modules/wscript_build
@@ -525,3 +525,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 d45a7d9..0f8fe54 100644
--- a/source3/wscript
+++ b/source3/wscript
@@ -1674,6 +1674,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 79399b2b79d804bf6f37f42219a34d3c27bc5d02 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Wed, 13 Dec 2017 11:34:05 -0700
Subject: [PATCH 3/5] selftest: Add share for error injection testing
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13189
Signed-off-by: Christof Schmitt <cs at samba.org>
---
selftest/target/Samba3.pm | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index afbc795..229435d 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -2150,6 +2150,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 f1b74665bf7134ed95eb4ad169d6d4507507b852 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Wed, 13 Dec 2017 12:47:31 -0700
Subject: [PATCH 4/5] selftest: Make location of log and config files available
in tests
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13189
Signed-off-by: Christof Schmitt <cs at samba.org>
---
selftest/selftest.pl | 2 ++
1 file changed, 2 insertions(+)
diff --git a/selftest/selftest.pl b/selftest/selftest.pl
index e16696a..4b7b6bb 100755
--- a/selftest/selftest.pl
+++ b/selftest/selftest.pl
@@ -877,6 +877,8 @@ my @exported_envvars = (
"RESOLV_CONF",
"UNACCEPTABLE_PASSWORD",
"LOCK_DIR",
+ "SMBD_TEST_LOG",
+ "SERVERCONFFILE",
# nss_wrapper
"NSS_WRAPPER_PASSWD",
--
1.8.3.1
From d4e13af3548b0b10640322a7bbec685d9a4e0cc4 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Wed, 13 Dec 2017 12:58:18 -0700
Subject: [PATCH 5/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 at samba.org>
---
source3/script/tests/test_chdir_error.sh | 32 ++++++++++++++++++++++++++++++++
source3/selftest/tests.py | 3 +++
2 files changed, 35 insertions(+)
create mode 100755 source3/script/tests/test_chdir_error.sh
diff --git a/source3/script/tests/test_chdir_error.sh b/source3/script/tests/test_chdir_error.sh
new file mode 100755
index 0000000..33b631c
--- /dev/null
+++ b/source3/script/tests/test_chdir_error.sh
@@ -0,0 +1,32 @@
+#!/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 $SERVERCONFFILE)/error_inject.conf
+echo 'error_inject:chdir = estale' > $error_inject_conf
+
+panic_count_1=$(grep -c PANIC $SMBD_TEST_LOG)
+
+testit_expect_failure "smbclient" $VALGRIND \
+ $BINDIR/smbclient //$SERVER_IP/error_inject \
+ -U$USERNAME%$PASSWORD -c dir ||
+ failed=$(expr $failed + 1)
+
+# Verify that the test did not trigger a smbd panic
+panic_count_2=$(grep -c PANIC $SMBD_TEST_LOG)
+
+testit "check panic" 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 1b35768..a98178d 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -584,6 +584,9 @@ plantestsuite("samba3.blackbox.net_tdb", "simpleserver:local",
smbclient3, '$SERVER', 'tmp', '$USERNAME', '$PASSWORD',
configuration, '$LOCAL_PATH', '$LOCK_DIR' ])
+plantestsuite("samba3.blackbox.chdir_error", "simpleserver:local",
+ [ os.path.join(samba3srcdir, "script/tests/test_chdir_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
More information about the samba-technical
mailing list