[PATCH] Fix AppleDouble conversion in vfs_fruit

Ralph Böhme slow at samba.org
Tue Oct 9 11:03:50 UTC 2018


On Tue, Oct 09, 2018 at 01:00:23PM +0200, Ralph Böhme via samba-technical wrote:
>On Tue, Oct 09, 2018 at 12:35:22PM +0200, Ralph Böhme wrote:
>>please hold on, something's fishy. This was passing manual make test 
>>vfs.fruit whereas it's now failing. Checking...
>
>The culprit was a last minute change in 3/21 to use a pointer instead 
>of an array to store the filler bytes. The problem is that whenever 
>ad_pack() is called it overwrites the original filler bytes.
>
>So attached is the original version of the patchset without this 
>change. Passes manual make test vfs.fruit again.
>
>CI: https://gitlab.com/samba-team/devel/samba/pipelines/32396036

and here's the patch.

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG Key Fingerprint:           FAE2 C608 8A24 2520 51C5
                               59E4 AA1E 9B71 2639 9E46
-------------- next part --------------
From e46529da5cfacc9032f10b6c6bea67028d904dd7 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 7 Oct 2018 18:26:47 +0200
Subject: [PATCH 01/21] s4:torture: FinderInfo conversion test with AppleDouble
 without xattr data

This testcase demonstrates that the AppleDouble conversion in vfs_fruit
doesn't correctly convert the FinderInfo data from the AppleDouble file
to a stream.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 selftest/knownfail.d/samba3.vfs.fruit |   3 +
 source4/torture/vfs/fruit.c           | 258 ++++++++++++++++++++++++++++++++++
 2 files changed, 261 insertions(+)

diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit
index 8df25bccb79..bc46c2f4922 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.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
+^samba3.vfs.fruit metadata_stream.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
+^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
index a69ab7e5350..8d12a4c16e5 100644
--- a/source4/torture/vfs/fruit.c
+++ b/source4/torture/vfs/fruit.c
@@ -886,6 +886,147 @@ static char osx_adouble_w_xattr[] = {
 	0x00, 0x00, 0x00, 0x1c, 0x00, 0x1e, 0xff, 0xff
 };
 
+/*
+ * The buf below contains the following AppleDouble encoded data:
+ *
+ * -------------------------------------------------------------------------------
+ * MagicNumber: 00051607                                        : AppleDouble
+ * Version    : 00020000                                        : Version 2
+ * Filler     : 4D 61 63 20 4F 53 20 58 20 20 20 20 20 20 20 20 : Mac OS X
+ * Num. of ent: 0002                                            : 2
+ *
+ * -------------------------------------------------------------------------------
+ * Entry ID   : 00000002 : Resource Fork
+ * Offset     : 00000052 : 82
+ * Length     : 0000011E : 286
+ *
+ * -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
+ * 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 : ................
+ * 00000050   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000060   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000070   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000080   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000090   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000A0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000B0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000C0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000D0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000E0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000F0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000100   : 00 00 01 00 00 00 01 00 00 00 00 00 00 00 00 1E : ................
+ * 00000110   : 00 00 00 00 00 00 00 00 00 1C 00 1E FF FF       : ..............
+ *
+ * Entry ID   : 00000009 : Finder Info
+ * Offset     : 00000032 : 50
+ * Length     : 00000020 : 32
+ *
+ * -NOTE------: cannot detect whether FInfo or DInfo. assume FInfo.
+ *
+ * -FInfo-----:
+ * Type       : 57415645 : WAVE
+ * Creator    : 5054756C : PTul
+ * isAlias    : 0
+ * Invisible  : 0
+ * hasBundle  : 0
+ * nameLocked : 0
+ * Stationery : 0
+ * CustomIcon : 0
+ * Reserved   : 0
+ * Inited     : 0
+ * NoINITS    : 0
+ * Shared     : 0
+ * SwitchLaunc: 0
+ * Hidden Ext : 0
+ * color      : 000      : none
+ * isOnDesk   : 0
+ * Location v : 0000     : 0
+ * Location h : 0000     : 0
+ * Fldr       : 0000     : ..
+ *
+ * -FXInfo----:
+ * Rsvd|IconID: 0000     : 0
+ * Rsvd       : 0000     : ..
+ * Rsvd       : 0000     : ..
+ * Rsvd       : 0000     : ..
+ * AreInvalid : 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * CustomBadge: 0
+ * ObjctIsBusy: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * RoutingInfo: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * Rsvd|commnt: 0000     : 0
+ * PutAway    : 00000000 : 0
+ *
+ * -RAW DUMP--:  0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F : (ASCII)
+ * 00000000   : 57 41 56 45 50 54 75 6C 00 00 00 00 00 00 00 00 : WAVEPTul........
+ * 00000010   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ *  *
+ * It was created with:
+ * $ hexdump -ve '"\t" 7/1 "0x%02x, " 1/1 " 0x%02x," "\n"'
+ */
+static char osx_adouble_without_xattr[] = {
+	0x00, 0x05, 0x16, 0x07, 0x00, 0x02, 0x00, 0x00,
+	0x4d, 0x61, 0x63, 0x20, 0x4f, 0x53, 0x20, 0x58,
+	0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+	0x00, 0x02, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00,
+	0x00, 0x52, 0x00, 0x00, 0x01, 0x1e, 0x00, 0x00,
+	0x00, 0x09, 0x00, 0x00, 0x00, 0x32, 0x00, 0x00,
+	0x00, 0x20, 0x57, 0x41, 0x56, 0x45, 0x50, 0x54,
+	0x75, 0x6c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	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,
+	0x65, 0x6e, 0x74, 0x69, 0x6f, 0x6e, 0x61, 0x6c,
+	0x6c, 0x79, 0x20, 0x6c, 0x65, 0x66, 0x74, 0x20,
+	0x62, 0x6c, 0x61, 0x6e, 0x6b, 0x20, 0x20, 0x20,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	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, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x1c, 0x00, 0x1e, 0xff, 0xff
+};
+
 /**
  * talloc and intialize an AfpInfo
  **/
@@ -2021,6 +2162,122 @@ static bool test_adouble_conversion(struct torture_context *tctx,
 	return ret;
 }
 
+/*
+ * Test conversion of AppleDouble file without embedded xattr data
+ */
+static bool test_adouble_conversion_wo_xattr(struct torture_context *tctx,
+					     struct smb2_tree *tree)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
+	const char *fname = BASEDIR "\\test_adouble_conversion";
+	const char *adname = BASEDIR "/._test_adouble_conversion";
+	NTSTATUS status;
+	struct smb2_handle testdirh;
+	bool ret = true;
+	const char *streams[] = {
+		"::$DATA",
+		AFPINFO_STREAM,
+		AFPRESOURCE_STREAM
+	};
+	struct smb2_create create;
+	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);
+
+	smb2_deltree(tree, BASEDIR);
+
+	status = torture_smb2_testdir(tree, BASEDIR, &testdirh);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"torture_smb2_testdir failed\n");
+	smb2_util_close(tree, testdirh);
+
+	ret = torture_setup_file(tctx, tree, fname, false);
+	torture_assert_goto(tctx, ret == true, ret, done,
+			    "torture_setup_file failed\n");
+
+	ret = torture_setup_file(tctx, tree, adname, false);
+	torture_assert_goto(tctx, ret == true, ret, done,
+			    "torture_setup_file failed\n");
+
+	ret = write_stream(tree, __location__, tctx, mem_ctx,
+			   adname, NULL, 0,
+			   sizeof(osx_adouble_without_xattr),
+			   osx_adouble_without_xattr);
+	torture_assert_goto(tctx, ret == true, ret, done,
+			    "write_stream failed\n");
+
+	ret = enable_aapl(tctx, tree);
+	torture_assert_goto(tctx, ret == true, ret, done, "enable_aapl failed");
+
+	/*
+	 * Issue a smb2_find(), this triggers the server-side conversion
+	 */
+
+	create = (struct smb2_create) {
+		.in.desired_access = SEC_RIGHTS_DIR_READ,
+		.in.create_options = NTCREATEX_OPTIONS_DIRECTORY,
+		.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY,
+		.in.share_access = NTCREATEX_SHARE_ACCESS_READ,
+		.in.create_disposition = NTCREATEX_DISP_OPEN,
+		.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS,
+		.in.fname = BASEDIR,
+	};
+
+	status = smb2_create(tree, tctx, &create);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_create failed\n");
+
+	find = (struct smb2_find) {
+		.in.file.handle = create.out.file.handle,
+		.in.pattern = "*",
+		.in.max_response_size = 0x1000,
+		.in.level = SMB2_FIND_ID_BOTH_DIRECTORY_INFO,
+	};
+
+	status = smb2_find_level(tree, tree, &find, &count, &d);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_find_level failed\n");
+
+	status = smb2_util_close(tree, create.out.file.handle);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_util_close failed");
+
+	/*
+	 * Check number of streams
+	 */
+
+	ret = check_stream_list(tree, tctx, fname, 3, streams, false);
+	torture_assert_goto(tctx, ret == true, ret, done, "check_stream_list");
+
+
+	/*
+	 * Check Resourcefork data can be read.
+	 */
+
+	ret = check_stream(tree, __location__, tctx, mem_ctx,
+			   fname, AFPRESOURCE_STREAM,
+			   16, datalen, 0, datalen, data);
+	torture_assert_goto(tctx, ret == true, ret, done,
+			    "check AFPRESOURCE_STREAM failed\n");
+
+	/*
+	 * Check FinderInfo data has been migrated to stream.
+	 */
+
+	ret = check_stream(tree, __location__, tctx, mem_ctx,
+			   fname, AFPINFO_STREAM,
+			   0, 60, 16, 8, "WAVEPTul");
+	torture_assert_goto(tctx, ret == true, ret, done,
+			    "check AFPINFO_STREAM failed\n");
+
+done:
+	smb2_deltree(tree, BASEDIR);
+	talloc_free(mem_ctx);
+	return ret;
+}
+
 static bool test_aapl(struct torture_context *tctx,
 		      struct smb2_tree *tree)
 {
@@ -4866,6 +5123,7 @@ struct torture_suite *torture_vfs_fruit(TALLOC_CTX *ctx)
 	torture_suite_add_1smb2_test(suite, "copy-chunk streams", test_copy_chunk_streams);
 	torture_suite_add_1smb2_test(suite, "OS X AppleDouble file conversion", test_adouble_conversion);
 	torture_suite_add_1smb2_test(suite, "NFS ACE entries", test_nfs_aces);
+	torture_suite_add_1smb2_test(suite, "OS X AppleDouble file conversion without embedded xattr", test_adouble_conversion_wo_xattr);
 
 	return suite;
 }
-- 
2.13.6


From e830d0d0947432ce3f9b4c2bd1532004b77495b5 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 11 Sep 2018 14:05:43 +0200
Subject: [PATCH 02/21] vfs_fruit: fix two comments

Thanks to the recent addition of ad_convert_xattr() we now correctly
handle this case.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 894c361a05f..65d6618ff2d 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1055,8 +1055,7 @@ static bool ad_convert_xattr(struct adouble *ad,
  * Convert from Apple's ._ file to Netatalk
  *
  * Apple's AppleDouble may contain a FinderInfo entry longer then 32
- * bytes containing packed xattrs. Netatalk can't deal with that, so
- * we simply discard the packed xattrs.
+ * bytes containing packed xattrs.
  *
  * @return -1 in case an error occurred, 0 if no conversion was done, 1
  * otherwise
@@ -1374,9 +1373,7 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 
 	/*
 	 * Try to fixup AppleDouble files created by OS X with xattrs
-	 * appended to the ADEID_FINDERI entry. We simply remove the
-	 * xattrs blob, this means any fancy xattr that was stored
-	 * there is lost.
+	 * appended to the ADEID_FINDERI entry.
 	 */
 
 	ret = ad_convert(ad, smb_fname, ad->ad_fd);
-- 
2.13.6


From ed5e78a7411a544a3829ca5e6de8ce730bb9fbff Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 15:12:44 +0200
Subject: [PATCH 03/21] vfs_fruit: store filler bytes from AppleDouble file
 header in struct adouble

This can later be used to distinguish between macOS created AppleDouble
files and AppleDouble files created by Samba or Netatalk.

macOS:    "Mac OS X        "
Samba:    "Netatalk        "
Netatalk: "Netatalk        "

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 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 65d6618ff2d..057e4e4265b 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -262,6 +262,7 @@ typedef enum {ADOUBLE_META, ADOUBLE_RSRC} adouble_type_t;
 #define ADEDLEN_VERSION     4
 #define ADEDLEN_FILLER      16
 #define AD_FILLER_TAG       "Netatalk        " /* should be 16 bytes */
+#define AD_FILLER_TAG_OSX   "Mac OS X        " /* should be 16 bytes */
 #define ADEDLEN_NENTRIES    2
 #define AD_HEADER_LEN       (ADEDLEN_MAGIC + ADEDLEN_VERSION + \
 			     ADEDLEN_FILLER + ADEDLEN_NENTRIES) /* 26 */
@@ -414,6 +415,7 @@ struct adouble {
 	adouble_type_t            ad_type;
 	uint32_t                  ad_magic;
 	uint32_t                  ad_version;
+	uint8_t                   ad_filler[ADEDLEN_FILLER];
 	struct ad_entry           ad_eid[ADEID_MAX];
 	char                     *ad_data;
 	struct ad_xattr_header    adx_header;
@@ -837,6 +839,8 @@ static bool ad_unpack(struct adouble *ad, const size_t nentries,
 		return false;
 	}
 
+	memcpy(ad->ad_filler, ad->ad_data + ADEDOFF_FILLER, ADEDLEN_FILLER);
+
 	adentries = RSVAL(ad->ad_data, ADEDOFF_NENTRIES);
 	if (adentries != nentries) {
 		DEBUG(1, ("invalid number of entries: %zu\n",
-- 
2.13.6


From 0634f5ff5ea307b55a698a2aff97872c072b14b1 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 2 Oct 2018 14:51:05 +0200
Subject: [PATCH 04/21] vfs_fruit: move setting ADEID_FINDERI length to
 ad_convert_xattr()

ad_convert_xattr() does the conversion of the xattr data in the
AppleDouble file, so we should update it's size there and should not
defer it to the caller.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 057e4e4265b..2dec9089006 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1052,6 +1052,7 @@ static bool ad_convert_xattr(struct adouble *ad,
 		fsp = NULL;
 	}
 
+	ad_setentrylen(ad, ADEID_FINDERI, ADEDLEN_FINDERI);
 	return true;
 }
 
@@ -1096,7 +1097,6 @@ static int ad_convert(struct adouble *ad,
 			ad_getentrylen(ad, ADEID_RFORK));
 	}
 
-	ad_setentrylen(ad, ADEID_FINDERI, ADEDLEN_FINDERI);
 	ad_setentryoff(ad, ADEID_RFORK,
 		       ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI);
 
-- 
2.13.6


From 37f6a78860c283369dfc25cdd40ec80b3bf744f7 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 4 Oct 2018 08:23:59 +0200
Subject: [PATCH 05/21] vfs_fruit: do direct return from error checks in
 ad_convert()

Subsequent commits will move the mmap() into the subfunctions. This
change just prepares for that.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 2dec9089006..7d544ea7f94 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1081,8 +1081,7 @@ static int ad_convert(struct adouble *ad,
 	map = mmap(NULL, origlen, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
 	if (map == MAP_FAILED) {
 		DEBUG(2, ("mmap AppleDouble: %s\n", strerror(errno)));
-		rc = -1;
-		goto exit;
+		return -1;
 	}
 
 	ok = ad_convert_xattr(ad, smb_fname, map);
@@ -1106,12 +1105,18 @@ static int ad_convert(struct adouble *ad,
 	 */
 	rc = ftruncate(fd, ad_getentryoff(ad, ADEID_RFORK)
 		       + ad_getentrylen(ad, ADEID_RFORK));
-
-exit:
-	if (map != MAP_FAILED) {
+	if (rc != 0) {
 		munmap(map, origlen);
+		return -1;
 	}
-	return rc;
+
+	rc = munmap(map, origlen);
+	if (rc != 0) {
+		DBG_ERR("munmap failed: %s\n", strerror(errno));
+		return -1;
+	}
+
+	return 0;
 }
 
 /**
-- 
2.13.6


From 691483f38c6a01a35efdab5a7102a732482fd574 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 4 Oct 2018 08:51:28 +0200
Subject: [PATCH 06/21] vfs_fruit: remove unneeded fd argument from
 ad_convert()

Use the struct adouble member ad_fd instead of passing it as an
argument. Who did that in the first place? :)

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 7d544ea7f94..7ed5830223d 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1066,8 +1066,7 @@ static bool ad_convert_xattr(struct adouble *ad,
  * otherwise
  **/
 static int ad_convert(struct adouble *ad,
-		      const struct smb_filename *smb_fname,
-		      int fd)
+		      const struct smb_filename *smb_fname)
 {
 	int rc = 0;
 	char *map = MAP_FAILED;
@@ -1078,7 +1077,8 @@ static int ad_convert(struct adouble *ad,
 		ad_getentrylen(ad, ADEID_RFORK);
 
 	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
-	map = mmap(NULL, origlen, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
+	map = mmap(NULL, origlen, PROT_READ|PROT_WRITE, MAP_SHARED,
+		   ad->ad_fd, 0);
 	if (map == MAP_FAILED) {
 		DEBUG(2, ("mmap AppleDouble: %s\n", strerror(errno)));
 		return -1;
@@ -1103,7 +1103,7 @@ static int ad_convert(struct adouble *ad,
 	 * FIXME: direct ftruncate(), but we don't have a fsp for the
 	 * VFS call
 	 */
-	rc = ftruncate(fd, ad_getentryoff(ad, ADEID_RFORK)
+	rc = ftruncate(ad->ad_fd, ad_getentryoff(ad, ADEID_RFORK)
 		       + ad_getentrylen(ad, ADEID_RFORK));
 	if (rc != 0) {
 		munmap(map, origlen);
@@ -1385,7 +1385,7 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 	 * appended to the ADEID_FINDERI entry.
 	 */
 
-	ret = ad_convert(ad, smb_fname, ad->ad_fd);
+	ret = ad_convert(ad, smb_fname);
 	if (ret != 0) {
 		DBG_WARNING("Failed to convert [%s]\n", smb_fname->base_name);
 		return len;
-- 
2.13.6


From a4918eb42bb3f00a57407a94d55877065769b1ab Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:14:40 +0200
Subject: [PATCH 07/21] vfs_fruit: move storing of modified struct adouble to
 ad_convert()

ad_convert() modified it, so let ad_convert() also save it to disk. No
change in behaviour.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 7ed5830223d..57d3c800260 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1071,6 +1071,7 @@ static int ad_convert(struct adouble *ad,
 	int rc = 0;
 	char *map = MAP_FAILED;
 	size_t origlen;
+	ssize_t len;
 	bool ok;
 
 	origlen = ad_getentryoff(ad, ADEID_RFORK) +
@@ -1116,6 +1117,18 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
+	ok = ad_pack(ad);
+	if (!ok) {
+		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
+		return -1;
+	}
+
+	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
+	if (len != AD_DATASZ_DOT_UND) {
+		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -1391,18 +1404,6 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 		return len;
 	}
 
-	ok = ad_pack(ad);
-	if (!ok) {
-		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
-		return -1;
-	}
-
-	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
-	if (len != AD_DATASZ_DOT_UND) {
-		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
-		return -1;
-	}
-
 	p_ad = ad_get_entry(ad, ADEID_FINDERI);
 	if (p_ad == NULL) {
 		return -1;
-- 
2.13.6


From 511a7e999119c08daf16d00829271421918ce989 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:25:27 +0200
Subject: [PATCH 08/21] vfs_fruit: move FinderInfo conversion to helper
 function and call it from ad_convert()

No change in behaviour.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 192 ++++++++++++++++++++++++--------------------
 1 file changed, 104 insertions(+), 88 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 57d3c800260..5706ed1eae2 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1056,6 +1056,102 @@ static bool ad_convert_xattr(struct adouble *ad,
 	return true;
 }
 
+static bool ad_convert_finderinfo(struct adouble *ad,
+				  const struct smb_filename *smb_fname)
+{
+	char *p_ad = NULL;
+	AfpInfo *ai = NULL;
+	DATA_BLOB aiblob;
+	struct smb_filename *stream_name = NULL;
+	files_struct *fsp = NULL;
+	size_t size;
+	ssize_t nwritten;
+	NTSTATUS status;
+	int saved_errno = 0;
+
+	p_ad = ad_get_entry(ad, ADEID_FINDERI);
+	if (p_ad == NULL) {
+		return false;
+	}
+
+	ai = afpinfo_new(talloc_tos());
+	if (ai == NULL) {
+		return false;
+	}
+
+	memcpy(ai->afpi_FinderInfo, p_ad, ADEDLEN_FINDERI);
+
+	aiblob = data_blob_talloc(talloc_tos(), NULL, AFP_INFO_SIZE);
+	if (aiblob.data == NULL) {
+		TALLOC_FREE(ai);
+		return false;
+	}
+
+	size = afpinfo_pack(ai, (char *)aiblob.data);
+	TALLOC_FREE(ai);
+	if (size != AFP_INFO_SIZE) {
+		return false;
+	}
+
+	stream_name = synthetic_smb_fname(talloc_tos(),
+					  smb_fname->base_name,
+					  AFPINFO_STREAM,
+					  NULL,
+					  smb_fname->flags);
+	if (stream_name == NULL) {
+		data_blob_free(&aiblob);
+		DBG_ERR("synthetic_smb_fname failed\n");
+		return false;
+	}
+
+	DBG_DEBUG("stream_name: %s\n", smb_fname_str_dbg(stream_name));
+
+	status = SMB_VFS_CREATE_FILE(
+		ad->ad_handle->conn,		/* conn */
+		NULL,				/* req */
+		0,				/* root_dir_fid */
+		stream_name,			/* fname */
+		FILE_GENERIC_WRITE,		/* access_mask */
+		FILE_SHARE_READ | FILE_SHARE_WRITE, /* share_access */
+		FILE_OPEN_IF,			/* create_disposition */
+		0,				/* create_options */
+		0,				/* file_attributes */
+		INTERNAL_OPEN_ONLY,		/* oplock_request */
+		NULL,				/* lease */
+		0,				/* allocation_size */
+		0,				/* private_flags */
+		NULL,				/* sd */
+		NULL,				/* ea_list */
+		&fsp,				/* result */
+		NULL,				/* psbuf */
+		NULL, NULL);			/* create context */
+	TALLOC_FREE(stream_name);
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_ERR("SMB_VFS_CREATE_FILE failed\n");
+		return false;
+	}
+
+	nwritten = SMB_VFS_PWRITE(fsp,
+				  aiblob.data,
+				  aiblob.length,
+				  0);
+	if (nwritten == -1) {
+		DBG_ERR("SMB_VFS_PWRITE failed\n");
+		saved_errno = errno;
+		close_file(NULL, fsp, ERROR_CLOSE);
+		errno = saved_errno;
+		return false;
+	}
+
+	status = close_file(NULL, fsp, NORMAL_CLOSE);
+	if (!NT_STATUS_IS_OK(status)) {
+		return false;
+	}
+	fsp = NULL;
+
+	return true;
+}
+
 /**
  * Convert from Apple's ._ file to Netatalk
  *
@@ -1129,6 +1225,13 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
+	ok = ad_convert_finderinfo(ad, smb_fname);
+	if (!ok) {
+		DBG_ERR("Failed to convert [%s]\n",
+			smb_fname_str_dbg(smb_fname));
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -1325,15 +1428,8 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 {
 	SMB_STRUCT_STAT sbuf;
 	char *p_ad = NULL;
-	AfpInfo *ai = NULL;
-	DATA_BLOB aiblob;
-	struct smb_filename *stream_name = NULL;
-	files_struct *fsp = NULL;
-	ssize_t len;
 	size_t size;
-	ssize_t nwritten;
-	NTSTATUS status;
-	int saved_errno = 0;
+	ssize_t len;
 	int ret;
 	bool ok;
 
@@ -1404,86 +1500,6 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 		return len;
 	}
 
-	p_ad = ad_get_entry(ad, ADEID_FINDERI);
-	if (p_ad == NULL) {
-		return -1;
-	}
-
-	ai = afpinfo_new(talloc_tos());
-	if (ai == NULL) {
-		return -1;
-	}
-
-	memcpy(ai->afpi_FinderInfo, p_ad, ADEDLEN_FINDERI);
-
-	aiblob = data_blob_talloc(talloc_tos(), NULL, AFP_INFO_SIZE);
-	if (aiblob.data == NULL) {
-		TALLOC_FREE(ai);
-		return -1;
-	}
-
-	size = afpinfo_pack(ai, (char *)aiblob.data);
-	TALLOC_FREE(ai);
-	if (size != AFP_INFO_SIZE) {
-		return -1;
-	}
-
-	stream_name = synthetic_smb_fname(talloc_tos(),
-					  smb_fname->base_name,
-					  AFPINFO_STREAM,
-					  NULL,
-					  smb_fname->flags);
-	if (stream_name == NULL) {
-		data_blob_free(&aiblob);
-		DBG_ERR("synthetic_smb_fname failed\n");
-		return -1;
-	}
-
-	DBG_DEBUG("stream_name: %s\n", smb_fname_str_dbg(stream_name));
-
-	status = SMB_VFS_CREATE_FILE(
-		ad->ad_handle->conn,		/* conn */
-		NULL,				/* req */
-		0,				/* root_dir_fid */
-		stream_name,			/* fname */
-		FILE_GENERIC_WRITE,		/* access_mask */
-		FILE_SHARE_READ | FILE_SHARE_WRITE, /* share_access */
-		FILE_OPEN_IF,			/* create_disposition */
-		0,				/* create_options */
-		0,				/* file_attributes */
-		INTERNAL_OPEN_ONLY,		/* oplock_request */
-		NULL,				/* lease */
-		0,				/* allocation_size */
-		0,				/* private_flags */
-		NULL,				/* sd */
-		NULL,				/* ea_list */
-		&fsp,				/* result */
-		NULL,				/* psbuf */
-		NULL, NULL);			/* create context */
-	TALLOC_FREE(stream_name);
-	if (!NT_STATUS_IS_OK(status)) {
-		DBG_ERR("SMB_VFS_CREATE_FILE failed\n");
-		return -1;
-	}
-
-	nwritten = SMB_VFS_PWRITE(fsp,
-				  aiblob.data,
-				  aiblob.length,
-				  0);
-	if (nwritten == -1) {
-		DBG_ERR("SMB_VFS_PWRITE failed\n");
-		saved_errno = errno;
-		close_file(NULL, fsp, ERROR_CLOSE);
-		errno = saved_errno;
-		return -1;
-	}
-
-	status = close_file(NULL, fsp, NORMAL_CLOSE);
-	if (!NT_STATUS_IS_OK(status)) {
-		return -1;
-	}
-	fsp = NULL;
-
 	return len;
 }
 
-- 
2.13.6


From 6b2ac54a2c19c56e66d93e0dbe28761c77a59e92 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:26:46 +0200
Subject: [PATCH 09/21] vfs_fruit: move FinderInfo lenght check to ad_convert()

The final step in consolidating all conversion related work in
ad_convert(). No change in behaviour.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 5706ed1eae2..73605c63445 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1170,6 +1170,10 @@ static int ad_convert(struct adouble *ad,
 	ssize_t len;
 	bool ok;
 
+	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
+		return 0;
+	}
+
 	origlen = ad_getentryoff(ad, ADEID_RFORK) +
 		ad_getentrylen(ad, ADEID_RFORK);
 
@@ -1485,10 +1489,6 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 		return -1;
 	}
 
-	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
-		return len;
-	}
-
 	/*
 	 * Try to fixup AppleDouble files created by OS X with xattrs
 	 * appended to the ADEID_FINDERI entry.
-- 
2.13.6


From 2fdc58d5029853844bc451146288883368aaaca4 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 19:13:16 +0200
Subject: [PATCH 10/21] vfs_fruit: split out truncating from ad_convert()

This may look a little ill-advised as this increases line count, but
the goal here is modularizing ad_convert() itself and making it as slick
as possible helps achieving that goal.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 73605c63445..41476625b33 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1152,6 +1152,24 @@ static bool ad_convert_finderinfo(struct adouble *ad,
 	return true;
 }
 
+static bool ad_convert_truncate(struct adouble *ad,
+				const struct smb_filename *smb_fname)
+{
+	int rc;
+
+	/*
+	 * FIXME: direct ftruncate(), but we don't have a fsp for the
+	 * VFS call
+	 */
+	rc = ftruncate(ad->ad_fd, ad_getentryoff(ad, ADEID_RFORK)
+		       + ad_getentrylen(ad, ADEID_RFORK));
+	if (rc != 0) {
+		return false;
+	}
+
+	return true;
+}
+
 /**
  * Convert from Apple's ._ file to Netatalk
  *
@@ -1200,13 +1218,8 @@ static int ad_convert(struct adouble *ad,
 	ad_setentryoff(ad, ADEID_RFORK,
 		       ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI);
 
-	/*
-	 * FIXME: direct ftruncate(), but we don't have a fsp for the
-	 * VFS call
-	 */
-	rc = ftruncate(ad->ad_fd, ad_getentryoff(ad, ADEID_RFORK)
-		       + ad_getentrylen(ad, ADEID_RFORK));
-	if (rc != 0) {
+	ok = ad_convert_truncate(ad, smb_fname);
+	if (!ok) {
 		munmap(map, origlen);
 		return -1;
 	}
-- 
2.13.6


From 4bae23cc5cab811da09123d3d5676604cfdeb13c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 19:15:04 +0200
Subject: [PATCH 11/21] vfs_fruit: use ADEDOFF_RFORK_DOT_UND offset macro in
 ad_convert_truncate()

We really want the fixed size offset here, not a calculated one. Note
that "ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI" is equal to
ADEDOFF_RFORK_DOT_UND.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 41476625b33..02db5923c6a 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1161,8 +1161,8 @@ static bool ad_convert_truncate(struct adouble *ad,
 	 * FIXME: direct ftruncate(), but we don't have a fsp for the
 	 * VFS call
 	 */
-	rc = ftruncate(ad->ad_fd, ad_getentryoff(ad, ADEID_RFORK)
-		       + ad_getentrylen(ad, ADEID_RFORK));
+	rc = ftruncate(ad->ad_fd, ADEDOFF_RFORK_DOT_UND +
+		       ad_getentrylen(ad, ADEID_RFORK));
 	if (rc != 0) {
 		return false;
 	}
-- 
2.13.6


From 91514c7d26835ec39800b7f11170dbab1f827241 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:44:53 +0200
Subject: [PATCH 12/21] vfs_fruit: split out moving of the resource fork

No change in behaviour.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 02db5923c6a..0aeed3a66bb 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1170,6 +1170,24 @@ static bool ad_convert_truncate(struct adouble *ad,
 	return true;
 }
 
+static bool ad_convert_move_reso(struct adouble *ad,
+				 const struct smb_filename *smb_fname,
+				 char *map)
+{
+	if (ad_getentrylen(ad, ADEID_RFORK) == 0) {
+		return true;
+	}
+
+	memmove(map + ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI,
+		map + ad_getentryoff(ad, ADEID_RFORK),
+		ad_getentrylen(ad, ADEID_RFORK));
+
+	ad_setentryoff(ad, ADEID_RFORK,
+		       ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI);
+
+	return true;
+}
+
 /**
  * Convert from Apple's ._ file to Netatalk
  *
@@ -1209,15 +1227,12 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
-	if (ad_getentrylen(ad, ADEID_RFORK) > 0) {
-		memmove(map + ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI,
-			map + ad_getentryoff(ad, ADEID_RFORK),
-			ad_getentrylen(ad, ADEID_RFORK));
+	ok = ad_convert_move_reso(ad, smb_fname, map);
+	if (!ok) {
+		munmap(map, origlen);
+		return -1;
 	}
 
-	ad_setentryoff(ad, ADEID_RFORK,
-		       ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI);
-
 	ok = ad_convert_truncate(ad, smb_fname);
 	if (!ok) {
 		munmap(map, origlen);
-- 
2.13.6


From dc297d597dedbaea565d30e178046bbd212ce3e9 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 19:15:04 +0200
Subject: [PATCH 13/21] vfs_fruit: use ADEDOFF_RFORK_DOT_UND offset macro in
 ad_convert_move_reso()

We really want the fixed size offset here, not a calculated one. Note
that "ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI" is equal to
ADEDOFF_RFORK_DOT_UND.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 0aeed3a66bb..4c710bd2f99 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1178,12 +1178,11 @@ static bool ad_convert_move_reso(struct adouble *ad,
 		return true;
 	}
 
-	memmove(map + ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI,
+	memmove(map + ADEDOFF_RFORK_DOT_UND,
 		map + ad_getentryoff(ad, ADEID_RFORK),
 		ad_getentrylen(ad, ADEID_RFORK));
 
-	ad_setentryoff(ad, ADEID_RFORK,
-		       ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI);
+	ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_DOT_UND);
 
 	return true;
 }
-- 
2.13.6


From 62b7c996fd11f18867a543b28a33b0a70a1abca9 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:52:32 +0200
Subject: [PATCH 14/21] vfs_fruit: fix error returns in ad_convert_xattr()

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 4c710bd2f99..aaaf5fde97e 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -985,14 +985,14 @@ static bool ad_convert_xattr(struct adouble *ad,
 		    !NT_STATUS_EQUAL(status, NT_STATUS_NONE_MAPPED))
 		{
 			DBG_ERR("string_replace_allocate failed\n");
-			return -1;
+			return false;
 		}
 
 		tmp = mapped_name;
 		mapped_name = talloc_asprintf(talloc_tos(), ":%s", tmp);
 		TALLOC_FREE(tmp);
 		if (mapped_name == NULL) {
-			return -1;
+			return false;
 		}
 
 		stream_name = synthetic_smb_fname(talloc_tos(),
@@ -1003,7 +1003,7 @@ static bool ad_convert_xattr(struct adouble *ad,
 		TALLOC_FREE(mapped_name);
 		if (stream_name == NULL) {
 			DBG_ERR("synthetic_smb_fname failed\n");
-			return -1;
+			return false;
 		}
 
 		DBG_DEBUG("stream_name: %s\n", smb_fname_str_dbg(stream_name));
@@ -1030,7 +1030,7 @@ static bool ad_convert_xattr(struct adouble *ad,
 		TALLOC_FREE(stream_name);
 		if (!NT_STATUS_IS_OK(status)) {
 			DBG_ERR("SMB_VFS_CREATE_FILE failed\n");
-			return -1;
+			return false;
 		}
 
 		nwritten = SMB_VFS_PWRITE(fsp,
@@ -1042,12 +1042,12 @@ static bool ad_convert_xattr(struct adouble *ad,
 			saved_errno = errno;
 			close_file(NULL, fsp, ERROR_CLOSE);
 			errno = saved_errno;
-			return -1;
+			return false;
 		}
 
 		status = close_file(NULL, fsp, NORMAL_CLOSE);
 		if (!NT_STATUS_IS_OK(status)) {
-			return -1;
+			return false;
 		}
 		fsp = NULL;
 	}
-- 
2.13.6


From bcf58f49944b8a384b4ba0bd185305c0daa6a759 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:59:18 +0200
Subject: [PATCH 15/21] vfs_fruit: let the ad_convert_*() subfunctions mmap as
 needed

This may mean that we mmap twice when we convert an AppleDouble file,
but this is the only sane way to cleanly modularize ad_convert().

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 98 ++++++++++++++++++++++++++++-----------------
 1 file changed, 62 insertions(+), 36 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index aaaf5fde97e..f72bec153b2 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -943,13 +943,16 @@ static bool ad_unpack(struct adouble *ad, const size_t nentries,
 }
 
 static bool ad_convert_xattr(struct adouble *ad,
-			     const struct smb_filename *smb_fname,
-			     char *map)
+			     const struct smb_filename *smb_fname)
 {
 	static struct char_mappings **string_replace_cmaps = NULL;
+	char *map = MAP_FAILED;
+	size_t maplen;
 	uint16_t i;
 	int saved_errno = 0;
 	NTSTATUS status;
+	int rc;
+	bool ok;
 
 	if (ad->adx_header.adx_num_attrs == 0) {
 		return true;
@@ -967,6 +970,17 @@ static bool ad_convert_xattr(struct adouble *ad,
 		TALLOC_FREE(mappings);
 	}
 
+	maplen = ad_getentryoff(ad, ADEID_RFORK) +
+		ad_getentrylen(ad, ADEID_RFORK);
+
+	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
+	map = mmap(NULL, maplen, PROT_READ|PROT_WRITE, MAP_SHARED,
+		   ad->ad_fd, 0);
+	if (map == MAP_FAILED) {
+		DBG_ERR("mmap AppleDouble: %s\n", strerror(errno));
+		return false;
+	}
+
 	for (i = 0; i < ad->adx_header.adx_num_attrs; i++) {
 		struct ad_xattr_entry *e = &ad->adx_entries[i];
 		char *mapped_name = NULL;
@@ -985,14 +999,16 @@ static bool ad_convert_xattr(struct adouble *ad,
 		    !NT_STATUS_EQUAL(status, NT_STATUS_NONE_MAPPED))
 		{
 			DBG_ERR("string_replace_allocate failed\n");
-			return false;
+			ok = false;
+			goto fail;
 		}
 
 		tmp = mapped_name;
 		mapped_name = talloc_asprintf(talloc_tos(), ":%s", tmp);
 		TALLOC_FREE(tmp);
 		if (mapped_name == NULL) {
-			return false;
+			ok = false;
+			goto fail;
 		}
 
 		stream_name = synthetic_smb_fname(talloc_tos(),
@@ -1003,7 +1019,8 @@ static bool ad_convert_xattr(struct adouble *ad,
 		TALLOC_FREE(mapped_name);
 		if (stream_name == NULL) {
 			DBG_ERR("synthetic_smb_fname failed\n");
-			return false;
+			ok = false;
+			goto fail;
 		}
 
 		DBG_DEBUG("stream_name: %s\n", smb_fname_str_dbg(stream_name));
@@ -1030,7 +1047,8 @@ static bool ad_convert_xattr(struct adouble *ad,
 		TALLOC_FREE(stream_name);
 		if (!NT_STATUS_IS_OK(status)) {
 			DBG_ERR("SMB_VFS_CREATE_FILE failed\n");
-			return false;
+			ok = false;
+			goto fail;
 		}
 
 		nwritten = SMB_VFS_PWRITE(fsp,
@@ -1042,18 +1060,29 @@ static bool ad_convert_xattr(struct adouble *ad,
 			saved_errno = errno;
 			close_file(NULL, fsp, ERROR_CLOSE);
 			errno = saved_errno;
-			return false;
+			ok = false;
+			goto fail;
 		}
 
 		status = close_file(NULL, fsp, NORMAL_CLOSE);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			ok = false;
+			goto fail;
 		}
 		fsp = NULL;
 	}
 
 	ad_setentrylen(ad, ADEID_FINDERI, ADEDLEN_FINDERI);
-	return true;
+	ok = true;
+
+fail:
+	rc = munmap(map, maplen);
+	if (rc != 0) {
+		DBG_ERR("munmap failed: %s\n", strerror(errno));
+		return false;
+	}
+
+	return ok;
 }
 
 static bool ad_convert_finderinfo(struct adouble *ad,
@@ -1171,17 +1200,37 @@ static bool ad_convert_truncate(struct adouble *ad,
 }
 
 static bool ad_convert_move_reso(struct adouble *ad,
-				 const struct smb_filename *smb_fname,
-				 char *map)
+				 const struct smb_filename *smb_fname)
 {
+	char *map = MAP_FAILED;
+	size_t maplen;
+	int rc;
+
 	if (ad_getentrylen(ad, ADEID_RFORK) == 0) {
 		return true;
 	}
 
+	maplen = ad_getentryoff(ad, ADEID_RFORK) +
+		ad_getentrylen(ad, ADEID_RFORK);
+
+	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
+	map = mmap(NULL, maplen, PROT_READ|PROT_WRITE, MAP_SHARED,
+		   ad->ad_fd, 0);
+	if (map == MAP_FAILED) {
+		DBG_ERR("mmap AppleDouble: %s\n", strerror(errno));
+		return false;
+	}
+
 	memmove(map + ADEDOFF_RFORK_DOT_UND,
 		map + ad_getentryoff(ad, ADEID_RFORK),
 		ad_getentrylen(ad, ADEID_RFORK));
 
+	rc = munmap(map, maplen);
+	if (rc != 0) {
+		DBG_ERR("munmap failed: %s\n", strerror(errno));
+		return false;
+	}
+
 	ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_DOT_UND);
 
 	return true;
@@ -1199,9 +1248,6 @@ static bool ad_convert_move_reso(struct adouble *ad,
 static int ad_convert(struct adouble *ad,
 		      const struct smb_filename *smb_fname)
 {
-	int rc = 0;
-	char *map = MAP_FAILED;
-	size_t origlen;
 	ssize_t len;
 	bool ok;
 
@@ -1209,38 +1255,18 @@ static int ad_convert(struct adouble *ad,
 		return 0;
 	}
 
-	origlen = ad_getentryoff(ad, ADEID_RFORK) +
-		ad_getentrylen(ad, ADEID_RFORK);
-
-	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
-	map = mmap(NULL, origlen, PROT_READ|PROT_WRITE, MAP_SHARED,
-		   ad->ad_fd, 0);
-	if (map == MAP_FAILED) {
-		DEBUG(2, ("mmap AppleDouble: %s\n", strerror(errno)));
-		return -1;
-	}
-
-	ok = ad_convert_xattr(ad, smb_fname, map);
+	ok = ad_convert_xattr(ad, smb_fname);
 	if (!ok) {
-		munmap(map, origlen);
 		return -1;
 	}
 
-	ok = ad_convert_move_reso(ad, smb_fname, map);
+	ok = ad_convert_move_reso(ad, smb_fname);
 	if (!ok) {
-		munmap(map, origlen);
 		return -1;
 	}
 
 	ok = ad_convert_truncate(ad, smb_fname);
 	if (!ok) {
-		munmap(map, origlen);
-		return -1;
-	}
-
-	rc = munmap(map, origlen);
-	if (rc != 0) {
-		DBG_ERR("munmap failed: %s\n", strerror(errno));
 		return -1;
 	}
 
-- 
2.13.6


From 5bf303b04551ca5e06ef92fe17f387a58c3b28c0 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 17:07:45 +0200
Subject: [PATCH 16/21] vfs_fruit: let the ad_convert_*() subfunction update
 the on-disk AppleDoube header as needed

Another step in simplifying ad_convert() itself. It means that we may
write to disk twice, but is only ever done once per AppleDouble file.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 42 +++++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index f72bec153b2..c76dcb38671 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -949,6 +949,7 @@ static bool ad_convert_xattr(struct adouble *ad,
 	char *map = MAP_FAILED;
 	size_t maplen;
 	uint16_t i;
+	ssize_t len;
 	int saved_errno = 0;
 	NTSTATUS status;
 	int rc;
@@ -1073,6 +1074,20 @@ static bool ad_convert_xattr(struct adouble *ad,
 	}
 
 	ad_setentrylen(ad, ADEID_FINDERI, ADEDLEN_FINDERI);
+
+	ok = ad_pack(ad);
+	if (!ok) {
+		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
+		goto fail;
+	}
+
+	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
+	if (len != AD_DATASZ_DOT_UND) {
+		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
+		ok = false;
+		goto fail;
+	}
+
 	ok = true;
 
 fail:
@@ -1204,7 +1219,9 @@ static bool ad_convert_move_reso(struct adouble *ad,
 {
 	char *map = MAP_FAILED;
 	size_t maplen;
+	ssize_t len;
 	int rc;
+	bool ok;
 
 	if (ad_getentrylen(ad, ADEID_RFORK) == 0) {
 		return true;
@@ -1233,6 +1250,18 @@ static bool ad_convert_move_reso(struct adouble *ad,
 
 	ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_DOT_UND);
 
+	ok = ad_pack(ad);
+	if (!ok) {
+		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
+		return false;
+	}
+
+	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
+	if (len != AD_DATASZ_DOT_UND) {
+		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
+		return false;
+	}
+
 	return true;
 }
 
@@ -1248,7 +1277,6 @@ static bool ad_convert_move_reso(struct adouble *ad,
 static int ad_convert(struct adouble *ad,
 		      const struct smb_filename *smb_fname)
 {
-	ssize_t len;
 	bool ok;
 
 	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
@@ -1270,18 +1298,6 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
-	ok = ad_pack(ad);
-	if (!ok) {
-		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
-		return -1;
-	}
-
-	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
-	if (len != AD_DATASZ_DOT_UND) {
-		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
-		return -1;
-	}
-
 	ok = ad_convert_finderinfo(ad, smb_fname);
 	if (!ok) {
 		DBG_ERR("Failed to convert [%s]\n",
-- 
2.13.6


From 01ae5da66cb4d2efb16cf1544c22c51749a48ead Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 22:05:43 +0200
Subject: [PATCH 17/21] vfs_fruit: call ad_convert_move_reso() from
 ad_convert_xattr()

ad_convert_xattr() is the place that triggers the need to move the
resource fork, so it should also call ad_convert_move_reso().

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 113 ++++++++++++++++++++++----------------------
 1 file changed, 57 insertions(+), 56 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index c76dcb38671..f7cba8a0728 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -942,6 +942,58 @@ static bool ad_unpack(struct adouble *ad, const size_t nentries,
 	return true;
 }
 
+static bool ad_convert_move_reso(struct adouble *ad,
+				 const struct smb_filename *smb_fname)
+{
+	char *map = MAP_FAILED;
+	size_t maplen;
+	ssize_t len;
+	int rc;
+	bool ok;
+
+	if (ad_getentrylen(ad, ADEID_RFORK) == 0) {
+		return true;
+	}
+
+	maplen = ad_getentryoff(ad, ADEID_RFORK) +
+		ad_getentrylen(ad, ADEID_RFORK);
+
+	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
+	map = mmap(NULL, maplen, PROT_READ|PROT_WRITE, MAP_SHARED,
+		   ad->ad_fd, 0);
+	if (map == MAP_FAILED) {
+		DBG_ERR("mmap AppleDouble: %s\n", strerror(errno));
+		return false;
+	}
+
+
+	memmove(map + ADEDOFF_RFORK_DOT_UND,
+		map + ad_getentryoff(ad, ADEID_RFORK),
+		ad_getentrylen(ad, ADEID_RFORK));
+
+	rc = munmap(map, maplen);
+	if (rc != 0) {
+		DBG_ERR("munmap failed: %s\n", strerror(errno));
+		return false;
+	}
+
+	ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_DOT_UND);
+
+	ok = ad_pack(ad);
+	if (!ok) {
+		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
+		return false;
+	}
+
+	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
+	if (len != AD_DATASZ_DOT_UND) {
+		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
+		return false;
+	}
+
+	return true;
+}
+
 static bool ad_convert_xattr(struct adouble *ad,
 			     const struct smb_filename *smb_fname)
 {
@@ -1088,6 +1140,11 @@ static bool ad_convert_xattr(struct adouble *ad,
 		goto fail;
 	}
 
+	ok = ad_convert_move_reso(ad, smb_fname);
+	if (!ok) {
+		goto fail;
+	}
+
 	ok = true;
 
 fail:
@@ -1214,57 +1271,6 @@ static bool ad_convert_truncate(struct adouble *ad,
 	return true;
 }
 
-static bool ad_convert_move_reso(struct adouble *ad,
-				 const struct smb_filename *smb_fname)
-{
-	char *map = MAP_FAILED;
-	size_t maplen;
-	ssize_t len;
-	int rc;
-	bool ok;
-
-	if (ad_getentrylen(ad, ADEID_RFORK) == 0) {
-		return true;
-	}
-
-	maplen = ad_getentryoff(ad, ADEID_RFORK) +
-		ad_getentrylen(ad, ADEID_RFORK);
-
-	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
-	map = mmap(NULL, maplen, PROT_READ|PROT_WRITE, MAP_SHARED,
-		   ad->ad_fd, 0);
-	if (map == MAP_FAILED) {
-		DBG_ERR("mmap AppleDouble: %s\n", strerror(errno));
-		return false;
-	}
-
-	memmove(map + ADEDOFF_RFORK_DOT_UND,
-		map + ad_getentryoff(ad, ADEID_RFORK),
-		ad_getentrylen(ad, ADEID_RFORK));
-
-	rc = munmap(map, maplen);
-	if (rc != 0) {
-		DBG_ERR("munmap failed: %s\n", strerror(errno));
-		return false;
-	}
-
-	ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_DOT_UND);
-
-	ok = ad_pack(ad);
-	if (!ok) {
-		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
-		return false;
-	}
-
-	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
-	if (len != AD_DATASZ_DOT_UND) {
-		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
-		return false;
-	}
-
-	return true;
-}
-
 /**
  * Convert from Apple's ._ file to Netatalk
  *
@@ -1288,11 +1294,6 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
-	ok = ad_convert_move_reso(ad, smb_fname);
-	if (!ok) {
-		return -1;
-	}
-
 	ok = ad_convert_truncate(ad, smb_fname);
 	if (!ok) {
 		return -1;
-- 
2.13.6


From 37a2bea7b1d4362f369854ea43d4e55d6d05a3c8 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 8 Oct 2018 12:51:37 +0200
Subject: [PATCH 18/21] vfs_fruit: add check for OS X filler in FinderInfo
 conversion

This ensures that the function only acts on AppleDouble files created by
macOS and not AppleDouble files created by us that are already in the
correct format (only using the Resource Fork).

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index f7cba8a0728..9987f354d2b 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1169,6 +1169,12 @@ static bool ad_convert_finderinfo(struct adouble *ad,
 	ssize_t nwritten;
 	NTSTATUS status;
 	int saved_errno = 0;
+	int cmp;
+
+	cmp = memcmp(ad->ad_filler, AD_FILLER_TAG_OSX, ADEDLEN_FILLER);
+	if (cmp != 0) {
+		return true;
+	}
 
 	p_ad = ad_get_entry(ad, ADEID_FINDERI);
 	if (p_ad == NULL) {
-- 
2.13.6


From 0863911f9d119e8b3a34b52d683b9467135de477 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 8 Oct 2018 18:43:51 +0200
Subject: [PATCH 19/21] vfs_fruit: add out arg "converted_xattr" to
 ad_convert_xattr

Used to let the caller know if a conversion has been done. Currently not
used in the caller, that comes next.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 9987f354d2b..3b491aff301 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -995,7 +995,8 @@ static bool ad_convert_move_reso(struct adouble *ad,
 }
 
 static bool ad_convert_xattr(struct adouble *ad,
-			     const struct smb_filename *smb_fname)
+			     const struct smb_filename *smb_fname,
+			     bool *converted_xattr)
 {
 	static struct char_mappings **string_replace_cmaps = NULL;
 	char *map = MAP_FAILED;
@@ -1007,6 +1008,8 @@ static bool ad_convert_xattr(struct adouble *ad,
 	int rc;
 	bool ok;
 
+	*converted_xattr = false;
+
 	if (ad->adx_header.adx_num_attrs == 0) {
 		return true;
 	}
@@ -1145,6 +1148,7 @@ static bool ad_convert_xattr(struct adouble *ad,
 		goto fail;
 	}
 
+	*converted_xattr = true;
 	ok = true;
 
 fail:
@@ -1290,12 +1294,13 @@ static int ad_convert(struct adouble *ad,
 		      const struct smb_filename *smb_fname)
 {
 	bool ok;
+	bool converted_xattr;
 
 	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
 		return 0;
 	}
 
-	ok = ad_convert_xattr(ad, smb_fname);
+	ok = ad_convert_xattr(ad, smb_fname, &converted_xattr);
 	if (!ok) {
 		return -1;
 	}
-- 
2.13.6


From a696c3d6e4c938d589e867f94c51ce07aacdafce Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 8 Oct 2018 18:47:32 +0200
Subject: [PATCH 20/21] vfs_fruit: make call to ad_convert_truncate() optional

Call ad_convert_truncate() based on whether the previous call
ad_convert_xattr() returned converted_xattr=true.

Upcoming fixes for a different Samba bug (#13642) will hook into calling
ad_convert_truncate() in other cases, this also prepares for that.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 3b491aff301..c16cad0f1c3 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1305,9 +1305,11 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
-	ok = ad_convert_truncate(ad, smb_fname);
-	if (!ok) {
-		return -1;
+	if (converted_xattr) {
+		ok = ad_convert_truncate(ad, smb_fname);
+		if (!ok) {
+			return -1;
+		}
 	}
 
 	ok = ad_convert_finderinfo(ad, smb_fname);
-- 
2.13.6


From 5a49934cfb20e860ad93215135f1ce2808944c9a Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 9 Oct 2018 10:15:37 +0200
Subject: [PATCH 21/21] vfs_fruit: move check in ad_convert() to ad_convert_*()
 subfunctions

Currently the whole conversion is skipped if the FinderInfo entry in the
AppleDouble file is of the default size (ie not containing xattrs).

That also means we never converted FinderInfo from the AppleDouble file
to stream format. This change finally fixes this.

Note that this keeps failing with streams_depot, much like the existing
known-fail of "samba3.vfs.fruit streams_depot.OS X AppleDouble file
conversion". Fixing the conversion to work with vfs_streams_depot is a
task for another day.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 selftest/knownfail.d/samba3.vfs.fruit | 2 --
 source3/modules/vfs_fruit.c           | 8 ++++----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit
index bc46c2f4922..6307e2b3404 100644
--- a/selftest/knownfail.d/samba3.vfs.fruit
+++ b/selftest/knownfail.d/samba3.vfs.fruit
@@ -1,4 +1,2 @@
 ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
-^samba3.vfs.fruit metadata_stream.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
 ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index c16cad0f1c3..4420cb6021e 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1010,6 +1010,10 @@ static bool ad_convert_xattr(struct adouble *ad,
 
 	*converted_xattr = false;
 
+	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
+		return true;
+	}
+
 	if (ad->adx_header.adx_num_attrs == 0) {
 		return true;
 	}
@@ -1296,10 +1300,6 @@ static int ad_convert(struct adouble *ad,
 	bool ok;
 	bool converted_xattr;
 
-	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
-		return 0;
-	}
-
 	ok = ad_convert_xattr(ad, smb_fname, &converted_xattr);
 	if (!ok) {
 		return -1;
-- 
2.13.6



More information about the samba-technical mailing list