Attachment 16662 Details for Bug 14742 – git-am fix for 4.14.next.

[patch] git-am fix for 4.14.next.

bug-14742-v4.14 (text/plain), 10.93 KB, created by Jeremy Allison on 2021-06-16 17:49:37 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2021-06-16 17:49:37 UTC

Size: 10.93 KB

patch

obsolete

>From 35eb02c8c3bb290dbd3b00ce560f5c1bfe54f1c4 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Wed, 16 Jun 2021 10:47:33 -0700
>Subject: [PATCH 1/2] 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
>
>Back-ported from ac10058d7f6b4605157f508189a448310f5f18da
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> selftest/knownfail.d/setpathsymlink |   2 +
> source3/selftest/tests.py           |   1 +
> source3/torture/proto.h             |   1 +
> source3/torture/test_posix.c        | 195 ++++++++++++++++++++++++++++
> source3/torture/torture.c           |   4 +
> 5 files changed, 203 insertions(+)
> create mode 100644 selftest/knownfail.d/setpathsymlink
>
>diff --git a/selftest/knownfail.d/setpathsymlink b/selftest/knownfail.d/setpathsymlink
>new file mode 100644
>index 00000000000..9d7ded388c2
>--- /dev/null
>+++ b/selftest/knownfail.d/setpathsymlink
>@@ -0,0 +1,2 @@
>+^samba3.smbtorture_s3.crypt.POSIX-SYMLINK-SETPATHINFO.smbtorture\(nt4_dc_smb1\)
>+^samba3.smbtorture_s3.plain.POSIX-SYMLINK-SETPATHINFO.smbtorture\(nt4_dc_smb1\)
>diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>index fe2fee610e5..cd886f9bf50 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -247,6 +247,7 @@ posix_tests = ["POSIX", "POSIX-APPEND", "POSIX-SYMLINK-ACL", "POSIX-SYMLINK-EA",
>                "POSIX-LS-SINGLE",
>                "POSIX-READLINK",
>                "POSIX-STAT",
>+               "POSIX-SYMLINK-SETPATHINFO",
>               ]
> 
> for t in posix_tests:
>diff --git a/source3/torture/proto.h b/source3/torture/proto.h
>index 521c662e2fb..e6d00b290a6 100644
>--- a/source3/torture/proto.h
>+++ b/source3/torture/proto.h
>@@ -90,6 +90,7 @@ bool run_posix_ls_single_test(int dummy);
> bool run_posix_readlink_test(int dummy);
> bool run_posix_stat_test(int dummy);
> bool run_case_insensitive_create(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 3ccb51d222b..a4e25aad063 100644
>--- a/source3/torture/test_posix.c
>+++ b/source3/torture/test_posix.c
>@@ -23,6 +23,7 @@
> #include "libsmb/clirap.h"
> #include "libsmb/proto.h"
> #include "../libcli/smb/smbXcli_base.h"
>+#include "trans2.h"
> 
> extern struct cli_credentials *torture_creds;
> extern fstring host, workgroup, share, password, username, myname;
>@@ -734,3 +735,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 d1ea9b85a72..a56a73150fb 100644
>--- a/source3/torture/torture.c
>+++ b/source3/torture/torture.c
>@@ -14967,6 +14967,10 @@ static struct {
> 		.name  = "ASYNC-ECHO",
> 		.fn    = run_async_echo,
> 	},
>+	{
>+		.name  = "POSIX-SYMLINK-SETPATHINFO",
>+		.fn    = run_posix_symlink_setpathinfo_test,
>+	},
> 	{
> 		.name  = "UID-REGRESSION-TEST",
> 		.fn    = run_uid_regression_test,
>-- 
>2.27.0
>
>
>From 2bfcf38218b47e9e9db79ea16b96428be66ccea1 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 15 Jun 2021 15:42:33 -0700
>Subject: [PATCH 2/2] 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@samba.org>
>Reviewed-by: Noel Power <npower@samba.org>
>
>Autobuild-User(master): Noel Power <npower@samba.org>
>Autobuild-Date(master): Wed Jun 16 11:58:00 UTC 2021 on sn-devel-184
>
>(cherry picked from commit 263c95aee38c9198ad9a30c4d960d72f46b7c27a)
>---
> selftest/knownfail.d/setpathsymlink |  2 --
> source3/smbd/trans2.c               | 14 +++++++++++++-
> 2 files changed, 13 insertions(+), 3 deletions(-)
> delete mode 100644 selftest/knownfail.d/setpathsymlink
>
>diff --git a/selftest/knownfail.d/setpathsymlink b/selftest/knownfail.d/setpathsymlink
>deleted file mode 100644
>index 9d7ded388c2..00000000000
>--- a/selftest/knownfail.d/setpathsymlink
>+++ /dev/null
>@@ -1,2 +0,0 @@
>-^samba3.smbtorture_s3.crypt.POSIX-SYMLINK-SETPATHINFO.smbtorture\(nt4_dc_smb1\)
>-^samba3.smbtorture_s3.plain.POSIX-SYMLINK-SETPATHINFO.smbtorture\(nt4_dc_smb1\)
>diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
>index fac45df586e..70a492a96a8 100644
>--- a/source3/smbd/trans2.c
>+++ b/source3/smbd/trans2.c
>@@ -785,6 +785,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(conn, fsp, fsp->fsp_name);
>@@ -6860,7 +6864,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;
> 	}
> 
>@@ -7899,6 +7903,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;
>@@ -7956,6 +7964,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 */
>-- 
>2.27.0
>

Flags:

slow: review-

Actions: View

Attachments on bug 14742: 16662 | 16663 | 16743 | 16747