Attachment 14309 Details for Bug 13451 – Patch for 4.7 backported from master

[patch] Patch for 4.7 backported from master

bug13451-v47.patch (text/plain), 16.21 KB, created by Ralph Böhme on 2018-07-07 07:14:20 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2018-07-07 07:14:20 UTC

Size: 16.21 KB

patch

obsolete

>From 2b9456602a43e0deb2080f8a56be9552d70c4e26 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sat, 26 May 2018 16:30:47 +0200
>Subject: [PATCH 1/6] selftest: run smb2.streams tests against a share with
> vfs_streams_xattr
>
>The tests are currently only run against streams_depot, where stream IO
>is handle based, compared to streams_xattr which is path
>based. vfs_streams_xattr is also used much more in real world setups, so
>we should run our tests against it.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13451
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(backported from commit aa096ab70a466388a9947f73a525b2dcbb9821e5)
>---
> selftest/knownfail        | 3 +++
> source3/selftest/tests.py | 4 ++++
> 2 files changed, 7 insertions(+)
>
>diff --git a/selftest/knownfail b/selftest/knownfail
>index 267c928955e..01b6b3aff5b 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -167,6 +167,9 @@
> ^samba3.smb2.streams.rename
> ^samba3.smb2.streams.rename2
> ^samba3.smb2.streams.attributes
>+^samba3.smb2.streams streams_xattr.rename$nt4_dc$
>+^samba3.smb2.streams streams_xattr.rename2$nt4_dc$
>+^samba3.smb2.streams streams_xattr.attributes$nt4_dc$
> ^samba3.smb2.getinfo.complex
> ^samba3.smb2.getinfo.fsinfo # quotas don't work yet
> ^samba3.smb2.setinfo.setinfo
>diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>index 59a3a551d3f..c89c27cc9db 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -540,6 +540,10 @@ tests= base + raw + smb2 + rpc + unix + local + rap + nbt + libsmbclient + idmap
>         plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
>         plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/aio -U$USERNAME%$PASSWORD', 'aio')
>         plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
>+    elif t == "smb2.streams":
>+        plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
>+        plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
>+        plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/streams_xattr -U$USERNAME%$PASSWORD', 'streams_xattr')
>     else:
>         plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
>         plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
>-- 
>2.13.6
>
>
>From 55baa4f35a7394b0d73c773e0f064963ffb858ca Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sat, 26 May 2018 16:07:14 +0200
>Subject: [PATCH 2/6] s4:torture/smb2/streams: try to rename basefile while is
> has open streams
>
>This tests the following:
>
>- create a file with a stream
>- open the the stream and keep it open
>- on a second connection, try to rename the basefile, this should fail
>  with NT_STATUS_ACCESS_DENIED
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13451
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 00d19bdab15102083b8ba395ede161824c898be1)
>---
> selftest/knownfail.d/samba3.smb2.streams |  2 +
> source4/torture/smb2/streams.c           | 82 ++++++++++++++++++++++++++++++++
> 2 files changed, 84 insertions(+)
> create mode 100644 selftest/knownfail.d/samba3.smb2.streams
>
>diff --git a/selftest/knownfail.d/samba3.smb2.streams b/selftest/knownfail.d/samba3.smb2.streams
>new file mode 100644
>index 00000000000..26d40a67bda
>--- /dev/null
>+++ b/selftest/knownfail.d/samba3.smb2.streams
>@@ -0,0 +1,2 @@
>+samba3.smb2.streams.basefile-rename-with-open-stream$.*$
>+samba3.smb2.streams streams_xattr.basefile-rename-with-open-stream$nt4_dc$
>diff --git a/source4/torture/smb2/streams.c b/source4/torture/smb2/streams.c
>index d302bf923c9..b39d96d4924 100644
>--- a/source4/torture/smb2/streams.c
>+++ b/source4/torture/smb2/streams.c
>@@ -1830,6 +1830,86 @@ static bool test_stream_attributes(struct torture_context *tctx,
> 	return ret;
> }
> 
>+static bool test_basefile_rename_with_open_stream(struct torture_context *tctx,
>+						  struct smb2_tree *tree)
>+{
>+	bool ret = true;
>+	NTSTATUS status;
>+	struct smb2_tree *tree2 = NULL;
>+	struct smb2_create create, create2;
>+	struct smb2_handle h1 = {{0}}, h2 = {{0}};
>+	const char *fname = "test_rename_openfile";
>+	const char *sname = "test_rename_openfile:foo";
>+	const char *fname_renamed = "test_rename_openfile_renamed";
>+	union smb_setfileinfo sinfo;
>+	const char *data = "test data";
>+
>+	ret = torture_smb2_connection(tctx, &tree2);
>+	torture_assert_goto(tctx, ret == true, ret, done,
>+			    "torture_smb2_connection failed\n");
>+
>+	torture_comment(tctx, "Creating file with stream\n");
>+
>+	ZERO_STRUCT(create);
>+	create.in.desired_access = SEC_FILE_ALL;
>+	create.in.share_access = NTCREATEX_SHARE_ACCESS_MASK;
>+	create.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
>+	create.in.create_disposition = NTCREATEX_DISP_OPEN_IF;
>+	create.in.impersonation_level = SMB2_IMPERSONATION_IMPERSONATION;
>+	create.in.fname = sname;
>+
>+	status = smb2_create(tree, tctx, &create);
>+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
>+					"smb2_create failed\n");
>+
>+	h1 = create.out.file.handle;
>+
>+	torture_comment(tctx, "Writing to stream\n");
>+
>+	status = smb2_util_write(tree, h1, data, 0, strlen(data));
>+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
>+					"smb2_util_write failed\n");
>+
>+	torture_comment(tctx, "Renaming base file\n");
>+
>+	ZERO_STRUCT(create2);
>+	create2.in.desired_access = SEC_FILE_ALL;
>+	create2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
>+	create2.in.share_access = NTCREATEX_SHARE_ACCESS_MASK;
>+	create2.in.create_disposition = NTCREATEX_DISP_OPEN;
>+	create2.in.impersonation_level = SMB2_IMPERSONATION_IMPERSONATION;
>+	create2.in.fname = fname;
>+
>+	status = smb2_create(tree2, tctx, &create2);
>+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
>+					"smb2_create failed\n");
>+
>+	h2 = create2.out.file.handle;
>+
>+	ZERO_STRUCT(sinfo);
>+	sinfo.rename_information.level = RAW_SFILEINFO_RENAME_INFORMATION;
>+	sinfo.rename_information.in.file.handle = h2;
>+	sinfo.rename_information.in.new_name = fname_renamed;
>+
>+	status = smb2_setinfo_file(tree2, &sinfo);
>+	torture_assert_ntstatus_equal_goto(
>+		tctx, status, NT_STATUS_ACCESS_DENIED, ret, done,
>+		"smb2_setinfo_file didn't return NT_STATUS_ACCESS_DENIED\n");
>+
>+	smb2_util_close(tree2, h2);
>+
>+done:
>+	if (!smb2_util_handle_empty(h1)) {
>+		smb2_util_close(tree, h1);
>+	}
>+	if (!smb2_util_handle_empty(h2)) {
>+		smb2_util_close(tree2, h2);
>+	}
>+	smb2_util_unlink(tree, fname);
>+	smb2_util_unlink(tree, fname_renamed);
>+
>+	return ret;
>+}
> 
> /*
>    basic testing of streams calls SMB2
>@@ -1850,6 +1930,8 @@ struct torture_suite *torture_smb2_streams_init(TALLOC_CTX *ctx)
> 	torture_suite_add_1smb2_test(suite, "attributes", test_stream_attributes);
> 	torture_suite_add_1smb2_test(suite, "delete", test_stream_delete);
> 	torture_suite_add_1smb2_test(suite, "zero-byte", test_zero_byte_stream);
>+	torture_suite_add_1smb2_test(suite, "basefile-rename-with-open-stream",
>+					test_basefile_rename_with_open_stream);
> 
> 	suite->description = talloc_strdup(suite, "SMB2-STREAM tests");
> 
>-- 
>2.13.6
>
>
>From d27073c27a7220c50cca185d30343023cc7b9466 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sat, 26 May 2018 18:33:00 +0200
>Subject: [PATCH 3/6] s4:torture/vfs/fruit: adjust test testing basefile rename
> to expect failure
>
>Renaming a basefile that has open streams must fail with
>NT_STATUS_ACCESS_DENIED.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13451
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit f166207fc0344b51879d863857055ab7ff36a09b)
>---
> selftest/knownfail.d/samba3.vfs.fruit |  3 +++
> source4/torture/vfs/fruit.c           | 25 ++++---------------------
> 2 files changed, 7 insertions(+), 21 deletions(-)
>
>diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit
>index 8df25bccb79..bf97dbc5822 100644
>--- a/selftest/knownfail.d/samba3.vfs.fruit
>+++ b/selftest/knownfail.d/samba3.vfs.fruit
>@@ -1 +1,4 @@
> ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion$nt4_dc$
>+^samba3.vfs.fruit metadata_netatalk.read open rsrc after rename$nt4_dc$
>+^samba3.vfs.fruit metadata_stream.read open rsrc after rename$nt4_dc$
>+^samba3.vfs.fruit streams_depot.read open rsrc after rename$nt4_dc$
>diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
>index 597eb1abeff..3d44da67b10 100644
>--- a/source4/torture/vfs/fruit.c
>+++ b/source4/torture/vfs/fruit.c
>@@ -3897,7 +3897,6 @@ static bool test_rename_and_read_rsrc(struct torture_context *tctx,
> 	const char *fname_renamed = "test_rename_openfile_renamed";
> 	const char *data = "1234567890";
> 	union smb_setfileinfo sinfo;
>-	struct smb2_read r;
> 
> 	ret = enable_aapl(tctx, tree);
> 	torture_assert_goto(tctx, ret == true, ret, done, "enable_aapl failed");
>@@ -3949,28 +3948,12 @@ static bool test_rename_and_read_rsrc(struct torture_context *tctx,
> 	sinfo.rename_information.in.new_name = fname_renamed;
> 
> 	status = smb2_setinfo_file(tree, &sinfo);
>-	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_setinfo_file failed");
>-
>-	smb2_util_close(tree, h2);
>-
>-	ZERO_STRUCT(r);
>-	r.in.file.handle = h1;
>-	r.in.length      = 10;
>-	r.in.offset      = 0;
>-
>-	torture_comment(tctx, "Read resource fork of renamed file\n");
>-
>-	status = smb2_read(tree, tree, &r);
>-	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_read failed");
>+	torture_assert_ntstatus_equal_goto(
>+		tctx, status, NT_STATUS_ACCESS_DENIED, ret, done,
>+		"smb2_setinfo_file failed");
> 
> 	smb2_util_close(tree, h1);
>-
>-	torture_assert_goto(tctx, r.out.data.length == 10, ret, done,
>-			    talloc_asprintf(tctx, "smb2_read returned %jd bytes, expected 10\n",
>-					    (intmax_t)r.out.data.length));
>-
>-	torture_assert_goto(tctx, memcmp(r.out.data.data, data, 10) == 0, ret, done,
>-			    talloc_asprintf(tctx, "Bad data in stream\n"));
>+	smb2_util_close(tree, h2);
> 
> done:
> 	smb2_util_unlink(tree, fname);
>-- 
>2.13.6
>
>
>From 928f5d408690b1977e48ce42770acc2f0c8fdd85 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sun, 27 May 2018 13:01:50 +0200
>Subject: [PATCH 4/6] s3:smbd: add private option
> NTCREATEX_OPTIONS_PRIVATE_STREAM_BASEOPEN
>
>This will be used to mark basefile opens of streams opens. This is
>needed to later implement a function that can determine if a file has
>stream opens.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13451
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 37e7ff05ab9443c0330e68f5c701ffecedf2d738)
>---
> source3/include/smb.h | 3 +++
> source3/smbd/open.c   | 7 ++++++-
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/source3/include/smb.h b/source3/include/smb.h
>index 24a73e57a3d..df2cd2a3817 100644
>--- a/source3/include/smb.h
>+++ b/source3/include/smb.h
>@@ -420,6 +420,9 @@ Offset  Data			length.
> /* Private options for printer support */
> #define NTCREATEX_OPTIONS_PRIVATE_DELETE_ON_CLOSE 0x0008
> 
>+/* Private option for streams support */
>+#define NTCREATEX_OPTIONS_PRIVATE_STREAM_BASEOPEN 0x0010
>+
> /* Flag for NT transact rename call. */
> #define RENAME_REPLACE_IF_EXISTS 1
> 
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index cd2f1c0baf9..a47f4c9c091 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -5092,6 +5092,7 @@ static NTSTATUS create_file_unixpath(connection_struct *conn,
> 	    && (!(private_flags & NTCREATEX_OPTIONS_PRIVATE_STREAM_DELETE))) {
> 		uint32_t base_create_disposition;
> 		struct smb_filename *smb_fname_base = NULL;
>+		uint32_t base_privflags;
> 
> 		if (create_options & FILE_DIRECTORY_FILE) {
> 			status = NT_STATUS_NOT_A_DIRECTORY;
>@@ -5142,13 +5143,17 @@ static NTSTATUS create_file_unixpath(connection_struct *conn,
> 			}
> 		}
> 
>+		base_privflags = NTCREATEX_OPTIONS_PRIVATE_STREAM_BASEOPEN;
>+
> 		/* Open the base file. */
> 		status = create_file_unixpath(conn, NULL, smb_fname_base, 0,
> 					      FILE_SHARE_READ
> 					      | FILE_SHARE_WRITE
> 					      | FILE_SHARE_DELETE,
> 					      base_create_disposition,
>-					      0, 0, 0, NULL, 0, 0, NULL, NULL,
>+					      0, 0, 0, NULL, 0,
>+					      base_privflags,
>+					      NULL, NULL,
> 					      &base_fsp, NULL);
> 		TALLOC_FREE(smb_fname_base);
> 
>-- 
>2.13.6
>
>
>From 9c7e702c9d8ceab9ddd2291a743a66bbbf12a391 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sun, 27 May 2018 13:03:25 +0200
>Subject: [PATCH 5/6] s3:locking: add file_has_open_streams()
>
>This can be used to check if a file opened by fsp also has stream opens.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13451
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit dd8cf54c79fe8536e34cde15801d60931cd47b8b)
>---
> source3/locking/locking.c | 31 +++++++++++++++++++++++++++++++
> source3/locking/proto.h   |  1 +
> 2 files changed, 32 insertions(+)
>
>diff --git a/source3/locking/locking.c b/source3/locking/locking.c
>index 4e9f1bbc681..f71cd176029 100644
>--- a/source3/locking/locking.c
>+++ b/source3/locking/locking.c
>@@ -1318,3 +1318,34 @@ struct timespec get_share_mode_write_time(struct share_mode_lock *lck)
> 	}
> 	return d->old_write_time;
> }
>+
>+bool file_has_open_streams(files_struct *fsp)
>+{
>+	struct share_mode_lock *lock = NULL;
>+	struct share_mode_data *d = NULL;
>+	uint32_t i;
>+
>+	lock = get_existing_share_mode_lock(talloc_tos(), fsp->file_id);
>+	if (lock == NULL) {
>+		return false;
>+	}
>+	d = lock->data;
>+
>+	for (i = 0; i < d->num_share_modes; i++) {
>+		struct share_mode_entry *e = &d->share_modes[i];
>+
>+		if (share_mode_stale_pid(d, i)) {
>+			continue;
>+		}
>+
>+		if (e->private_options &
>+		    NTCREATEX_OPTIONS_PRIVATE_STREAM_BASEOPEN)
>+		{
>+			TALLOC_FREE(lock);
>+			return true;
>+		}
>+	}
>+
>+	TALLOC_FREE(lock);
>+	return false;
>+}
>diff --git a/source3/locking/proto.h b/source3/locking/proto.h
>index 33184e0fa0a..4cd38091f3c 100644
>--- a/source3/locking/proto.h
>+++ b/source3/locking/proto.h
>@@ -205,6 +205,7 @@ bool is_delete_on_close_set(struct share_mode_lock *lck, uint32_t name_hash);
> bool set_sticky_write_time(struct file_id fileid, struct timespec write_time);
> bool set_write_time(struct file_id fileid, struct timespec write_time);
> struct timespec get_share_mode_write_time(struct share_mode_lock *lck);
>+bool file_has_open_streams(files_struct *fsp);
> int share_mode_forall(int (*fn)(struct file_id fid,
> 				const struct share_mode_data *data,
> 				void *private_data),
>-- 
>2.13.6
>
>
>From b39bd977d4e6cad17fffbd807413acdfe138e2d7 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sat, 26 May 2018 18:32:21 +0200
>Subject: [PATCH 6/6] s3:smbd: don't allow renaming basefile if streams are
> open
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13451
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 465b7d07e5db787c3d6330371e5e42ecbb1b57b9)
>---
> selftest/knownfail.d/samba3.smb2.streams | 2 --
> selftest/knownfail.d/samba3.vfs.fruit    | 3 ---
> source3/smbd/reply.c                     | 4 ++++
> 3 files changed, 4 insertions(+), 5 deletions(-)
> delete mode 100644 selftest/knownfail.d/samba3.smb2.streams
>
>diff --git a/selftest/knownfail.d/samba3.smb2.streams b/selftest/knownfail.d/samba3.smb2.streams
>deleted file mode 100644
>index 26d40a67bda..00000000000
>--- a/selftest/knownfail.d/samba3.smb2.streams
>+++ /dev/null
>@@ -1,2 +0,0 @@
>-samba3.smb2.streams.basefile-rename-with-open-stream$.*$
>-samba3.smb2.streams streams_xattr.basefile-rename-with-open-stream$nt4_dc$
>diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit
>index bf97dbc5822..8df25bccb79 100644
>--- a/selftest/knownfail.d/samba3.vfs.fruit
>+++ b/selftest/knownfail.d/samba3.vfs.fruit
>@@ -1,4 +1 @@
> ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion$nt4_dc$
>-^samba3.vfs.fruit metadata_netatalk.read open rsrc after rename$nt4_dc$
>-^samba3.vfs.fruit metadata_stream.read open rsrc after rename$nt4_dc$
>-^samba3.vfs.fruit streams_depot.read open rsrc after rename$nt4_dc$
>diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
>index 81acedf0413..5295ff76bfa 100644
>--- a/source3/smbd/reply.c
>+++ b/source3/smbd/reply.c
>@@ -6645,6 +6645,10 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
> 		return status;
> 	}
> 
>+	if (file_has_open_streams(fsp)) {
>+		return NT_STATUS_ACCESS_DENIED;
>+	}
>+
> 	/* Make a copy of the dst smb_fname structs */
> 
> 	smb_fname_dst = cp_smb_filename(ctx, smb_fname_dst_in);
>-- 
>2.13.6
>

Flags:

jra: review+

Actions: View

Attachments on bug 13451: 14309 | 14310