[SCM] Samba Shared Repository - branch master updated

Ralph Böhme slow at samba.org
Tue Jan 9 16:10:02 UTC 2018


The branch, master has been updated
       via  e61e9e9 vfs_fruit: set delete-on-close for empty finderinfo
       via  70d8f7c vfs_fruit: filter out AFP_AfpInfo streams with pending delete-on-close
       via  c41e1ea vfs_fruit: factor out delete_invalid_meta_stream() from fruit_streaminfo_meta_stream()
       via  df31e94 s4/torture/fruit: enhance zero AFP_AfpInfo stream test
       via  a22833c s4/torture/fruit: ensure AFP_AfpInfo blobs are 0-initialized
      from  7901f7c selftest: close connections after tests in samba4.ldap.rodc_rwdc.python

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit e61e9e98e9ff461055daae2fe78f0202f7ed8663
Author: Ralph Boehme <slow at samba.org>
Date:   Wed Dec 6 22:09:52 2017 +0100

    vfs_fruit: set delete-on-close for empty finderinfo
    
    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 at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Ralph Böhme <slow at samba.org>
    Autobuild-Date(master): Tue Jan  9 17:09:12 CET 2018 on sn-devel-144

commit 70d8f7c5d25f35b58620c2db8f57c7c0758267b3
Author: Ralph Boehme <slow at samba.org>
Date:   Thu Dec 7 17:32:35 2017 +0100

    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 at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit c41e1ea9247611473d30184efd953c61955ead15
Author: Ralph Boehme <slow at samba.org>
Date:   Thu Dec 7 14:56:36 2017 +0100

    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 at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit df31e94eb6241f5e5594f6fd0ec1ad7896e02e27
Author: Ralph Boehme <slow at samba.org>
Date:   Thu Dec 7 13:43:02 2017 +0100

    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 at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit a22833c2971dc7234b32741305f40ed62e232e0b
Author: Ralph Boehme <slow at samba.org>
Date:   Wed Dec 6 22:05:23 2017 +0100

    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 at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 source3/modules/vfs_fruit.c | 173 ++++++++++++++++++++++++++++++++++----------
 source4/torture/vfs/fruit.c |  89 ++++++++++++++++++++++-
 2 files changed, 218 insertions(+), 44 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index dfc0dcc..9533da4 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -4166,26 +4166,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,
@@ -4196,26 +4205,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);
@@ -4240,6 +4236,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;
 }
 
@@ -4896,6 +4908,40 @@ static int fruit_fstat(vfs_handle_struct *handle, files_struct *fsp,
 	return rc;
 }
 
+static NTSTATUS delete_invalid_meta_stream(
+	vfs_handle_struct *handle,
+	const struct smb_filename *smb_fname,
+	TALLOC_CTX *mem_ctx,
+	unsigned int *pnum_streams,
+	struct stream_struct **pstreams)
+{
+	struct smb_filename *sname = NULL;
+	int ret;
+	bool ok;
+
+	ok = del_fruit_stream(mem_ctx, pnum_streams, pstreams, AFPINFO_STREAM);
+	if (!ok) {
+		return NT_STATUS_INTERNAL_ERROR;
+	}
+
+	sname = synthetic_smb_fname(talloc_tos(),
+				    smb_fname->base_name,
+				    AFPINFO_STREAM_NAME,
+				    NULL, 0);
+	if (sname == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	ret = SMB_VFS_NEXT_UNLINK(handle, sname);
+	TALLOC_FREE(sname);
+	if (ret != 0) {
+		DBG_ERR("Removing [%s] failed\n", smb_fname_str_dbg(sname));
+		return map_nt_error_from_unix(errno);
+	}
+
+	return NT_STATUS_OK;
+}
+
 static NTSTATUS fruit_streaminfo_meta_stream(
 	vfs_handle_struct *handle,
 	struct files_struct *fsp,
@@ -4907,8 +4953,14 @@ 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++) {
@@ -4921,35 +4973,76 @@ static NTSTATUS fruit_streaminfo_meta_stream(
 		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));
+	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));
 
-	ok = del_fruit_stream(mem_ctx, pnum_streams, pstreams, AFPINFO_STREAM);
-	if (!ok) {
-		return NT_STATUS_INTERNAL_ERROR;
+		return delete_invalid_meta_stream(handle, smb_fname, mem_ctx,
+						  pnum_streams, pstreams);
 	}
 
+	/*
+	 * 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) {
-		return NT_STATUS_NO_MEMORY;
+		status = NT_STATUS_NO_MEMORY;
+		goto out;
 	}
 
-	ret = SMB_VFS_NEXT_UNLINK(handle, sname);
-	TALLOC_FREE(sname);
+	ret = SMB_VFS_NEXT_STAT(handle, sname);
 	if (ret != 0) {
-		DBG_ERR("Removing [%s] failed\n", smb_fname_str_dbg(sname));
-		return map_nt_error_from_unix(errno);
+		status = map_nt_error_from_unix(errno);
+		goto out;
 	}
 
-	return NT_STATUS_OK;
+	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(
diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
index 476e920..d071cf6 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;
 	}
@@ -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;


-- 
Samba Shared Repository



More information about the samba-cvs mailing list