The Samba-Bugzilla – Attachment 13910 Details for
Bug 13181
Fail to copy file with empty FinderInfo from Windows client to Samba share with fruit
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for 4.7 cherry-picked from master
bug13181-v47.patch (text/plain), 16.49 KB, created by
Ralph Böhme
on 2018-01-16 09:56:20 UTC
(
hide
)
Description:
Patch for 4.7 cherry-picked from master
Filename:
MIME Type:
Creator:
Ralph Böhme
Created:
2018-01-16 09:56:20 UTC
Size:
16.49 KB
patch
obsolete
>From ed34d8a05bb4f4f67cb3883f1f707a426c987aec Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 6 Dec 2017 22:05:23 +0100 >Subject: [PATCH 1/5] s4/torture/fruit: ensure AFP_AfpInfo blobs are > 0-initialized > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=13181 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit a22833c2971dc7234b32741305f40ed62e232e0b) >--- > source4/torture/vfs/fruit.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c >index 04f04e2cd56..309b4d2220b 100644 >--- a/source4/torture/vfs/fruit.c >+++ b/source4/torture/vfs/fruit.c >@@ -909,7 +909,7 @@ static char *torture_afpinfo_pack(TALLOC_CTX *mem_ctx, > { > char *buf; > >- buf = talloc_array(mem_ctx, char, AFP_INFO_SIZE); >+ buf = talloc_zero_array(mem_ctx, char, AFP_INFO_SIZE); > if (buf == NULL) { > return NULL; > } >-- >2.13.6 > > >From 3fc424564e02ea2d9500157afa56e353d89c1d4e Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 7 Dec 2017 13:43:02 +0100 >Subject: [PATCH 2/5] s4/torture/fruit: enhance zero AFP_AfpInfo stream test > >This test more operations in the zeroed out FinderInfo test, ensuring >after zeroing out FinderInfo, operations on the filehandle still work >and that enumerating streams doesn't return the stream anymore. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=13181 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit df31e94eb6241f5e5594f6fd0ec1ad7896e02e27) >--- > selftest/knownfail.d/samba3.vfs.fruit | 2 + > source4/torture/vfs/fruit.c | 87 +++++++++++++++++++++++++++++++++-- > 2 files changed, 86 insertions(+), 3 deletions(-) > >diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit >index 8df25bccb79..e8fd5e11fc2 100644 >--- a/selftest/knownfail.d/samba3.vfs.fruit >+++ b/selftest/knownfail.d/samba3.vfs.fruit >@@ -1 +1,3 @@ > ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion\(nt4_dc\) >+^samba3.vfs.fruit metadata_stream.delete AFP_AfpInfo by writing all 0\(nt4_dc\) >+^samba3.vfs.fruit streams_depot.delete AFP_AfpInfo by writing all 0\(nt4_dc\) >diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c >index 309b4d2220b..0f5d93ec12f 100644 >--- a/source4/torture/vfs/fruit.c >+++ b/source4/torture/vfs/fruit.c >@@ -3346,11 +3346,17 @@ static bool test_afpinfo_all0(struct torture_context *tctx, > { > bool ret = true; > NTSTATUS status; >- struct smb2_handle h1; >+ struct smb2_create create; >+ struct smb2_handle h1 = {{0}}; >+ struct smb2_handle baseh = {{0}}; >+ union smb_setfileinfo setfinfo; >+ union smb_fileinfo getfinfo; > TALLOC_CTX *mem_ctx = talloc_new(tctx); > const char *fname = BASEDIR "\\file"; >+ const char *sname = BASEDIR "\\file" AFPINFO_STREAM; > const char *type_creator = "SMB,OLE!"; > AfpInfo *info = NULL; >+ char *infobuf = NULL; > const char *streams_basic[] = { > "::$DATA" > }; >@@ -3381,13 +3387,88 @@ static bool test_afpinfo_all0(struct torture_context *tctx, > > /* Write all 0 to AFP_AfpInfo */ > memset(info->afpi_FinderInfo, 0, AFP_FinderSize); >- ret = torture_write_afpinfo(tree, tctx, mem_ctx, fname, info); >- torture_assert_goto(tctx, ret == true, ret, done, "torture_write_afpinfo failed"); >+ infobuf = torture_afpinfo_pack(mem_ctx, info); >+ torture_assert_not_null_goto(tctx, infobuf, ret, done, >+ "torture_afpinfo_pack failed\n"); >+ >+ ZERO_STRUCT(create); >+ create.in.desired_access = SEC_FILE_ALL; >+ create.in.file_attributes = FILE_ATTRIBUTE_NORMAL; >+ create.in.create_disposition = NTCREATEX_DISP_OPEN; >+ create.in.share_access = NTCREATEX_SHARE_ACCESS_MASK; >+ create.in.fname = fname; >+ >+ status = smb2_create(tree, mem_ctx, &create); >+ torture_assert_goto(tctx, ret == true, ret, done, >+ "smb2_create failed\n"); >+ baseh = create.out.file.handle; >+ >+ ZERO_STRUCT(create); >+ create.in.desired_access = SEC_FILE_ALL; >+ create.in.file_attributes = FILE_ATTRIBUTE_NORMAL; >+ create.in.create_disposition = NTCREATEX_DISP_OVERWRITE_IF; >+ create.in.fname = sname; >+ >+ status = smb2_create(tree, mem_ctx, &create); >+ torture_assert_goto(tctx, ret == true, ret, done, >+ "smb2_create failed\n"); >+ h1 = create.out.file.handle; >+ >+ status = smb2_util_write(tree, h1, infobuf, 0, AFP_INFO_SIZE); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "smb2_util_write failed\n"); >+ >+ /* >+ * Get stream information on open handle, must return only default >+ * stream, the AFP_AfpInfo stream must not be returned. >+ */ >+ >+ ZERO_STRUCT(getfinfo); >+ getfinfo.generic.level = RAW_FILEINFO_STREAM_INFORMATION; >+ getfinfo.generic.in.file.handle = baseh; >+ >+ status = smb2_getinfo_file(tree, tctx, &getfinfo); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "get stream info\n"); >+ >+ torture_assert_int_equal_goto(tctx, getfinfo.stream_info.out.num_streams, >+ 1, ret, done, "stream count"); >+ >+ smb2_util_close(tree, baseh); >+ ZERO_STRUCT(baseh); >+ >+ /* >+ * Try to set some file-basic-info (time) on the stream. This catches >+ * naive implementation mistakes that simply deleted the backing store >+ * from the filesystem in the zero-out step. >+ */ >+ >+ ZERO_STRUCT(setfinfo); >+ unix_to_nt_time(&setfinfo.basic_info.in.write_time, time(NULL)); >+ setfinfo.basic_info.in.attrib = 0x20; >+ setfinfo.generic.level = RAW_SFILEINFO_BASIC_INFORMATION; >+ setfinfo.generic.in.file.handle = h1; >+ >+ status = smb2_setinfo_file(tree, &setfinfo); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "smb2_getinfo_file failed\n"); >+ >+ ret = check_stream_list(tree, tctx, fname, 1, streams_basic, false); >+ torture_assert_goto(tctx, ret == true, ret, done, "check_stream_list"); >+ >+ smb2_util_close(tree, h1); >+ ZERO_STRUCT(h1); > > ret = check_stream_list(tree, tctx, fname, 1, streams_basic, false); > torture_assert_goto(tctx, ret == true, ret, done, "Bad streams"); > > done: >+ if (!smb2_util_handle_empty(h1)) { >+ smb2_util_close(tree, h1); >+ } >+ if (!smb2_util_handle_empty(baseh)) { >+ smb2_util_close(tree, baseh); >+ } > smb2_util_unlink(tree, fname); > smb2_util_rmdir(tree, BASEDIR); > return ret; >-- >2.13.6 > > >From 46e97a5409fda5d55e8710c9d6eaf836b3be430c Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 7 Dec 2017 14:56:36 +0100 >Subject: [PATCH 3/5] vfs_fruit: factor out delete_invalid_meta_stream() from > fruit_streaminfo_meta_stream() > >No change in behaviour, just some refactoring before adding more code to >fruit_streaminfo_meta_stream() in the next commit. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=13181 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit c41e1ea9247611473d30184efd953c61955ead15) >--- > source3/modules/vfs_fruit.c | 57 +++++++++++++++++++++++++++------------------ > 1 file changed, 34 insertions(+), 23 deletions(-) > >diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c >index 8c3870b3495..dc2241784d1 100644 >--- a/source3/modules/vfs_fruit.c >+++ b/source3/modules/vfs_fruit.c >@@ -4864,39 +4864,17 @@ static int fruit_fstat(vfs_handle_struct *handle, files_struct *fsp, > return rc; > } > >-static NTSTATUS fruit_streaminfo_meta_stream( >+static NTSTATUS delete_invalid_meta_stream( > vfs_handle_struct *handle, >- struct files_struct *fsp, > const struct smb_filename *smb_fname, > TALLOC_CTX *mem_ctx, > unsigned int *pnum_streams, > struct stream_struct **pstreams) > { >- struct stream_struct *stream = *pstreams; >- unsigned int num_streams = *pnum_streams; > struct smb_filename *sname = NULL; >- int i; > int ret; > bool ok; > >- for (i = 0; i < num_streams; i++) { >- if (strequal_m(stream[i].name, AFPINFO_STREAM)) { >- break; >- } >- } >- >- if (i == num_streams) { >- return NT_STATUS_OK; >- } >- >- if (stream[i].size == AFP_INFO_SIZE) { >- return NT_STATUS_OK; >- } >- >- DBG_ERR("Removing invalid AFPINFO_STREAM size [%"PRIdMAX"] " >- "from [%s]\n", (intmax_t)stream[i].size, >- smb_fname_str_dbg(smb_fname)); >- > ok = del_fruit_stream(mem_ctx, pnum_streams, pstreams, AFPINFO_STREAM); > if (!ok) { > return NT_STATUS_INTERNAL_ERROR; >@@ -4920,6 +4898,39 @@ static NTSTATUS fruit_streaminfo_meta_stream( > return NT_STATUS_OK; > } > >+static NTSTATUS fruit_streaminfo_meta_stream( >+ vfs_handle_struct *handle, >+ struct files_struct *fsp, >+ const struct smb_filename *smb_fname, >+ TALLOC_CTX *mem_ctx, >+ unsigned int *pnum_streams, >+ struct stream_struct **pstreams) >+{ >+ struct stream_struct *stream = *pstreams; >+ unsigned int num_streams = *pnum_streams; >+ int i; >+ >+ for (i = 0; i < num_streams; i++) { >+ if (strequal_m(stream[i].name, AFPINFO_STREAM)) { >+ break; >+ } >+ } >+ >+ if (i == num_streams) { >+ return NT_STATUS_OK; >+ } >+ >+ if (stream[i].size != AFP_INFO_SIZE) { >+ DBG_ERR("Removing invalid AFPINFO_STREAM size [%jd] from [%s]\n", >+ (intmax_t)stream[i].size, smb_fname_str_dbg(smb_fname)); >+ >+ return delete_invalid_meta_stream(handle, smb_fname, mem_ctx, >+ pnum_streams, pstreams); >+ } >+ >+ return NT_STATUS_OK; >+} >+ > static NTSTATUS fruit_streaminfo_meta_netatalk( > vfs_handle_struct *handle, > struct files_struct *fsp, >-- >2.13.6 > > >From 18ed4bda0db88dd13b383b910d651dfdc7bb6c34 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 7 Dec 2017 17:32:35 +0100 >Subject: [PATCH 4/5] vfs_fruit: filter out AFP_AfpInfo streams with pending > delete-on-close > >This is in preperation of fixing the implementation of removing the >AFP_AfpInfo stream by zeroing the FinderInfo out. > >We currently remove the stream blob from the underyling filesystem >backing store, but that results in certain operations to fail on any >still open file-handle. > >The fix comes in the next commit which will convert to backing store >delete operation to a set delete-on-close on the stream. > >This commit adds filtering on streams that have the delete-on-close >set. It is only needed for the fruit:metadata=stream case, as with >fruit:metadata=netatalk the filtering is already done in >fruit_streaminfo_meta_netatalk(). > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=13181 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit 70d8f7c5d25f35b58620c2db8f57c7c0758267b3) >--- > source3/modules/vfs_fruit.c | 72 ++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 71 insertions(+), 1 deletion(-) > >diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c >index dc2241784d1..7d4d614415d 100644 >--- a/source3/modules/vfs_fruit.c >+++ b/source3/modules/vfs_fruit.c >@@ -4908,7 +4908,16 @@ static NTSTATUS fruit_streaminfo_meta_stream( > { > struct stream_struct *stream = *pstreams; > unsigned int num_streams = *pnum_streams; >+ struct smb_filename *sname = NULL; >+ char *full_name = NULL; >+ uint32_t name_hash; >+ struct share_mode_lock *lck = NULL; >+ struct file_id id = {0}; >+ bool delete_on_close_set; > int i; >+ int ret; >+ NTSTATUS status; >+ bool ok; > > for (i = 0; i < num_streams; i++) { > if (strequal_m(stream[i].name, AFPINFO_STREAM)) { >@@ -4928,7 +4937,68 @@ static NTSTATUS fruit_streaminfo_meta_stream( > pnum_streams, pstreams); > } > >- return NT_STATUS_OK; >+ /* >+ * Now check if there's a delete-on-close pending on the stream. If so, >+ * hide the stream. This behaviour was verified against a macOS 10.12 >+ * SMB server. >+ */ >+ >+ sname = synthetic_smb_fname(talloc_tos(), >+ smb_fname->base_name, >+ AFPINFO_STREAM_NAME, >+ NULL, 0); >+ if (sname == NULL) { >+ status = NT_STATUS_NO_MEMORY; >+ goto out; >+ } >+ >+ ret = SMB_VFS_NEXT_STAT(handle, sname); >+ if (ret != 0) { >+ status = map_nt_error_from_unix(errno); >+ goto out; >+ } >+ >+ id = SMB_VFS_NEXT_FILE_ID_CREATE(handle, &sname->st); >+ >+ lck = get_existing_share_mode_lock(talloc_tos(), id); >+ if (lck == NULL) { >+ status = NT_STATUS_OK; >+ goto out; >+ } >+ >+ full_name = talloc_asprintf(talloc_tos(), >+ "%s%s", >+ sname->base_name, >+ AFPINFO_STREAM); >+ if (full_name == NULL) { >+ status = NT_STATUS_NO_MEMORY; >+ goto out; >+ } >+ >+ status = file_name_hash(handle->conn, full_name, &name_hash); >+ if (!NT_STATUS_IS_OK(status)) { >+ goto out; >+ } >+ >+ delete_on_close_set = is_delete_on_close_set(lck, name_hash); >+ if (delete_on_close_set) { >+ ok = del_fruit_stream(mem_ctx, >+ pnum_streams, >+ pstreams, >+ AFPINFO_STREAM); >+ if (!ok) { >+ status = NT_STATUS_INTERNAL_ERROR; >+ goto out; >+ } >+ } >+ >+ status = NT_STATUS_OK; >+ >+out: >+ TALLOC_FREE(sname); >+ TALLOC_FREE(lck); >+ TALLOC_FREE(full_name); >+ return status; > } > > static NTSTATUS fruit_streaminfo_meta_netatalk( >-- >2.13.6 > > >From 27cbd1d5e198a75d9b7fd6ffd4884a55acce064d Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 6 Dec 2017 22:09:52 +0100 >Subject: [PATCH 5/5] vfs_fruit: set delete-on-close for empty finderinfo >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >We previously removed the stream from the underlying filesystem stream >backing store when the client zeroes out FinderInfo in the AFP_AfpInfo >stream, but this causes certain operations to fail (eg stat) when trying >to access the stream over any file-handle open on that stream. > >So instead of deleting, set delete-on-close on the stream. The previous >commit already implemented not to list list streams with delete-on-close >set which is necessary to implemenent correct macOS semantics for this >particular stream. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=13181 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> > >Autobuild-User(master): Ralph Böhme <slow@samba.org> >Autobuild-Date(master): Tue Jan 9 17:09:12 CET 2018 on sn-devel-144 > >(cherry picked from commit e61e9e98e9ff461055daae2fe78f0202f7ed8663) >--- > selftest/knownfail.d/samba3.vfs.fruit | 2 -- > source3/modules/vfs_fruit.c | 60 +++++++++++++++++++++-------------- > 2 files changed, 36 insertions(+), 26 deletions(-) > >diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit >index e8fd5e11fc2..8df25bccb79 100644 >--- a/selftest/knownfail.d/samba3.vfs.fruit >+++ b/selftest/knownfail.d/samba3.vfs.fruit >@@ -1,3 +1 @@ > ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion\(nt4_dc\) >-^samba3.vfs.fruit metadata_stream.delete AFP_AfpInfo by writing all 0\(nt4_dc\) >-^samba3.vfs.fruit streams_depot.delete AFP_AfpInfo by writing all 0\(nt4_dc\) >diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c >index 7d4d614415d..1e9ec589d05 100644 >--- a/source3/modules/vfs_fruit.c >+++ b/source3/modules/vfs_fruit.c >@@ -4134,26 +4134,35 @@ static ssize_t fruit_pwrite_meta_stream(vfs_handle_struct *handle, > size_t n, off_t offset) > { > AfpInfo *ai = NULL; >- int ret; >+ size_t nwritten; >+ bool ok; > > ai = afpinfo_unpack(talloc_tos(), data); > if (ai == NULL) { > return -1; > } > >- if (ai_empty_finderinfo(ai)) { >- ret = SMB_VFS_NEXT_UNLINK(handle, fsp->fsp_name); >- if (ret != 0 && errno != ENOENT && errno != ENOATTR) { >- DBG_ERR("Can't delete metadata for %s: %s\n", >- fsp_str_dbg(fsp), strerror(errno)); >- TALLOC_FREE(ai); >- return -1; >- } >+ nwritten = SMB_VFS_NEXT_PWRITE(handle, fsp, data, n, offset); >+ if (nwritten != n) { >+ return -1; >+ } > >+ if (!ai_empty_finderinfo(ai)) { > return n; > } > >- return SMB_VFS_NEXT_PWRITE(handle, fsp, data, n, offset); >+ ok = set_delete_on_close( >+ fsp, >+ true, >+ handle->conn->session_info->security_token, >+ handle->conn->session_info->unix_token); >+ if (!ok) { >+ DBG_ERR("set_delete_on_close on [%s] failed\n", >+ fsp_str_dbg(fsp)); >+ return -1; >+ } >+ >+ return n; > } > > static ssize_t fruit_pwrite_meta_netatalk(vfs_handle_struct *handle, >@@ -4164,26 +4173,13 @@ static ssize_t fruit_pwrite_meta_netatalk(vfs_handle_struct *handle, > AfpInfo *ai = NULL; > char *p = NULL; > int ret; >+ bool ok; > > ai = afpinfo_unpack(talloc_tos(), data); > if (ai == NULL) { > return -1; > } > >- if (ai_empty_finderinfo(ai)) { >- ret = SMB_VFS_REMOVEXATTR(handle->conn, >- fsp->fsp_name, >- AFPINFO_EA_NETATALK); >- >- if (ret != 0 && errno != ENOENT && errno != ENOATTR) { >- DBG_ERR("Can't delete metadata for %s: %s\n", >- fsp_str_dbg(fsp), strerror(errno)); >- return -1; >- } >- >- return n; >- } >- > ad = ad_fget(talloc_tos(), handle, fsp, ADOUBLE_META); > if (ad == NULL) { > ad = ad_init(talloc_tos(), handle, ADOUBLE_META); >@@ -4208,6 +4204,22 @@ static ssize_t fruit_pwrite_meta_netatalk(vfs_handle_struct *handle, > } > > TALLOC_FREE(ad); >+ >+ if (!ai_empty_finderinfo(ai)) { >+ return n; >+ } >+ >+ ok = set_delete_on_close( >+ fsp, >+ true, >+ handle->conn->session_info->security_token, >+ handle->conn->session_info->unix_token); >+ if (!ok) { >+ DBG_ERR("set_delete_on_close on [%s] failed\n", >+ fsp_str_dbg(fsp)); >+ return -1; >+ } >+ > return n; > } > >-- >2.13.6 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=13910">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
jra
:
review+
Actions:
View
Attachments on
bug 13181
: 13910 |
13911