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

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


The branch, v4-7-test has been updated
       via  671a3c6 smbd: Fix coredump on failing chdir during logoff
       via  e21538a selftest: Add test for failing chdir call in smbd
       via  bba8e0f selftest: Make location of log file available in tests
       via  02c60f2 selftest: Add share for error injection testing
       via  028d129 vfs_error_inject: Add new module
       via  47b6eca ctdb-recovery-helper: Deregister message handler in error paths
       via  d983766 sysacls: change datatypes to 32 bits
       via  0752022 pysmbd: fix use of sysacl API
      from  424e40f HEIMDAL:kdc: fix dh->q allocation check in get_dh_param()

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


- Log -----------------------------------------------------------------
commit 671a3c6a5e8f5025ad4cdaff38461fce49737c7c
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-7-test): Karolin Seeger <kseeger at samba.org>
    Autobuild-Date(v4-7-test): Tue Jan  2 14:19:44 CET 2018 on sn-devel-144

commit e21538a676539471016425201c36ef2ad637204f
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 bba8e0f761186f92f6864c03aeca1dc082c5f21a
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 02c60f2e83b9ecd1024138954c635be248fd3a7b
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 028d12916c872075c5b11a1038fd0d1d5a9274de
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 47b6ecaf953a3ed3edbdba04a25a32f0241af2c4
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 d983766eaf7ff93db5aa0ac478bbacdda8aef62d
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>
    (cherry picked from commit 75e7da9741c529f96fa409655ac5b326a6c071c5)

commit 07520226dc0be32b120e6c209199755586d997cb
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      | 100 ++++++++++++++++++++++++++++++++
 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, 230 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 9f7fc07..e966b3a 100644
--- a/ctdb/server/ctdb_recovery_helper.c
+++ b/ctdb/server/ctdb_recovery_helper.c
@@ -397,6 +397,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,
@@ -594,8 +595,8 @@ static void pull_database_new_done(struct tevent_req *subreq)
 	if (! status) {
 		D_ERR("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);
@@ -604,13 +605,15 @@ static void pull_database_new_done(struct tevent_req *subreq)
 		D_ERR("mismatch (%u != %u) in DB_PULL records for db %s\n",
 		      num_records, state->num_records,
 		      recdb_name(state->recdb));
-		tevent_req_error(req, EIO);
-		return;
+		state->result = EIO;
+		goto unregister;
 	}
 
 	D_INFO("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);
@@ -638,6 +641,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 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",
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);
 
diff --git a/source3/include/smb_acls.h b/source3/include/smb_acls.h
index 73b67af..c5a2339 100644
--- a/source3/include/smb_acls.h
+++ b/source3/include/smb_acls.h
@@ -27,8 +27,14 @@ struct files_struct;
 struct smb_filename;
 
 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..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 <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 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/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'])
diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
index 63fc5d6..be30b86 100644
--- a/source3/smbd/pysmbd.c
+++ b/source3/smbd/pysmbd.c
@@ -234,6 +234,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)
 {
@@ -261,7 +294,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;
 	}
@@ -276,7 +309,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;
 	}
@@ -291,7 +324,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;
 	}
@@ -312,7 +345,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;
 		}
@@ -328,7 +361,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 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";
 		}
 	}
 
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'))


-- 
Samba Shared Repository



More information about the samba-cvs mailing list