[SCM] Samba Shared Repository - branch v4-6-test updated

Karolin Seeger kseeger at samba.org
Tue Jan 2 13:02:05 UTC 2018


The branch, v4-6-test has been updated
       via  6ba6125 smbd: Fix coredump on failing chdir during logoff
       via  60eb51d selftest: Add test for failing chdir call in smbd
       via  e6ec5ae selftest: Make location of log file available in tests
       via  90d87d4 selftest: Add share for error injection testing
       via  919d16e vfs_error_inject: Add new module
       via  d932fcf ctdb-recovery-helper: Deregister message handler in error paths
       via  a3dc640 sysacls: change datatypes to 32 bits
       via  e64528a pysmbd: fix use of sysacl API
      from  f502340 HEIMDAL:kdc: fix dh->q allocation check in get_dh_param()

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-6-test


- Log -----------------------------------------------------------------
commit 6ba61252745b097ab73b78534e227660d564ca11
Author: Christof Schmitt <cs at samba.org>
Date:   Wed Dec 13 11:34:23 2017 -0700

    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>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Sat Dec 16 01:56:06 CET 2017 on sn-devel-144
    
    (cherry picked from commit 7fa91fc4791d076c609eaf119753e38dd3c50a1c)
    
    Autobuild-User(v4-6-test): Karolin Seeger <kseeger at samba.org>
    Autobuild-Date(v4-6-test): Tue Jan  2 14:01:29 CET 2018 on sn-devel-144

commit 60eb51d6e6c23636e64fb21ece88a484679a753e
Author: Christof Schmitt <cs at samba.org>
Date:   Wed Dec 13 12:58:18 2017 -0700

    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>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    (cherry picked from commit 0d3000be2af8f8c4a37892d95ae694ad834d7b3a)

commit e6ec5ae88258ab1ec8d8bc3023b852f1939eae90
Author: Christof Schmitt <cs at samba.org>
Date:   Wed Dec 13 12:47:31 2017 -0700

    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 at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    (cherry picked from commit b0e1fc74fdacecb86f46b47e527b3fdf1906d27b)

commit 90d87d4b9c557a8c60edbd1ba26680df29d5640c
Author: Christof Schmitt <cs at samba.org>
Date:   Wed Dec 13 11:34:05 2017 -0700

    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>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    (cherry picked from commit 8b6402f3e5ff98c2701e626e47246b2400f76e5f)

commit 919d16e3f3c586c1c8785e1104221019dad733d0
Author: Christof Schmitt <cs at samba.org>
Date:   Fri Dec 8 15:29:07 2017 -0700

    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>
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    (cherry picked from commit 24623d53256c2424563709dedc19af1a106ccc73)

commit d932fcf6c0177e038272c37f4704c6e92cf0cab6
Author: Amitay Isaacs <amitay at gmail.com>
Date:   Wed Dec 13 16:12:09 2017 +1100

    ctdb-recovery-helper: Deregister message handler in error paths
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13188
    
    If PULL_DB control times out but the remote node is still sending the
    data, then the tevent_req for pull_database_send will be freed without
    removing the message handler.  So when the data is received, srvid
    handler will be called and it will try to access tevent_req which will
    result in use-after-free and abort.
    
    Signed-off-by: Amitay Isaacs <amitay at gmail.com>
    Reviewed-by: Martin Schwenke <martin at meltin.net>

commit a3dc640c9d5978f3746c570ca9e34c1c57fa83bc
Author: Uri Simchoni <uri at samba.org>
Date:   Tue Dec 5 20:56:49 2017 +0200

    sysacls: change datatypes to 32 bits
    
    The SMB_ACL_PERMSET_T and SMB_ACL_PERM_T were defined as
    mode_t, which is 16-bits on some (non-Linux) systems. However,
    pidl *always* encodes mode_t as uint32_t. That created a bug on
    big-endian systems as sys_acl_get_permset() returns a SMB_ACL_PERMSET_T
    pointer to an internal a_perm structure member defined in IDL as a mode_t,
    which pidl turns into a uin32_t in the emitted header file.
    
    Changing to 32 bits fixes that.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13176
    
    Signed-off-by: Uri Simchoni <uri at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    (back-ported from commit 75e7da9741c529f96fa409655ac5b326a6c071c5)

commit e64528a915398c7d5ac6da508f7ebdf4faf7b444
Author: Uri Simchoni <uri at samba.org>
Date:   Tue Dec 5 20:49:03 2017 +0200

    pysmbd: fix use of sysacl API
    
    Fix pysmbd to use the sysacl (POSIX ACL support) as intended, and
    not assume too much about the inner structure and implementation
    of the permissions in the sysacl API.
    
    This will allow the inner structure to change in a following commit.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13176
    
    Signed-off-by: Uri Simchoni <uri at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    (cherry picked from commit d6f5ee6707fa5404e2bef6fc81ae06b393ebd8ff)

-----------------------------------------------------------------------

Summary of changes:
 ctdb/server/ctdb_recovery_helper.c      | 16 ++++--
 selftest/selftest.pl                    |  1 +
 selftest/target/Samba3.pm               |  4 ++
 source3/include/smb_acls.h              | 10 +++-
 source3/modules/vfs_error_inject.c      | 99 +++++++++++++++++++++++++++++++++
 source3/modules/wscript_build           |  7 +++
 source3/script/tests/test_smbd_error.sh | 56 +++++++++++++++++++
 source3/selftest/tests.py               |  3 +
 source3/smbd/pysmbd.c                   | 43 ++++++++++++--
 source3/smbd/server_exit.c              |  4 --
 source3/wscript                         |  1 +
 11 files changed, 229 insertions(+), 15 deletions(-)
 create mode 100644 source3/modules/vfs_error_inject.c
 create mode 100755 source3/script/tests/test_smbd_error.sh


Changeset truncated at 500 lines:

diff --git a/ctdb/server/ctdb_recovery_helper.c b/ctdb/server/ctdb_recovery_helper.c
index 474b900..2c7bb4b 100644
--- a/ctdb/server/ctdb_recovery_helper.c
+++ b/ctdb/server/ctdb_recovery_helper.c
@@ -428,6 +428,7 @@ struct pull_database_state {
 	uint32_t pnn;
 	uint64_t srvid;
 	int num_records;
+	int result;
 };
 
 static void pull_database_handler(uint64_t srvid, TDB_DATA data,
@@ -625,8 +626,8 @@ static void pull_database_new_done(struct tevent_req *subreq)
 	if (! status) {
 		LOG("control DB_PULL failed for %s on node %u, ret=%d\n",
 		    recdb_name(state->recdb), state->pnn, ret);
-		tevent_req_error(req, ret);
-		return;
+		state->result = ret;
+		goto unregister;
 	}
 
 	ret = ctdb_reply_control_db_pull(reply, &num_records);
@@ -634,13 +635,15 @@ static void pull_database_new_done(struct tevent_req *subreq)
 	if (num_records != state->num_records) {
 		LOG("mismatch (%u != %u) in DB_PULL records for %s\n",
 		    num_records, state->num_records, recdb_name(state->recdb));
-		tevent_req_error(req, EIO);
-		return;
+		state->result = EIO;
+		goto unregister;
 	}
 
 	LOG("Pulled %d records for db %s from node %d\n",
 	    state->num_records, recdb_name(state->recdb), state->pnn);
 
+unregister:
+
 	subreq = ctdb_client_remove_message_handler_send(
 					state, state->ev, state->client,
 					state->srvid, req);
@@ -668,6 +671,11 @@ static void pull_database_unregister_done(struct tevent_req *subreq)
 		return;
 	}
 
+	if (state->result != 0) {
+		tevent_req_error(req, state->result);
+		return;
+	}
+
 	tevent_req_done(req);
 }
 
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",
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);
 
diff --git a/source3/include/smb_acls.h b/source3/include/smb_acls.h
index 3ac23db..cd2452b 100644
--- a/source3/include/smb_acls.h
+++ b/source3/include/smb_acls.h
@@ -26,8 +26,14 @@ struct vfs_handle_struct;
 struct files_struct;
 
 typedef int			SMB_ACL_TYPE_T;
-typedef mode_t			*SMB_ACL_PERMSET_T;
-typedef mode_t			SMB_ACL_PERM_T;
+/*
+ * struct smb_acl_entry is defined in IDL as
+ * using mode_t values, pidl always converts these
+ * to uint32_t. Ensure the external type definitions
+ * match.
+ */
+typedef uint32_t		*SMB_ACL_PERMSET_T;
+typedef uint32_t		SMB_ACL_PERM_T;
 
 typedef enum smb_acl_tag_t SMB_ACL_TAG_T;
 typedef struct smb_acl_t *SMB_ACL_T;
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/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",
diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
index fca8f10..06a02cb 100644
--- a/source3/smbd/pysmbd.c
+++ b/source3/smbd/pysmbd.c
@@ -224,6 +224,39 @@ static NTSTATUS get_nt_acl_conn(TALLOC_CTX *mem_ctx,
 	return status;
 }
 
+static int set_acl_entry_perms(SMB_ACL_ENTRY_T entry, mode_t perm_mask)
+{
+	SMB_ACL_PERMSET_T perms = NULL;
+
+	if (sys_acl_get_permset(entry, &perms) != 0) {
+		return -1;
+	}
+
+	if (sys_acl_clear_perms(perms) != 0) {
+		return -1;
+	}
+
+	if ((perm_mask & SMB_ACL_READ) != 0 &&
+	    sys_acl_add_perm(perms, SMB_ACL_READ) != 0) {
+		return -1;
+	}
+
+	if ((perm_mask & SMB_ACL_WRITE) != 0 &&
+	    sys_acl_add_perm(perms, SMB_ACL_WRITE) != 0) {
+		return -1;
+	}
+
+	if ((perm_mask & SMB_ACL_EXECUTE) != 0 &&
+	    sys_acl_add_perm(perms, SMB_ACL_EXECUTE) != 0) {
+		return -1;
+	}
+
+	if (sys_acl_set_permset(entry, perms) != 0) {
+		return -1;
+	}
+
+	return 0;
+}
 
 static SMB_ACL_T make_simple_acl(gid_t gid, mode_t chmod_mode)
 {
@@ -251,7 +284,7 @@ static SMB_ACL_T make_simple_acl(gid_t gid, mode_t chmod_mode)
 		return NULL;
 	}
 
-	if (sys_acl_set_permset(entry, &mode_user) != 0) {
+	if (set_acl_entry_perms(entry, mode_user) != 0) {
 		TALLOC_FREE(frame);
 		return NULL;
 	}
@@ -266,7 +299,7 @@ static SMB_ACL_T make_simple_acl(gid_t gid, mode_t chmod_mode)
 		return NULL;
 	}
 
-	if (sys_acl_set_permset(entry, &mode_group) != 0) {
+	if (set_acl_entry_perms(entry, mode_group) != 0) {
 		TALLOC_FREE(frame);
 		return NULL;
 	}
@@ -281,7 +314,7 @@ static SMB_ACL_T make_simple_acl(gid_t gid, mode_t chmod_mode)
 		return NULL;
 	}
 
-	if (sys_acl_set_permset(entry, &mode_other) != 0) {
+	if (set_acl_entry_perms(entry, mode_other) != 0) {
 		TALLOC_FREE(frame);
 		return NULL;
 	}
@@ -302,7 +335,7 @@ static SMB_ACL_T make_simple_acl(gid_t gid, mode_t chmod_mode)
 			return NULL;
 		}
 
-		if (sys_acl_set_permset(entry, &mode_group) != 0) {
+		if (set_acl_entry_perms(entry, mode_group) != 0) {
 			TALLOC_FREE(frame);
 			return NULL;
 		}
@@ -318,7 +351,7 @@ static SMB_ACL_T make_simple_acl(gid_t gid, mode_t chmod_mode)
 		return NULL;
 	}
 
-	if (sys_acl_set_permset(entry, &mode) != 0) {
+	if (set_acl_entry_perms(entry, mode) != 0) {
 		TALLOC_FREE(frame);
 		return NULL;
 	}
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";
 		}
 	}
 
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'))


-- 
Samba Shared Repository



More information about the samba-cvs mailing list