[PATCH] vfs_fruit: resource fork open request with flags=O_CREAT|O_RDONLY

Ralph Böhme slow at samba.org
Wed Feb 8 10:11:09 UTC 2017


Hi!

Attached is patch for bug:
<https://bugzilla.samba.org/show_bug.cgi?id=12565>

This one should go on top of the large fruit rewrite patch as well. It was
triggered in an interesting way:

- a customer was using a streams module, but without fruit

- resource fork stream data was stored in the streams VFS backend
  (streams_xattr, but it would happend with streams_depot just the same)

- they enabled fruit by adding it to the VFS stack

- now the following sequence of SMB requests were seen:

  1. list streams of a file, this wrongly includes the resource stream from the
  streams_xattr backend

  2. client thinks "oh hey, there's a resource fork, let's fetch it", so it
  sends a create request with open_if (!) disposition and read-only access (!)

  3. this ends up in vfs_fruit who does *not* find an existing resource fork
  AppleDouble file and thus creates one (remember: open_if)

  4. after creating it, it goes out to write the AppleDouble header information
  for the newly created file, but this fails because we opened the file in
  O_RDONLY as requested by the client *ouch*

  5. this leaves behind an uninitialized 0-byte ._ AppleDouble file leading to
  all sorts of follow-up errors

I wrote a toture test that simulated this request and ran it against macOS SMB
server. To my surprise it simply returned NT_STATUS_OBJECT_NAME_NOT_FOUND
(again, remember: open_if disposition, go figure...).

Another instance where the macOS SMB server exposes filesystem behaviour and we
have to adapt to be compatible.

Cheerio!
-slow
-------------- next part --------------
From fc7bdd4e7675664e1e6ce440505e0a5a43007f1d Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 7 Feb 2017 07:44:40 +0100
Subject: [PATCH 1/2] vfs_fruit: resource fork open request with
 flags=O_CREAT|O_RDONLY

When receiving an SMB create request with read-only access mode and
open_if disposition, we end of calling the open() function with
flags=O_CREAT|O_RDONLY for the ._ AppleDouble file.

If the file doesn't exist, ie there's currently no rsrc stream, we create
it but then we fail to write the AppleDouble header into the file due to
the O_RDONLY open mode, leaving a 0 byte size ._ file.

Running this create requests against macOS SMB server yields an
interesting result: it returns NT_STATUS_OBJECT_NAME_NOT_FOUND. Another
instance where the SMB server exposes FSA behaviour and we have to adapt
to be compatible.

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

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

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 3315d44..3283d5d 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -2957,6 +2957,20 @@ static int fruit_open_rsrc(vfs_handle_struct *handle,
 	SMB_VFS_HANDLE_GET_DATA(handle, config,
 				struct fruit_config_data, return -1);
 
+	if (((flags & O_ACCMODE) == O_RDONLY)
+	    && (flags & O_CREAT)
+	    && !VALID_STAT(fsp->fsp_name->st))
+	{
+		/*
+		 * This means the stream doesn't exist. macOS SMB server fails
+		 * this with NT_STATUS_OBJECT_NAME_NOT_FOUND, so must we. Cf bug
+		 * 12565 and the test for this combination in
+		 * test_rfork_create().
+		 */
+		errno = ENOENT;
+		return -1;
+	}
+
 	switch (config->rsrc) {
 	case FRUIT_RSRC_STREAM:
 		fd = SMB_VFS_NEXT_OPEN(handle, smb_fname, fsp, flags, mode);
-- 
2.9.3


From ba4ccddaab4f592cd34571c028a32b0aa9405670 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 7 Feb 2017 15:13:15 +0100
Subject: [PATCH 2/2] s4/torture: vfs_fruit: test for bug 12565

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

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source4/torture/vfs/fruit.c | 67 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
index 92ef06a..023847c 100644
--- a/source4/torture/vfs/fruit.c
+++ b/source4/torture/vfs/fruit.c
@@ -1814,6 +1814,71 @@ done:
 	return ret;
 }
 
+static bool test_rfork_create_ro(struct torture_context *tctx,
+				 struct smb2_tree *tree)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
+	const char *fname = BASEDIR "\\torture_rfork_create";
+	const char *rfork = BASEDIR "\\torture_rfork_create" AFPRESOURCE_STREAM;
+	NTSTATUS status;
+	struct smb2_handle testdirh;
+	bool ret = true;
+	struct smb2_create create;
+
+	smb2_util_unlink(tree, fname);
+	status = torture_smb2_testdir(tree, BASEDIR, &testdirh);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "torture_smb2_testdir\n");
+	smb2_util_close(tree, testdirh);
+
+	ret = torture_setup_file(mem_ctx, tree, fname, false);
+	if (ret == false) {
+		goto done;
+	}
+
+	torture_comment(tctx, "(%s) Try opening read-only with open_if create disposition, should return ENOENT\n",
+			__location__);
+
+	ZERO_STRUCT(create);
+	create.in.fname = rfork;
+	create.in.create_disposition = NTCREATEX_DISP_OPEN_IF;
+	create.in.desired_access = SEC_FILE_READ_DATA | SEC_STD_READ_CONTROL;
+	create.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+	create.in.share_access = FILE_SHARE_READ | FILE_SHARE_DELETE;
+	status = smb2_create(tree, mem_ctx, &(create));
+	torture_assert_ntstatus_equal_goto(tctx, status, NT_STATUS_OBJECT_NAME_NOT_FOUND,
+					   ret, done, "smb2_create failed\n");
+
+	torture_comment(tctx, "(%s) Now write something to the rsrc stream, then the same open should succeed\n",
+			__location__);
+
+	ret = write_stream(tree, __location__, tctx, mem_ctx,
+			   fname, AFPRESOURCE_STREAM_NAME,
+			   0, 3, "foo");
+	torture_assert_goto(tctx, ret == true, ret, done, "write_stream failed\n");
+
+	ret = check_stream(tree, __location__, tctx, mem_ctx,
+			   fname, AFPRESOURCE_STREAM,
+			   0, 3, 0, 3, "foo");
+	torture_assert_goto(tctx, ret == true, ret, done, "check_stream");
+
+	ZERO_STRUCT(create);
+	create.in.fname = rfork;
+	create.in.create_disposition = NTCREATEX_DISP_OPEN_IF;
+	create.in.desired_access = SEC_FILE_READ_DATA | SEC_STD_READ_CONTROL;
+	create.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+	create.in.share_access = FILE_SHARE_READ | FILE_SHARE_DELETE;
+	status = smb2_create(tree, mem_ctx, &(create));
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_create failed\n");
+
+	smb2_util_close(tree, create.out.file.handle);
+
+done:
+	smb2_util_unlink(tree, fname);
+	smb2_deltree(tree, BASEDIR);
+	talloc_free(mem_ctx);
+	return ret;
+}
+
 static bool test_adouble_conversion(struct torture_context *tctx,
 				    struct smb2_tree *tree)
 {
@@ -3939,8 +4004,8 @@ struct torture_suite *torture_vfs_fruit(void)
 	torture_suite_add_1smb2_test(suite, "delete", test_delete_file_with_rfork);
 	torture_suite_add_1smb2_test(suite, "read open rsrc after rename", test_rename_and_read_rsrc);
 	torture_suite_add_1smb2_test(suite, "readdir_attr with names with illegal ntfs characters", test_readdir_attr_illegal_ntfs);
-
 	torture_suite_add_2ns_smb2_test(suite, "invalid AFP_AfpInfo", test_invalid_afpinfo);
+	torture_suite_add_1smb2_test(suite, "creating rsrc with read-only access", test_rfork_create_ro);
 
 	return suite;
 }
-- 
2.9.3



More information about the samba-technical mailing list