Attachment 15195 Details for Bug 13958 – Patch for 4.9 and 4.10 cherry-picked from master

[patch] Patch for 4.9 and 4.10 cherry-picked from master

bug13958-v49,v410.patch (text/plain), 14.56 KB, created by Ralph Böhme on 2019-05-27 13:17:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2019-05-27 13:17:05 UTC

Size: 14.56 KB

patch

obsolete

>From b0c9f6956729e42ea96fc7295950ea7f8d3e3c62 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Tue, 21 May 2019 18:39:52 +0200
>Subject: [PATCH 1/4] s4:torture/vfs/fruit: ensure
> test_adouble_conversion_wo_xattr() uses a non-emtpy resourcefork
>
>This ensures the resource fork is not deleted as part of the AppleDouble file
>conversion for the option fruit:wipe_intentionally_left_blank_rfork=yes.
>
>This is currently not a problem in selftest, as we don't enable the option, but
>a subsequent commit will run all vfs.fruit tests against a share with this
>option enabled.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13958
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit bb5a457f2872a383b58d62981dade322fca9b283)
>---
> source4/torture/vfs/fruit.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
>diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
>index e0ba935ff99..fa47fcca8fa 100644
>--- a/source4/torture/vfs/fruit.c
>+++ b/source4/torture/vfs/fruit.c
>@@ -902,7 +902,7 @@ static char osx_adouble_w_xattr[] = {
>  *
>  * -RAW DUMP--:  0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F : (ASCII)
>  * 00000000   : 00 00 01 00 00 00 01 00 00 00 00 00 00 00 00 1E : ................
>- * 00000010   : 54 68 69 73 20 72 65 73 6F 75 72 63 65 20 66 6F : This resource fo
>+ * 00000010   : F0 F1 F2 F3 F5 F5 F6 F7 F8 F9 FA FB FC FD FE FF : ................
>  * 00000020   : 72 6B 20 69 6E 74 65 6E 74 69 6F 6E 61 6C 6C 79 : rk intentionally
>  * 00000030   : 20 6C 65 66 74 20 62 6C 61 6E 6B 20 20 20 00 00 :  left blank   ..
>  * 00000040   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
>@@ -991,9 +991,9 @@ static char osx_adouble_without_xattr[] = {
> 	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> 	0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
> 	0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>-	0x00, 0x1e, 0x54, 0x68, 0x69, 0x73, 0x20, 0x72,
>-	0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x20,
>-	0x66, 0x6f, 0x72, 0x6b, 0x20, 0x69, 0x6e, 0x74,
>+	0x00, 0x1e, 0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5,
>+	0xf6, 0xf7, 0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd,
>+	0xfe, 0xff, 0x72, 0x6b, 0x20, 0x69, 0x6e, 0x74,
> 	0x65, 0x6e, 0x74, 0x69, 0x6f, 0x6e, 0x61, 0x6c,
> 	0x6c, 0x79, 0x20, 0x6c, 0x65, 0x66, 0x74, 0x20,
> 	0x62, 0x6c, 0x61, 0x6e, 0x6b, 0x20, 0x20, 0x20,
>@@ -2168,8 +2168,11 @@ static bool test_adouble_conversion_wo_xattr(struct torture_context *tctx,
> 	struct smb2_find find;
> 	unsigned int count;
> 	union smb_search_data *d;
>-	const char *data = "This resource fork intentionally left blank";
>-	size_t datalen = strlen(data);
>+	const char data[] = {
>+		0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7,
>+		0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff
>+	};
>+	size_t datalen = sizeof(data);
> 	bool is_osx = torture_setting_bool(tctx, "osx", false);
> 
> 	if (is_osx) {
>-- 
>2.21.0
>
>
>From 57de2dc671aa06ef60e648d734c25780e70109b6 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Tue, 21 May 2019 14:05:04 +0200
>Subject: [PATCH 2/4] selftest: run vfs.fruit test against a share that deletes
> empty resource forks
>
>This reveals a bug in the AppleDouble conversion code: the conversion code that
>unlinks an empty resource fork AppleDouble sidecar file ("._file") gets
>triggered as part of open_file_ntcreate(..., "file:AFP_AfpResource", ...):
>
>after SMB_VFS_OPEN() has been called with O_CREAT, what created the file, we
>call SMB_VFS_FSTAT() on the just created filehandle. This ends up in
>ad_convert(), finds the resource fork empty and thus deletes the file.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13958
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 8ed9b6b457923d2353d1d18838f4a278db48c6b9)
>---
> selftest/knownfail.d/samba3.vfs.fruit | 13 +++++++++++++
> source3/selftest/tests.py             |  1 +
> 2 files changed, 14 insertions(+)
>
>diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit
>index 6307e2b3404..b1a28bedff6 100644
>--- a/selftest/knownfail.d/samba3.vfs.fruit
>+++ b/selftest/knownfail.d/samba3.vfs.fruit
>@@ -1,2 +1,15 @@
> ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion$nt4_dc$
> ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr$nt4_dc$
>+^samba3.vfs.fruit fruit_delete_empty_adfiles.copyfile$nt4_dc$
>+^samba3.vfs.fruit fruit_delete_empty_adfiles.resource fork IO$nt4_dc$
>+^samba3.vfs.fruit fruit_delete_empty_adfiles.SMB2/CREATE context AAPL$nt4_dc$
>+^samba3.vfs.fruit fruit_delete_empty_adfiles.truncate resource fork to 0 bytes$nt4_dc$
>+^samba3.vfs.fruit fruit_delete_empty_adfiles.opening and creating resource fork$nt4_dc$
>+^samba3.vfs.fruit fruit_delete_empty_adfiles.create delete-on-close AFP_AfpResource$nt4_dc$
>+^samba3.vfs.fruit fruit_delete_empty_adfiles.setinfo delete-on-close AFP_AfpResource$nt4_dc$
>+^samba3.vfs.fruit fruit_delete_empty_adfiles.delete$nt4_dc$
>+^samba3.vfs.fruit fruit_delete_empty_adfiles.read open rsrc after rename$nt4_dc$
>+^samba3.vfs.fruit fruit_delete_empty_adfiles.readdir_attr with names with illegal ntfs characters$nt4_dc$
>+^samba3.vfs.fruit fruit_delete_empty_adfiles.copy-chunk streams$nt4_dc$
>+^samba3.vfs.fruit fruit_delete_empty_adfiles.empty_stream$nt4_dc$
>+^samba3.vfs.fruit fruit_delete_empty_adfiles.setinfo eof AFP_AfpResource$nt4_dc$
>diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>index 7067abc5fb4..1db806ef887 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -595,6 +595,7 @@ tests = base + raw + smb2 + rpc + unix + local + rap + nbt + libsmbclient + idma
>         plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/vfs_fruit -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/nt4_dc/share --option=torture:share2=vfs_wo_fruit', 'metadata_netatalk')
>         plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/vfs_fruit_metadata_stream -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/nt4_dc/share --option=torture:share2=vfs_wo_fruit', 'metadata_stream')
>         plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/vfs_fruit_stream_depot -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/nt4_dc/share --option=torture:share2=vfs_wo_fruit_stream_depot', 'streams_depot')
>+        plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/vfs_fruit_delete_empty_adfiles -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/nt4_dc/share --option=torture:share2=vfs_wo_fruit', 'fruit_delete_empty_adfiles')
>     elif t == "vfs.fruit_netatalk":
>         plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/vfs_fruit_xattr -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/nt4_dc/share')
>     elif t == "vfs.fruit_timemachine":
>-- 
>2.21.0
>
>
>From 47d8aaa3432cc2c4f33f5244ab473558190cfb73 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Tue, 21 May 2019 16:00:00 +0200
>Subject: [PATCH 3/4] vfs_fruit: add a forward declaration for ad_get()
>
>Will be needed in the next commit.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13958
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 4777d1163a7c18c89ce9be955903427a18134415)
>---
> source3/modules/vfs_fruit.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
>index f54038f53d4..b74d26ca711 100644
>--- a/source3/modules/vfs_fruit.c
>+++ b/source3/modules/vfs_fruit.c
>@@ -535,6 +535,10 @@ struct fio {
>  */
> static struct adouble *ad_init(TALLOC_CTX *ctx, vfs_handle_struct *handle,
> 			       adouble_type_t type);
>+static struct adouble *ad_get(TALLOC_CTX *ctx,
>+			      vfs_handle_struct *handle,
>+			      const struct smb_filename *smb_fname,
>+			      adouble_type_t type);
> static int ad_set(struct adouble *ad, const struct smb_filename *smb_fname);
> static int ad_fset(struct adouble *ad, files_struct *fsp);
> static int adouble_path(TALLOC_CTX *ctx,
>-- 
>2.21.0
>
>
>From b329b96634e9d415491d4eb2c05d1efb0b249b40 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Tue, 21 May 2019 16:00:53 +0200
>Subject: [PATCH 4/4] vfs_fruit: change trigger points of AppleDouble
> conversion
>
>This moves the trigger points where AppleDouble file conversion is run by
>ad_convert() from deep down the callchain in ad_read_rsrc_adouble() to high
>level VFS entry points.
>
>Currently ad_convert() will be triggered as part of open_file_ntcreate(...,
>"file:AFP_AfpResource", ...): after SMB_VFS_OPEN() has been called with O_CREAT,
>what created the file, we call SMB_VFS_FSTAT() on the just created
>filehandle. This ends up in ad_convert(), finds the resource fork empty and thus
>deletes the file.
>
>This commit moves calling of the conversion funtion to the high level VFS entry
>points where the converted metadata is needed:
>
>o for directory enumerations SMB_VFS_READDIR_ATTR() is called to fill in the
>  repurposed fields in the directory entry metadata
>
>o obviously for SMB_VFS_CREATE_FILE() on an macOS stream
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13958
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 78a4639b2d06cc69788861618d2e91945e142d2b)
>---
> selftest/knownfail.d/samba3.vfs.fruit | 13 ------
> source3/modules/vfs_fruit.c           | 61 ++++++++++++++++++---------
> 2 files changed, 40 insertions(+), 34 deletions(-)
>
>diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit
>index b1a28bedff6..6307e2b3404 100644
>--- a/selftest/knownfail.d/samba3.vfs.fruit
>+++ b/selftest/knownfail.d/samba3.vfs.fruit
>@@ -1,15 +1,2 @@
> ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion$nt4_dc$
> ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr$nt4_dc$
>-^samba3.vfs.fruit fruit_delete_empty_adfiles.copyfile$nt4_dc$
>-^samba3.vfs.fruit fruit_delete_empty_adfiles.resource fork IO$nt4_dc$
>-^samba3.vfs.fruit fruit_delete_empty_adfiles.SMB2/CREATE context AAPL$nt4_dc$
>-^samba3.vfs.fruit fruit_delete_empty_adfiles.truncate resource fork to 0 bytes$nt4_dc$
>-^samba3.vfs.fruit fruit_delete_empty_adfiles.opening and creating resource fork$nt4_dc$
>-^samba3.vfs.fruit fruit_delete_empty_adfiles.create delete-on-close AFP_AfpResource$nt4_dc$
>-^samba3.vfs.fruit fruit_delete_empty_adfiles.setinfo delete-on-close AFP_AfpResource$nt4_dc$
>-^samba3.vfs.fruit fruit_delete_empty_adfiles.delete$nt4_dc$
>-^samba3.vfs.fruit fruit_delete_empty_adfiles.read open rsrc after rename$nt4_dc$
>-^samba3.vfs.fruit fruit_delete_empty_adfiles.readdir_attr with names with illegal ntfs characters$nt4_dc$
>-^samba3.vfs.fruit fruit_delete_empty_adfiles.copy-chunk streams$nt4_dc$
>-^samba3.vfs.fruit fruit_delete_empty_adfiles.empty_stream$nt4_dc$
>-^samba3.vfs.fruit fruit_delete_empty_adfiles.setinfo eof AFP_AfpResource$nt4_dc$
>diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
>index b74d26ca711..be85c9f5412 100644
>--- a/source3/modules/vfs_fruit.c
>+++ b/source3/modules/vfs_fruit.c
>@@ -1450,27 +1450,37 @@ static bool ad_convert_delete_adfile(struct adouble *ad,
>  * @return -1 in case an error occurred, 0 if no conversion was done, 1
>  * otherwise
>  **/
>-static int ad_convert(struct adouble *ad,
>+static int ad_convert(struct vfs_handle_struct *handle,
> 		      const struct smb_filename *smb_fname)
> {
>+	struct adouble *ad = NULL;
> 	bool ok;
> 	bool converted_xattr = false;
> 	bool blank;
>+	int ret;
>+
>+	ad = ad_get(talloc_tos(), handle, smb_fname, ADOUBLE_RSRC);
>+	if (ad == NULL) {
>+		return 0;
>+	}
> 
> 	ok = ad_convert_xattr(ad, smb_fname, &converted_xattr);
> 	if (!ok) {
>-		return -1;
>+		ret = -1;
>+		goto done;
> 	}
> 
> 	ok = ad_convert_blank_rfork(ad, &blank);
> 	if (!ok) {
>-		return -1;
>+		ret = -1;
>+		goto done;
> 	}
> 
> 	if (converted_xattr || blank) {
> 		ok = ad_convert_truncate(ad, smb_fname);
> 		if (!ok) {
>-			return -1;
>+			ret = -1;
>+			goto done;
> 		}
> 	}
> 
>@@ -1478,15 +1488,20 @@ static int ad_convert(struct adouble *ad,
> 	if (!ok) {
> 		DBG_ERR("Failed to convert [%s]\n",
> 			smb_fname_str_dbg(smb_fname));
>-		return -1;
>+		ret = -1;
>+		goto done;
> 	}
> 
> 	ok = ad_convert_delete_adfile(ad, smb_fname);
> 	if (!ok) {
>-		return -1;
>+		ret = -1;
>+		goto done;
> 	}
> 
>-	return 0;
>+	ret = 0;
>+done:
>+	TALLOC_FREE(ad);
>+	return ret;
> }
> 
> /**
>@@ -1739,17 +1754,6 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
> 		return -1;
> 	}
> 
>-	/*
>-	 * Try to fixup AppleDouble files created by OS X with xattrs
>-	 * appended to the ADEID_FINDERI entry.
>-	 */
>-
>-	ret = ad_convert(ad, smb_fname);
>-	if (ret != 0) {
>-		DBG_WARNING("Failed to convert [%s]\n", smb_fname->base_name);
>-		return len;
>-	}
>-
> 	return len;
> }
> 
>@@ -2137,9 +2141,8 @@ static bool is_afpresource_stream(const struct smb_filename *smb_fname)
> }
> 
> /**
>- * Test whether stream is an Apple stream, not used atm
>+ * Test whether stream is an Apple stream.
>  **/
>-#if 0
> static bool is_apple_stream(const struct smb_filename *smb_fname)
> {
> 	if (is_afpinfo_stream(smb_fname)) {
>@@ -2150,7 +2153,6 @@ static bool is_apple_stream(const struct smb_filename *smb_fname)
> 	}
> 	return false;
> }
>-#endif
> 
> /**
>  * Initialize config struct from our smb.conf config parameters
>@@ -6051,6 +6053,8 @@ static NTSTATUS fruit_create_file(vfs_handle_struct *handle,
> 	struct fruit_config_data *config = NULL;
> 	files_struct *fsp = NULL;
> 	struct fio *fio = NULL;
>+	bool internal_open = (oplock_request & INTERNAL_OPEN_ONLY);
>+	int ret;
> 
> 	status = check_aapl(handle, req, in_context_blobs, out_context_blobs);
> 	if (!NT_STATUS_IS_OK(status)) {
>@@ -6060,6 +6064,14 @@ static NTSTATUS fruit_create_file(vfs_handle_struct *handle,
> 	SMB_VFS_HANDLE_GET_DATA(handle, config, struct fruit_config_data,
> 				return NT_STATUS_UNSUCCESSFUL);
> 
>+	if (is_apple_stream(smb_fname) && !internal_open) {
>+		ret = ad_convert(handle, smb_fname);
>+		if (ret != 0) {
>+			DBG_ERR("ad_convert() failed\n");
>+			return NT_STATUS_UNSUCCESSFUL;
>+		}
>+	}
>+
> 	status = SMB_VFS_NEXT_CREATE_FILE(
> 		handle, req, root_dir_fid, smb_fname,
> 		access_mask, share_access,
>@@ -6142,6 +6154,7 @@ static NTSTATUS fruit_readdir_attr(struct vfs_handle_struct *handle,
> 	struct fruit_config_data *config = NULL;
> 	struct readdir_attr_data *attr_data;
> 	NTSTATUS status;
>+	int ret;
> 
> 	SMB_VFS_HANDLE_GET_DATA(handle, config,
> 				struct fruit_config_data,
>@@ -6153,6 +6166,12 @@ static NTSTATUS fruit_readdir_attr(struct vfs_handle_struct *handle,
> 
> 	DEBUG(10, ("fruit_readdir_attr %s\n", fname->base_name));
> 
>+	ret = ad_convert(handle, fname);
>+	if (ret != 0) {
>+		DBG_ERR("ad_convert() failed\n");
>+		return NT_STATUS_UNSUCCESSFUL;
>+	}
>+
> 	*pattr_data = talloc_zero(mem_ctx, struct readdir_attr_data);
> 	if (*pattr_data == NULL) {
> 		return NT_STATUS_UNSUCCESSFUL;
>-- 
>2.21.0
>

Flags:

jra: review+

Actions: View

Attachments on bug 13958: 15195 | 15221