From a03919b31ca78246c7404e51f48ca0fb4a8e10f7 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 25 Oct 2019 15:41:40 +0200 Subject: [PATCH 1/5] torture: expand test "vfs.fruit.resource fork IO" to check size Reveals a bug where the resource fork size is capped at 65454 bytes. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14171 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher (cherry picked from commit b63069db9fb6efb33b7b917cd5b0ee06b0da9cdc) --- selftest/knownfail.d/samba3.vfs.fruit | 3 +++ source4/torture/vfs/fruit.c | 29 +++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit index 6307e2b3404..6982e100861 100644 --- a/selftest/knownfail.d/samba3.vfs.fruit +++ b/selftest/knownfail.d/samba3.vfs.fruit @@ -1,2 +1,5 @@ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion\(nt4_dc\) ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\) +^samba3.vfs.fruit fruit_delete_empty_adfiles.resource fork IO\(nt4_dc\) +^samba3.vfs.fruit metadata_stream.resource fork IO\(nt4_dc\) +^samba3.vfs.fruit metadata_netatalk.resource fork IO\(nt4_dc\) diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index e3d8539595d..0dedb92579f 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -2397,6 +2397,35 @@ static bool test_write_atalk_rfork_io(struct torture_context *tctx, fname, AFPRESOURCE_STREAM_NAME, (off_t)64*1024*1024, 10, rfork_content); + /* Check size after write */ + + ZERO_STRUCT(io); + io.smb2.in.create_disposition = NTCREATEX_DISP_OPEN; + io.smb2.in.desired_access = SEC_FILE_READ_ATTRIBUTE | + SEC_FILE_WRITE_ATTRIBUTE; + io.smb2.in.fname = rfork; + status = smb2_create(tree, mem_ctx, &(io.smb2)); + CHECK_STATUS(status, NT_STATUS_OK); + filehandle = io.smb2.out.file.handle; + + torture_comment(tctx, "(%s) check resource fork size after write\n", + __location__); + + ZERO_STRUCT(finfo); + finfo.generic.level = RAW_FILEINFO_ALL_INFORMATION; + finfo.generic.in.file.handle = filehandle; + status = smb2_getinfo_file(tree, mem_ctx, &finfo); + CHECK_STATUS(status, NT_STATUS_OK); + if (finfo.all_info.out.size != 64*1024*1024 + 10) { + torture_result(tctx, TORTURE_FAIL, + "(%s) Incorrect resource fork size\n", + __location__); + ret = false; + smb2_util_close(tree, filehandle); + goto done; + } + smb2_util_close(tree, filehandle); + ret &= check_stream(tree, __location__, tctx, mem_ctx, fname, AFPRESOURCE_STREAM_NAME, (off_t)64*1024*1024, 10, 0, 10, rfork_content); -- 2.21.0 From e0e9ecaf3966b1f4586913dfedac86cdac686bb8 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 24 Oct 2019 17:17:28 +0200 Subject: [PATCH 2/5] vfs_fruit: fix a long line BUG: https://bugzilla.samba.org/show_bug.cgi?id=14171 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher (backported from commit f0c8ac47a4608eabeae334d39885aab98198b753) --- source3/modules/vfs_fruit.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 85c7af21d58..049a6d72668 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -1697,7 +1697,11 @@ static ssize_t ad_read_rsrc_adouble(vfs_handle_struct *handle, ad->ad_data = p_ad; } - len = SMB_VFS_NEXT_PREAD(handle, ad->ad_fsp, ad->ad_data, talloc_array_length(ad->ad_data), 0); + len = SMB_VFS_NEXT_PREAD(handle, + ad->ad_fsp, + ad->ad_data, + talloc_array_length(ad->ad_data), + 0); if (len != talloc_array_length(ad->ad_data)) { DBG_NOTICE("%s %s: bad size: %zd\n", smb_fname->base_name, strerror(errno), len); -- 2.21.0 From ea74f6004ffed5bb7a1b434ef1ebecb1c8c711f3 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 24 Oct 2019 17:26:08 +0200 Subject: [PATCH 3/5] vfs_fruit: README.Coding fix: multi-line if expression Also remove a TAB. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14171 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher (backported from commit baaaf59e948df625b01fa8b6317ab5c3babb4e8f) --- source3/modules/vfs_fruit.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 049a6d72668..4e5d61d9baa 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -1719,7 +1719,8 @@ static ssize_t ad_read_rsrc_adouble(vfs_handle_struct *handle, if ((ad_getentryoff(ad, ADEID_FINDERI) != ADEDOFF_FINDERI_DOT_UND) || (ad_getentrylen(ad, ADEID_FINDERI) < ADEDLEN_FINDERI) - || (ad_getentryoff(ad, ADEID_RFORK) < ADEDOFF_RFORK_DOT_UND)) { + || (ad_getentryoff(ad, ADEID_RFORK) < ADEDOFF_RFORK_DOT_UND)) + { DBG_ERR("invalid AppleDouble resource %s\n", smb_fname->base_name); errno = EINVAL; -- 2.21.0 From ed69a7de6a1a7af4eaeb60e4e120c20b1f4ca291 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 25 Oct 2019 15:21:32 +0200 Subject: [PATCH 4/5] lib/adouble: drop ad_data reallocate logic Simply set the buffer size to AD_XATTR_MAX_HDR_SIZE. When reading the AppleDouble file, read up to AD_XATTR_MAX_HDR_SIZE from the file. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14171 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher (backported from commit 9a3da6bebcdb924ca2027337544d79ac2088677e) --- source3/modules/vfs_fruit.c | 46 ++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 4e5d61d9baa..131513423bd 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -1664,8 +1664,7 @@ static ssize_t ad_read_rsrc_adouble(vfs_handle_struct *handle, struct adouble *ad, const struct smb_filename *smb_fname) { - char *p_ad = NULL; - size_t size; + size_t to_read; ssize_t len; int ret; bool ok; @@ -1677,32 +1676,17 @@ static ssize_t ad_read_rsrc_adouble(vfs_handle_struct *handle, return -1; } - /* - * AppleDouble file header content and size, two cases: - * - * - without xattrs it is exactly AD_DATASZ_DOT_UND (82) bytes large - * - with embedded xattrs it can be larger, up to AD_XATTR_MAX_HDR_SIZE - * - * Read as much as we can up to AD_XATTR_MAX_HDR_SIZE. - */ - size = ad->ad_fsp->fsp_name->st.st_ex_size; - if (size > talloc_array_length(ad->ad_data)) { - if (size > AD_XATTR_MAX_HDR_SIZE) { - size = AD_XATTR_MAX_HDR_SIZE; - } - p_ad = talloc_realloc(ad, ad->ad_data, char, size); - if (p_ad == NULL) { - return -1; - } - ad->ad_data = p_ad; + to_read = ad->ad_fsp->fsp_name->st.st_ex_size; + if (to_read > AD_XATTR_MAX_HDR_SIZE) { + to_read = AD_XATTR_MAX_HDR_SIZE; } len = SMB_VFS_NEXT_PREAD(handle, ad->ad_fsp, ad->ad_data, - talloc_array_length(ad->ad_data), + to_read, 0); - if (len != talloc_array_length(ad->ad_data)) { + if (len != to_read) { DBG_NOTICE("%s %s: bad size: %zd\n", smb_fname->base_name, strerror(errno), len); return -1; @@ -1800,7 +1784,23 @@ static struct adouble *ad_alloc(TALLOC_CTX *ctx, adsize = AD_DATASZ_XATTR; break; case ADOUBLE_RSRC: - adsize = AD_DATASZ_DOT_UND; + /* + * AppleDouble ._ file case, optimize for fewer (but larger) + * IOs. Two cases: + * + * - without xattrs size of the header is exactly + * AD_DATASZ_DOT_UND (82) bytes + * + * - with embedded xattrs it can be larger, up to + * AD_XATTR_MAX_HDR_SIZE + * + * Larger headers are not supported, but this is a reasonable + * limit that is also employed by the macOS client. + * + * We used the largest possible size to be able to read the full + * header with one IO. + */ + adsize = AD_XATTR_MAX_HDR_SIZE; break; default: return NULL; -- 2.21.0 From ddd9d04b1aec1df60b04f3d558d2548256b1645a Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 24 Oct 2019 17:15:18 +0200 Subject: [PATCH 5/5] lib/adouble: pass filesize to ad_unpack() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ad_unpack() needs the filesize, not the capped IO size we're using in the caller to read up to "size" bystem from the ._ AppleDouble file. This fixes a regression introduced by bdc257a1cbac7e8c73a084b618ba642476807483 for bug 13968. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14171 RN: vfs_fruit returns capped resource fork length Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher Autobuild-User(master): Ralph Böhme Autobuild-Date(master): Wed Oct 30 14:52:34 UTC 2019 on sn-devel-184 (backported from commit f3df83a2c346d945487a27a9d258ee6331ea7dbb) --- selftest/knownfail.d/samba3.vfs.fruit | 3 --- source3/modules/vfs_fruit.c | 4 +++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit index 6982e100861..6307e2b3404 100644 --- a/selftest/knownfail.d/samba3.vfs.fruit +++ b/selftest/knownfail.d/samba3.vfs.fruit @@ -1,5 +1,2 @@ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion\(nt4_dc\) ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\) -^samba3.vfs.fruit fruit_delete_empty_adfiles.resource fork IO\(nt4_dc\) -^samba3.vfs.fruit metadata_stream.resource fork IO\(nt4_dc\) -^samba3.vfs.fruit metadata_netatalk.resource fork IO\(nt4_dc\) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 131513423bd..b8ede0cdfdb 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -1693,7 +1693,9 @@ static ssize_t ad_read_rsrc_adouble(vfs_handle_struct *handle, } /* Now parse entries */ - ok = ad_unpack(ad, ADEID_NUM_DOT_UND, size); + ok = ad_unpack(ad, + ADEID_NUM_DOT_UND, + ad->ad_fsp->fsp_name->st.st_ex_size); if (!ok) { DBG_ERR("invalid AppleDouble resource %s\n", smb_fname->base_name); -- 2.21.0