From f0d9f7939dc868f2b125b91eba40f81c2ba978c9 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 8b9728480f6ab22da0831400796f3c39ec543df8) --- 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 a9ae891a23f..58a94dd143c 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 df6da22d2775fbc4003e39a8a40171c30a5e46bb Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 7e010abbaad79643f31361de47340c218fa39505) --- 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 1102059bbee..32c0c138f6b 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 ae6aef3ca64735dc6b223c51df1aeea072d389a2 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 8ee7e6135e39520f486e8f8f4ba36009c9113229) --- 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 32c0c138f6b..3f7c4b73e81 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 2b4918a985dc6a97051bd066b40ae69c1e05f5ba Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit d161e047710322491802d75f47598f96727cd004) --- 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 3f7c4b73e81..be1b495231c 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 a87e044c93d5f2654883d7309b3bdd00cc33c43b Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 8bc36d723ff41afe768f42b833aa951e1ee8fb38) --- 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 be1b495231c..7b6c4c5826a 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 ed5a5b5706f88d6015491c58f259310219b30aab Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 99cc9ef82b50b57149f71a40d4b22a3fc32a5a97) --- 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 7b6c4c5826a..4b1d8946e26 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 a637a13f89e4094d2ae2b5d73ae646abfa3caf45 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit b355a09576563d0f681d0bf830d0e2c769533399) --- 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 4b1d8946e26..55b367711e3 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 09f3d7effd9c4268c1aea8700f725867a0d1233f Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit d27d0326c3c8cb6d217e6c8056fae2c98ef82803) --- 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 55b367711e3..b4dcd8809db 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 4f7d12e48e503133eeb03d02d8dd16af29d07160 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 4f1174b6eb2257d789a1eb9c925ccc561bab2f16) --- 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 b4dcd8809db..b69f872169b 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 a2116d5e68aca6a884f596e5e56826110b8dccf9 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 98bd7c0a46f72b097011a5c59e899ac4862ff651) --- 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 b69f872169b..5a3c1b895ff 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 942390e8fbdd3c3599f9f732a4af5840eb4aaad2 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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_RFORK)" is equal to ADEDOFF_RFORK_DOT_UND in this case. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit b948681b2bbaba202843858857fb9edbb543bdf2) --- 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 5a3c1b895ff..9a089e8737e 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 5f0c05d6b3d73318ec160574d0f7a48a4b59489d Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 4c7e1de46f4287818ef525c8939029ccb09adf65) --- 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 9a089e8737e..c2d2d35f083 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 fccf3020ae2fe55588a7caed1430136591664c74 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 93b7e0562159eae40e196f6be8d82283f0be2888) --- 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 c2d2d35f083..ce813ab94c0 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 8bcd49ac4d8e1d5130e1ac10227e607db15a6697 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit f91e0c857a5a44a5eab7696fafda758044739978) --- 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 ce813ab94c0..89f66a70229 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 746fea8e70565537907c6a9329763628c87d8771 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 918c6c59901d0bf939dcc178b661b6ae8201e903) --- 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 89f66a70229..1c94195cf40 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 090cff2abab23bdbe5a21b2b1b371142eefabaa3 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 1692ca5fd8ae2560dae6828f3c5c05a65c530726) --- 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 1c94195cf40..319e1915038 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 a5d11a6cb59d011d3ca9807752e021812bdbc618 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 70d3ae5a89fc62db192c44b92a5b7fb67a93d88e) --- 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 319e1915038..562ea01a174 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 7d2c08b092c638fff823e73c4a18cce8b53212da Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 5598e6bc3583a88f474afa2996d1f9362d1bd9fb) --- 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 562ea01a174..5441a3552e0 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 1dbc2fcf34f12d74b89a03c516eb3f4a67ccc453 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit acb72c1ea7fecc9a7e8eb0219096b1bbdfd8850e) --- 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 5441a3552e0..6a87e235987 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 = false; 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 c9ac135e54efdc4518728b65718e1604288c74c7 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 9cf087a474bb2d7d29ca0daeaef412f6b545d0e0) --- 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 6a87e235987..0a34b1b2647 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 f19f7fc14107908f0df2696587ecaf7c4fc47aea Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 9 Oct 2018 10:15:37 +0200 Subject: [PATCH 21/21] vfs_fruit: move check in ad_convert() to ad_convert_*() subfunctions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Jeremy Allison Autobuild-User(master): Ralph Böhme Autobuild-Date(master): Thu Oct 11 01:30:13 CEST 2018 on sn-devel-144 (cherry picked from commit 31daab88e6a415e72ead69844e3eccf5dc02e53c) --- 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 0a34b1b2647..58ef3cc68d3 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 = false; - 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