Attachment 13911 Details for Bug 13181 – Patch for 4.6 backported from master

[patch] Patch for 4.6 backported from master

bug13181-v46.patch (text/plain), 16.41 KB, created by Ralph Böhme on 2018-01-16 09:57:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2018-01-16 09:57:51 UTC

Size: 16.41 KB

patch

obsolete

>From e59ceb30e14b102e77e7979993062dd4487d35e6 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Wed, 6 Dec 2017 22:05:23 +0100
>Subject: [PATCH 1/5] s4/torture/fruit: ensure AFP_AfpInfo blobs are
> 0-initialized
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13181
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit a22833c2971dc7234b32741305f40ed62e232e0b)
>---
> source4/torture/vfs/fruit.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
>index 76890ff75e0..88e3de27059 100644
>--- a/source4/torture/vfs/fruit.c
>+++ b/source4/torture/vfs/fruit.c
>@@ -909,7 +909,7 @@ static char *torture_afpinfo_pack(TALLOC_CTX *mem_ctx,
> {
> 	char *buf;
> 
>-	buf = talloc_array(mem_ctx, char, AFP_INFO_SIZE);
>+	buf = talloc_zero_array(mem_ctx, char, AFP_INFO_SIZE);
> 	if (buf == NULL) {
> 		return NULL;
> 	}
>-- 
>2.13.6
>
>
>From 475c3e97e19ea1a28b1d82314d40ad6f2b7d20d0 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Thu, 7 Dec 2017 13:43:02 +0100
>Subject: [PATCH 2/5] s4/torture/fruit: enhance zero AFP_AfpInfo stream test
>
>This test more operations in the zeroed out FinderInfo test, ensuring
>after zeroing out FinderInfo, operations on the filehandle still work
>and that enumerating streams doesn't return the stream anymore.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13181
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(backported from commit df31e94eb6241f5e5594f6fd0ec1ad7896e02e27)
>---
> selftest/knownfail.d/samba3.vfs.fruit |  2 +
> source4/torture/vfs/fruit.c           | 87 +++++++++++++++++++++++++++++++++--
> 2 files changed, 86 insertions(+), 3 deletions(-)
> create mode 100644 selftest/knownfail.d/samba3.vfs.fruit
>
>diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit
>new file mode 100644
>index 00000000000..1ba64febb20
>--- /dev/null
>+++ b/selftest/knownfail.d/samba3.vfs.fruit
>@@ -0,0 +1,2 @@
>+^samba3.vfs.fruit metadata_stream.delete AFP_AfpInfo by writing all 0$nt4_dc$
>+^samba3.vfs.fruit streams_depot.delete AFP_AfpInfo by writing all 0$nt4_dc$
>diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
>index 88e3de27059..05503b01c5e 100644
>--- a/source4/torture/vfs/fruit.c
>+++ b/source4/torture/vfs/fruit.c
>@@ -3340,11 +3340,17 @@ static bool test_afpinfo_all0(struct torture_context *tctx,
> {
> 	bool ret = true;
> 	NTSTATUS status;
>-	struct smb2_handle h1;
>+	struct smb2_create create;
>+	struct smb2_handle h1 = {{0}};
>+	struct smb2_handle baseh = {{0}};
>+	union smb_setfileinfo setfinfo;
>+	union smb_fileinfo getfinfo;
> 	TALLOC_CTX *mem_ctx = talloc_new(tctx);
> 	const char *fname = BASEDIR "\\file";
>+	const char *sname = BASEDIR "\\file" AFPINFO_STREAM;
> 	const char *type_creator = "SMB,OLE!";
> 	AfpInfo *info = NULL;
>+	char *infobuf = NULL;
> 	const char *streams_basic[] = {
> 		"::$DATA"
> 	};
>@@ -3375,13 +3381,88 @@ static bool test_afpinfo_all0(struct torture_context *tctx,
> 
> 	/* Write all 0 to AFP_AfpInfo */
> 	memset(info->afpi_FinderInfo, 0, AFP_FinderSize);
>-	ret = torture_write_afpinfo(tree, tctx, mem_ctx, fname, info);
>-	torture_assert_goto(tctx, ret == true, ret, done, "torture_write_afpinfo failed");
>+	infobuf = torture_afpinfo_pack(mem_ctx, info);
>+	torture_assert_not_null_goto(tctx, infobuf, ret, done,
>+				     "torture_afpinfo_pack failed\n");
>+
>+	ZERO_STRUCT(create);
>+	create.in.desired_access = SEC_FILE_ALL;
>+	create.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
>+	create.in.create_disposition = NTCREATEX_DISP_OPEN;
>+	create.in.share_access = NTCREATEX_SHARE_ACCESS_MASK;
>+	create.in.fname = fname;
>+
>+	status = smb2_create(tree, mem_ctx, &create);
>+	torture_assert_goto(tctx, ret == true, ret, done,
>+			    "smb2_create failed\n");
>+	baseh = create.out.file.handle;
>+
>+	ZERO_STRUCT(create);
>+	create.in.desired_access = SEC_FILE_ALL;
>+	create.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
>+	create.in.create_disposition = NTCREATEX_DISP_OVERWRITE_IF;
>+	create.in.fname = sname;
>+
>+	status = smb2_create(tree, mem_ctx, &create);
>+	torture_assert_goto(tctx, ret == true, ret, done,
>+			    "smb2_create failed\n");
>+	h1 = create.out.file.handle;
>+
>+	status = smb2_util_write(tree, h1, infobuf, 0, AFP_INFO_SIZE);
>+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
>+					"smb2_util_write failed\n");
>+
>+	/*
>+	 * Get stream information on open handle, must return only default
>+	 * stream, the AFP_AfpInfo stream must not be returned.
>+	 */
>+
>+	ZERO_STRUCT(getfinfo);
>+	getfinfo.generic.level = RAW_FILEINFO_STREAM_INFORMATION;
>+	getfinfo.generic.in.file.handle = baseh;
>+
>+	status = smb2_getinfo_file(tree, tctx, &getfinfo);
>+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
>+					"get stream info\n");
>+
>+	torture_assert_int_equal_goto(tctx, getfinfo.stream_info.out.num_streams,
>+				      1, ret, done, "stream count");
>+
>+	smb2_util_close(tree, baseh);
>+	ZERO_STRUCT(baseh);
>+
>+	/*
>+	 * Try to set some file-basic-info (time) on the stream. This catches
>+	 * naive implementation mistakes that simply deleted the backing store
>+	 * from the filesystem in the zero-out step.
>+	 */
>+
>+	ZERO_STRUCT(setfinfo);
>+	unix_to_nt_time(&setfinfo.basic_info.in.write_time, time(NULL));
>+	setfinfo.basic_info.in.attrib = 0x20;
>+	setfinfo.generic.level = RAW_SFILEINFO_BASIC_INFORMATION;
>+	setfinfo.generic.in.file.handle = h1;
>+
>+	status = smb2_setinfo_file(tree, &setfinfo);
>+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
>+					"smb2_getinfo_file failed\n");
>+
>+	ret = check_stream_list(tree, tctx, fname, 1, streams_basic, false);
>+	torture_assert_goto(tctx, ret == true, ret, done, "check_stream_list");
>+
>+	smb2_util_close(tree, h1);
>+	ZERO_STRUCT(h1);
> 
> 	ret = check_stream_list(tree, tctx, fname, 1, streams_basic, false);
> 	torture_assert_goto(tctx, ret == true, ret, done, "Bad streams");
> 
> done:
>+	if (!smb2_util_handle_empty(h1)) {
>+		smb2_util_close(tree, h1);
>+	}
>+	if (!smb2_util_handle_empty(baseh)) {
>+		smb2_util_close(tree, baseh);
>+	}
> 	smb2_util_unlink(tree, fname);
> 	smb2_util_rmdir(tree, BASEDIR);
> 	return ret;
>-- 
>2.13.6
>
>
>From dc39bdd769f3a34e1f120e3c8fcf95dc6ea67eaf Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Thu, 7 Dec 2017 14:56:36 +0100
>Subject: [PATCH 3/5] vfs_fruit: factor out delete_invalid_meta_stream() from
> fruit_streaminfo_meta_stream()
>
>No change in behaviour, just some refactoring before adding more code to
>fruit_streaminfo_meta_stream() in the next commit.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13181
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(backported from commit c41e1ea9247611473d30184efd953c61955ead15)
>---
> source3/modules/vfs_fruit.c | 56 +++++++++++++++++++++++++++------------------
> 1 file changed, 34 insertions(+), 22 deletions(-)
>
>diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
>index f7e57d095c9..92e38fbc8a0 100644
>--- a/source3/modules/vfs_fruit.c
>+++ b/source3/modules/vfs_fruit.c
>@@ -4717,38 +4717,17 @@ static int fruit_fstat(vfs_handle_struct *handle, files_struct *fsp,
> 	return rc;
> }
> 
>-static NTSTATUS fruit_streaminfo_meta_stream(
>+static NTSTATUS delete_invalid_meta_stream(
> 	vfs_handle_struct *handle,
>-	struct files_struct *fsp,
> 	const struct smb_filename *smb_fname,
> 	TALLOC_CTX *mem_ctx,
> 	unsigned int *pnum_streams,
> 	struct stream_struct **pstreams)
> {
>-	struct stream_struct *stream = *pstreams;
>-	unsigned int num_streams = *pnum_streams;
> 	struct smb_filename *sname = NULL;
>-	int i;
> 	int ret;
> 	bool ok;
> 
>-	for (i = 0; i < num_streams; i++) {
>-		if (strequal_m(stream[i].name, AFPINFO_STREAM)) {
>-			break;
>-		}
>-	}
>-
>-	if (i == num_streams) {
>-		return NT_STATUS_OK;
>-	}
>-
>-	if (stream[i].size == AFP_INFO_SIZE) {
>-		return NT_STATUS_OK;
>-	}
>-
>-	DBG_ERR("Removing invalid AFPINFO_STREAM size [%zd] from [%s]\n",
>-		stream[i].size, smb_fname_str_dbg(smb_fname));
>-
> 	ok = del_fruit_stream(mem_ctx, pnum_streams, pstreams, AFPINFO_STREAM);
> 	if (!ok) {
> 		return NT_STATUS_INTERNAL_ERROR;
>@@ -4772,6 +4751,39 @@ static NTSTATUS fruit_streaminfo_meta_stream(
> 	return NT_STATUS_OK;
> }
> 
>+static NTSTATUS fruit_streaminfo_meta_stream(
>+	vfs_handle_struct *handle,
>+	struct files_struct *fsp,
>+	const struct smb_filename *smb_fname,
>+	TALLOC_CTX *mem_ctx,
>+	unsigned int *pnum_streams,
>+	struct stream_struct **pstreams)
>+{
>+	struct stream_struct *stream = *pstreams;
>+	unsigned int num_streams = *pnum_streams;
>+	int i;
>+
>+	for (i = 0; i < num_streams; i++) {
>+		if (strequal_m(stream[i].name, AFPINFO_STREAM)) {
>+			break;
>+		}
>+	}
>+
>+	if (i == num_streams) {
>+		return NT_STATUS_OK;
>+	}
>+
>+	if (stream[i].size != AFP_INFO_SIZE) {
>+		DBG_ERR("Removing invalid AFPINFO_STREAM size [%jd] from [%s]\n",
>+			(intmax_t)stream[i].size, smb_fname_str_dbg(smb_fname));
>+
>+		return delete_invalid_meta_stream(handle, smb_fname, mem_ctx,
>+						  pnum_streams, pstreams);
>+	}
>+
>+	return NT_STATUS_OK;
>+}
>+
> static NTSTATUS fruit_streaminfo_meta_netatalk(
> 	vfs_handle_struct *handle,
> 	struct files_struct *fsp,
>-- 
>2.13.6
>
>
>From bfdc4fb962a831fe0e00facd182ed9666dcee2af Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Thu, 7 Dec 2017 17:32:35 +0100
>Subject: [PATCH 4/5] vfs_fruit: filter out AFP_AfpInfo streams with pending
> delete-on-close
>
>This is in preperation of fixing the implementation of removing the
>AFP_AfpInfo stream by zeroing the FinderInfo out.
>
>We currently remove the stream blob from the underyling filesystem
>backing store, but that results in certain operations to fail on any
>still open file-handle.
>
>The fix comes in the next commit which will convert to backing store
>delete operation to a set delete-on-close on the stream.
>
>This commit adds filtering on streams that have the delete-on-close
>set. It is only needed for the fruit:metadata=stream case, as with
>fruit:metadata=netatalk the filtering is already done in
>fruit_streaminfo_meta_netatalk().
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13181
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 70d8f7c5d25f35b58620c2db8f57c7c0758267b3)
>---
> source3/modules/vfs_fruit.c | 72 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 71 insertions(+), 1 deletion(-)
>
>diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
>index 92e38fbc8a0..18cbd23ba07 100644
>--- a/source3/modules/vfs_fruit.c
>+++ b/source3/modules/vfs_fruit.c
>@@ -4761,7 +4761,16 @@ static NTSTATUS fruit_streaminfo_meta_stream(
> {
> 	struct stream_struct *stream = *pstreams;
> 	unsigned int num_streams = *pnum_streams;
>+	struct smb_filename *sname = NULL;
>+	char *full_name = NULL;
>+	uint32_t name_hash;
>+	struct share_mode_lock *lck = NULL;
>+	struct file_id id = {0};
>+	bool delete_on_close_set;
> 	int i;
>+	int ret;
>+	NTSTATUS status;
>+	bool ok;
> 
> 	for (i = 0; i < num_streams; i++) {
> 		if (strequal_m(stream[i].name, AFPINFO_STREAM)) {
>@@ -4781,7 +4790,68 @@ static NTSTATUS fruit_streaminfo_meta_stream(
> 						  pnum_streams, pstreams);
> 	}
> 
>-	return NT_STATUS_OK;
>+	/*
>+	 * Now check if there's a delete-on-close pending on the stream. If so,
>+	 * hide the stream. This behaviour was verified against a macOS 10.12
>+	 * SMB server.
>+	 */
>+
>+	sname = synthetic_smb_fname(talloc_tos(),
>+				    smb_fname->base_name,
>+				    AFPINFO_STREAM_NAME,
>+				    NULL, 0);
>+	if (sname == NULL) {
>+		status = NT_STATUS_NO_MEMORY;
>+		goto out;
>+	}
>+
>+	ret = SMB_VFS_NEXT_STAT(handle, sname);
>+	if (ret != 0) {
>+		status = map_nt_error_from_unix(errno);
>+		goto out;
>+	}
>+
>+	id = SMB_VFS_NEXT_FILE_ID_CREATE(handle, &sname->st);
>+
>+	lck = get_existing_share_mode_lock(talloc_tos(), id);
>+	if (lck == NULL) {
>+		status = NT_STATUS_OK;
>+		goto out;
>+	}
>+
>+	full_name = talloc_asprintf(talloc_tos(),
>+				    "%s%s",
>+				    sname->base_name,
>+				    AFPINFO_STREAM);
>+	if (full_name == NULL) {
>+		status = NT_STATUS_NO_MEMORY;
>+		goto out;
>+	}
>+
>+	status = file_name_hash(handle->conn, full_name, &name_hash);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		goto out;
>+	}
>+
>+	delete_on_close_set = is_delete_on_close_set(lck, name_hash);
>+	if (delete_on_close_set) {
>+		ok = del_fruit_stream(mem_ctx,
>+				      pnum_streams,
>+				      pstreams,
>+				      AFPINFO_STREAM);
>+		if (!ok) {
>+			status = NT_STATUS_INTERNAL_ERROR;
>+			goto out;
>+		}
>+	}
>+
>+	status  = NT_STATUS_OK;
>+
>+out:
>+	TALLOC_FREE(sname);
>+	TALLOC_FREE(lck);
>+	TALLOC_FREE(full_name);
>+	return status;
> }
> 
> static NTSTATUS fruit_streaminfo_meta_netatalk(
>-- 
>2.13.6
>
>
>From 5ccaee33970906e6dae192073584bb51fa1f9d4f Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Wed, 6 Dec 2017 22:09:52 +0100
>Subject: [PATCH 5/5] vfs_fruit: set delete-on-close for empty finderinfo
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>We previously removed the stream from the underlying filesystem stream
>backing store when the client zeroes out FinderInfo in the AFP_AfpInfo
>stream, but this causes certain operations to fail (eg stat) when trying
>to access the stream over any file-handle open on that stream.
>
>So instead of deleting, set delete-on-close on the stream. The previous
>commit already implemented not to list list streams with delete-on-close
>set which is necessary to implemenent correct macOS semantics for this
>particular stream.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13181
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Ralph BÃ¶hme <slow@samba.org>
>Autobuild-Date(master): Tue Jan  9 17:09:12 CET 2018 on sn-devel-144
>
>(backported from commit e61e9e98e9ff461055daae2fe78f0202f7ed8663)
>---
> selftest/knownfail.d/samba3.vfs.fruit |  2 --
> source3/modules/vfs_fruit.c           | 60 +++++++++++++++++++++--------------
> 2 files changed, 36 insertions(+), 26 deletions(-)
> delete mode 100644 selftest/knownfail.d/samba3.vfs.fruit
>
>diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit
>deleted file mode 100644
>index 1ba64febb20..00000000000
>--- a/selftest/knownfail.d/samba3.vfs.fruit
>+++ /dev/null
>@@ -1,2 +0,0 @@
>-^samba3.vfs.fruit metadata_stream.delete AFP_AfpInfo by writing all 0$nt4_dc$
>-^samba3.vfs.fruit streams_depot.delete AFP_AfpInfo by writing all 0$nt4_dc$
>diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
>index 18cbd23ba07..7ae51ee3f6f 100644
>--- a/source3/modules/vfs_fruit.c
>+++ b/source3/modules/vfs_fruit.c
>@@ -4065,26 +4065,35 @@ static ssize_t fruit_pwrite_meta_stream(vfs_handle_struct *handle,
> 					size_t n, off_t offset)
> {
> 	AfpInfo *ai = NULL;
>-	int ret;
>+	size_t nwritten;
>+	bool ok;
> 
> 	ai = afpinfo_unpack(talloc_tos(), data);
> 	if (ai == NULL) {
> 		return -1;
> 	}
> 
>-	if (ai_empty_finderinfo(ai)) {
>-		ret = SMB_VFS_NEXT_UNLINK(handle, fsp->fsp_name);
>-		if (ret != 0 && errno != ENOENT && errno != ENOATTR) {
>-			DBG_ERR("Can't delete metadata for %s: %s\n",
>-				fsp_str_dbg(fsp), strerror(errno));
>-			TALLOC_FREE(ai);
>-			return -1;
>-		}
>+	nwritten = SMB_VFS_NEXT_PWRITE(handle, fsp, data, n, offset);
>+	if (nwritten != n) {
>+		return -1;
>+	}
> 
>+	if (!ai_empty_finderinfo(ai)) {
> 		return n;
> 	}
> 
>-	return SMB_VFS_NEXT_PWRITE(handle, fsp, data, n, offset);
>+	ok = set_delete_on_close(
>+			fsp,
>+			true,
>+			handle->conn->session_info->security_token,
>+			handle->conn->session_info->unix_token);
>+	if (!ok) {
>+		DBG_ERR("set_delete_on_close on [%s] failed\n",
>+			fsp_str_dbg(fsp));
>+		return -1;
>+	}
>+
>+	return n;
> }
> 
> static ssize_t fruit_pwrite_meta_netatalk(vfs_handle_struct *handle,
>@@ -4095,26 +4104,13 @@ static ssize_t fruit_pwrite_meta_netatalk(vfs_handle_struct *handle,
> 	AfpInfo *ai = NULL;
> 	char *p = NULL;
> 	int ret;
>+	bool ok;
> 
> 	ai = afpinfo_unpack(talloc_tos(), data);
> 	if (ai == NULL) {
> 		return -1;
> 	}
> 
>-	if (ai_empty_finderinfo(ai)) {
>-		ret = SMB_VFS_REMOVEXATTR(handle->conn,
>-					  fsp->fsp_name->base_name,
>-					  AFPINFO_EA_NETATALK);
>-
>-		if (ret != 0 && errno != ENOENT && errno != ENOATTR) {
>-			DBG_ERR("Can't delete metadata for %s: %s\n",
>-				fsp_str_dbg(fsp), strerror(errno));
>-			return -1;
>-		}
>-
>-		return n;
>-	}
>-
> 	ad = ad_fget(talloc_tos(), handle, fsp, ADOUBLE_META);
> 	if (ad == NULL) {
> 		ad = ad_init(talloc_tos(), handle, ADOUBLE_META);
>@@ -4139,6 +4135,22 @@ static ssize_t fruit_pwrite_meta_netatalk(vfs_handle_struct *handle,
> 	}
> 
> 	TALLOC_FREE(ad);
>+
>+	if (!ai_empty_finderinfo(ai)) {
>+		return n;
>+	}
>+
>+	ok = set_delete_on_close(
>+		fsp,
>+		true,
>+		handle->conn->session_info->security_token,
>+		handle->conn->session_info->unix_token);
>+	if (!ok) {
>+		DBG_ERR("set_delete_on_close on [%s] failed\n",
>+			fsp_str_dbg(fsp));
>+		return -1;
>+	}
>+
> 	return n;
> }
> 
>-- 
>2.13.6
>

Flags:

jra: review+

Actions: View

Attachments on bug 13181: 13910 | 13911