[SCM] Samba Shared Repository - branch master updated

Noel Power npower at samba.org
Wed Jun 16 11:58:01 UTC 2021


The branch, master has been updated
       via  263c95aee38 s3: smbd: Fix smbd crash on dangling symlink with posix connection calling several non-posix info levels.
       via  ac10058d7f6 s3: torture: Add POSIX-SYMLINK-SETPATHINFO regression test.
      from  620b9914435 mdssvc: avoid direct filesystem access, use the VFS

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 263c95aee38c9198ad9a30c4d960d72f46b7c27a
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Jun 15 15:42:33 2021 -0700

    s3: smbd: Fix smbd crash on dangling symlink with posix connection calling several non-posix info levels.
    
    Tidy up fsp == NULL checks. Remove knownfail.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14742
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>
    
    Autobuild-User(master): Noel Power <npower at samba.org>
    Autobuild-Date(master): Wed Jun 16 11:58:00 UTC 2021 on sn-devel-184

commit ac10058d7f6b4605157f508189a448310f5f18da
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Jun 15 15:11:20 2021 -0700

    s3: torture: Add POSIX-SYMLINK-SETPATHINFO regression test.
    
    This ensure we never blunder into indirecting a NULL fsp pointer
    in the server. Currently this crashes the server in several info
    levels.
    
    Add knownfail.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14742
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Noel Power <npower at samba.org>

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

Summary of changes:
 source3/selftest/tests.py    |   1 +
 source3/smbd/trans2.c        |  14 +++-
 source3/torture/proto.h      |   1 +
 source3/torture/test_posix.c | 194 +++++++++++++++++++++++++++++++++++++++++++
 source3/torture/torture.c    |   4 +
 5 files changed, 213 insertions(+), 1 deletion(-)


Changeset truncated at 500 lines:

diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index c86e6bc9c7d..5e9bebdcbce 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -267,6 +267,7 @@ posix_tests = ["POSIX", "POSIX-APPEND", "POSIX-SYMLINK-ACL", "POSIX-SYMLINK-EA",
                "POSIX-DIR-DEFAULT-ACL",
                "POSIX-SYMLINK-RENAME",
                "POSIX-SYMLINK-GETPATHINFO",
+               "POSIX-SYMLINK-SETPATHINFO",
               ]
 
 for t in posix_tests:
diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index d6a1ea81ce0..23c13da4c58 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -744,6 +744,10 @@ NTSTATUS set_ea(connection_struct *conn, files_struct *fsp,
 		return NT_STATUS_EAS_NOT_SUPPORTED;
 	}
 
+	if (fsp == NULL) {
+		return NT_STATUS_INVALID_HANDLE;
+	}
+
 	posix_pathnames = (fsp->fsp_name->flags & SMB_FILENAME_POSIX_PATH);
 
 	status = refuse_symlink_fsp(fsp);
@@ -6862,7 +6866,7 @@ static NTSTATUS smb_set_file_full_ea_info(connection_struct *conn,
 	struct ea_list *ea_list = NULL;
 	NTSTATUS status;
 
-	if (!fsp) {
+	if (fsp == NULL) {
 		return NT_STATUS_INVALID_HANDLE;
 	}
 
@@ -7887,6 +7891,10 @@ static NTSTATUS smb_set_file_basic_info(connection_struct *conn,
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
+	if (fsp == NULL) {
+		return NT_STATUS_INVALID_HANDLE;
+	}
+
 	status = check_access_fsp(fsp, FILE_WRITE_ATTRIBUTES);
 	if (!NT_STATUS_IS_OK(status)) {
 		return status;
@@ -7944,6 +7952,10 @@ static NTSTATUS smb_set_info_standard(connection_struct *conn,
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
+	if (fsp == NULL) {
+		return NT_STATUS_INVALID_HANDLE;
+	}
+
 	/* create time */
 	ft.create_time = time_t_to_full_timespec(srv_make_unix_date2(pdata));
 	/* access time */
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index 471609d2837..dfcbd815af0 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -95,6 +95,7 @@ bool run_posix_dir_default_acl_test(int dummy);
 bool run_case_insensitive_create(int dummy);
 bool run_posix_symlink_rename_test(int dummy);
 bool run_posix_symlink_getpathinfo_test(int dummy);
+bool run_posix_symlink_setpathinfo_test(int dummy);
 
 bool run_nbench2(int dummy);
 bool run_async_echo(int dummy);
diff --git a/source3/torture/test_posix.c b/source3/torture/test_posix.c
index 85e4e6c510f..20b2b136761 100644
--- a/source3/torture/test_posix.c
+++ b/source3/torture/test_posix.c
@@ -1694,3 +1694,197 @@ out:
 	TALLOC_FREE(frame);
 	return correct;
 }
+
+/* List of info levels to try with a POSIX symlink path. */
+
+static struct {
+	uint32_t level;
+	const char *name;
+	uint32_t data_len;
+} posix_smb1_setpath_array[] = {
+  { SMB_SET_FILE_UNIX_BASIC,	"SMB_SET_FILE_UNIX_BASIC",	100},
+  { SMB_SET_FILE_UNIX_INFO2,	"SMB_SET_FILE_UNIX_INFO2",	116},
+  { SMB_SET_FILE_UNIX_LINK,	"SMB_SET_FILE_UNIX_LINK",	8},
+  { SMB_SET_FILE_UNIX_HLINK,	"SMB_SET_FILE_UNIX_HLINK",	8},
+  { SMB_SET_POSIX_ACL,		"SMB_SET_POSIX_ACL",		6},
+  { SMB_SET_POSIX_LOCK,		"SMB_SET_POSIX_LOCK",		24},
+  { SMB_INFO_STANDARD,		"SMB_INFO_STANDARD",		12},
+  { SMB_INFO_SET_EA,		"SMB_INFO_SET_EA",		10},
+  { SMB_FILE_BASIC_INFORMATION, "SMB_FILE_BASIC_INFORMATION",	36},
+  { SMB_SET_FILE_ALLOCATION_INFO, "SMB_SET_FILE_ALLOCATION_INFO", 8},
+  { SMB_SET_FILE_END_OF_FILE_INFO,"SMB_SET_FILE_END_OF_FILE_INFO",8},
+  { SMB_SET_FILE_DISPOSITION_INFO,"SMB_SET_FILE_DISPOSITION_INFO",1},
+  { SMB_FILE_POSITION_INFORMATION,"SMB_FILE_POSITION_INFORMATION",8},
+  { SMB_FILE_FULL_EA_INFORMATION, "SMB_FILE_FULL_EA_INFORMATION",10},
+  { SMB_FILE_MODE_INFORMATION,	"SMB_FILE_MODE_INFORMATION",	4},
+  { SMB_FILE_SHORT_NAME_INFORMATION,"SMB_FILE_SHORT_NAME_INFORMATION",12},
+  { SMB_FILE_RENAME_INFORMATION,"SMB_FILE_RENAME_INFORMATION",	20},
+  { SMB_FILE_LINK_INFORMATION,	"SMB_FILE_LINK_INFORMATION",	20},
+};
+
+static NTSTATUS do_setpath(TALLOC_CTX *ctx,
+			   struct cli_state *cli_unix,
+			   const char *fname,
+			   size_t i)
+{
+	NTSTATUS status;
+	uint8_t *data = NULL;
+
+	data = talloc_zero_array(ctx,
+				 uint8_t,
+				 posix_smb1_setpath_array[i].data_len);
+	if (data == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	status = cli_setpathinfo(cli_unix,
+			posix_smb1_setpath_array[i].level,
+			fname,
+			data,
+			posix_smb1_setpath_array[i].data_len);
+	TALLOC_FREE(data);
+
+	/*
+	 * We don't care what came back, so long as the
+	 * server didn't crash.
+	 */
+	if (NT_STATUS_EQUAL(status,
+			NT_STATUS_CONNECTION_DISCONNECTED)) {
+		printf("cli_setpathinfo info %x (%s) of %s failed"
+			"error NT_STATUS_CONNECTION_DISCONNECTED\n",
+			(unsigned int)posix_smb1_setpath_array[i].level,
+			posix_smb1_setpath_array[i].name,
+			fname);
+		return status;
+	}
+
+	printf("cli_setpathinfo info %x (%s) of %s got %s "
+		"(this is not an error)\n",
+		(unsigned int)posix_smb1_setpath_array[i].level,
+		posix_smb1_setpath_array[i].name,
+		fname,
+		nt_errstr(status));
+
+	return NT_STATUS_OK;
+}
+
+/*
+  Ensure we can call SMB1 setpathinfo in a symlink,
+  pointing to a real object or dangling. We mostly
+  expect errors, but the server must not crash.
+ */
+bool run_posix_symlink_setpathinfo_test(int dummy)
+{
+	TALLOC_CTX *frame = NULL;
+	struct cli_state *cli_unix = NULL;
+	NTSTATUS status;
+	uint16_t fnum = (uint16_t)-1;
+	const char *fname_real = "file_setpath_real";
+	const char *fname_real_symlink = "file_real_setpath_symlink";
+	const char *nonexist = "nonexist_setpath";
+	const char *nonexist_symlink = "dangling_setpath_symlink";
+	bool correct = false;
+	size_t i;
+
+	frame = talloc_stackframe();
+
+	printf("Starting POSIX-SYMLINK-SETPATHINFO test\n");
+
+	if (!torture_open_connection(&cli_unix, 0)) {
+		TALLOC_FREE(frame);
+		return false;
+	}
+
+	torture_conn_set_sockopt(cli_unix);
+
+	status = torture_setup_unix_extensions(cli_unix);
+	if (!NT_STATUS_IS_OK(status)) {
+		TALLOC_FREE(frame);
+		return false;
+	}
+
+	/* Start with a clean slate. */
+	cli_posix_unlink(cli_unix, fname_real);
+	cli_posix_unlink(cli_unix, fname_real_symlink);
+	cli_posix_unlink(cli_unix, nonexist);
+	cli_posix_unlink(cli_unix, nonexist_symlink);
+
+	/* Create a real file. */
+	status = cli_posix_open(cli_unix,
+				fname_real,
+				O_RDWR|O_CREAT,
+				0644,
+				&fnum);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("cli_posix_open of %s failed error %s\n",
+		       fname_real,
+		       nt_errstr(status));
+		goto out;
+	}
+	status = cli_close(cli_unix, fnum);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("cli_close failed %s\n", nt_errstr(status));
+		goto out;
+	}
+	fnum = (uint16_t)-1;
+
+	/* Create symlink to real target. */
+	status = cli_posix_symlink(cli_unix,
+				   fname_real,
+				   fname_real_symlink);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("cli_posix_symlink of %s -> %s failed error %s\n",
+		       fname_real_symlink,
+		       fname_real,
+		       nt_errstr(status));
+		goto out;
+	}
+
+	/* Now create symlink to non-existing target. */
+	status = cli_posix_symlink(cli_unix,
+				   nonexist,
+				   nonexist_symlink);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("cli_posix_symlink of %s -> %s failed error %s\n",
+		       nonexist_symlink,
+		       nonexist,
+		       nt_errstr(status));
+		goto out;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(posix_smb1_setpath_array); i++) {
+		status = do_setpath(frame,
+				  cli_unix,
+				  fname_real_symlink,
+				  i);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto out;
+		}
+		status = do_setpath(frame,
+				  cli_unix,
+				  nonexist_symlink,
+				  i);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto out;
+		}
+	}
+
+	printf("POSIX-SYMLINK-SETPATHINFO test passed\n");
+	correct = true;
+
+out:
+	if (fnum != (uint16_t)-1) {
+		cli_close(cli_unix, fnum);
+	}
+	cli_posix_unlink(cli_unix, fname_real);
+	cli_posix_unlink(cli_unix, fname_real_symlink);
+	cli_posix_unlink(cli_unix, nonexist);
+	cli_posix_unlink(cli_unix, nonexist_symlink);
+
+	if (!torture_close_connection(cli_unix)) {
+		correct = false;
+	}
+
+	TALLOC_FREE(frame);
+	return correct;
+}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 0187ebf7a43..892d6d6beab 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -14967,6 +14967,10 @@ static struct {
 		.name  = "POSIX-SYMLINK-GETPATHINFO",
 		.fn    = run_posix_symlink_getpathinfo_test,
 	},
+	{
+		.name  = "POSIX-SYMLINK-SETPATHINFO",
+		.fn    = run_posix_symlink_setpathinfo_test,
+	},
 	{
 		.name  = "WINDOWS-BAD-SYMLINK",
 		.fn    = run_symlink_open_test,


-- 
Samba Shared Repository



More information about the samba-cvs mailing list