Blob Blame History Raw
From 222b16ac61329dc819ab5b9ccd3276c5a1a01c8f Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Thu, 2 Jul 2020 14:32:34 +0200
Subject: [PATCH 1/3] s4:torture/smb2: add smb2.delete-on-close-perms.BUG14427

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14427

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
(cherry picked from commit bcba4bb210d9482be4c2c8dadfb5cc185046cbaa)
---
 selftest/knownfail.d/bug14427          |  1 +
 source4/torture/smb2/delete-on-close.c | 43 +++++++++++++++++++++++++-
 2 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 selftest/knownfail.d/bug14427

diff --git a/selftest/knownfail.d/bug14427 b/selftest/knownfail.d/bug14427
new file mode 100644
index 00000000000..e136465ef87
--- /dev/null
+++ b/selftest/knownfail.d/bug14427
@@ -0,0 +1 @@
+^samba3.smb2.delete-on-close-perms.BUG14427
diff --git a/source4/torture/smb2/delete-on-close.c b/source4/torture/smb2/delete-on-close.c
index 3c495750f43..05242876dcb 100644
--- a/source4/torture/smb2/delete-on-close.c
+++ b/source4/torture/smb2/delete-on-close.c
@@ -698,6 +698,46 @@ static bool test_doc_read_only(struct torture_context *tctx,
 	return ret;
 }
 
+/*
+ * This is a regression test for
+ * https://bugzilla.samba.org/show_bug.cgi?id=14427
+ *
+ * It's not really a delete-on-close specific test.
+ */
+static bool test_doc_bug14427(struct torture_context *tctx, struct smb2_tree *tree1)
+{
+	struct smb2_tree *tree2 = NULL;
+	NTSTATUS status;
+	char fname[256];
+	bool ret = false;
+	bool ok;
+
+	/* Add some random component to the file name. */
+	snprintf(fname, sizeof(fname), "doc_bug14427_%s.dat",
+		 generate_random_str(tctx, 8));
+
+	ok = torture_smb2_tree_connect(tctx, tree1->session, tctx, &tree2);
+	torture_assert_goto(tctx, ok, ret, done,
+		"torture_smb2_tree_connect() failed.\n");
+
+	status = torture_setup_simple_file(tctx, tree1, fname);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+		"torture_setup_simple_file() failed on tree1.\n");
+
+	status = smb2_util_unlink(tree2, fname);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+		"smb2_util_unlink() failed on tree2.\n");
+	TALLOC_FREE(tree2);
+	ret = true;
+done:
+	if (tree2 != NULL) {
+		TALLOC_FREE(tree2);
+		smb2_util_unlink(tree1, fname);
+	}
+
+	TALLOC_FREE(tree1);
+	return ret;
+}
 
 /*
  *  Extreme testing of Delete On Close and permissions
@@ -713,7 +753,8 @@ struct torture_suite *torture_smb2_doc_init(TALLOC_CTX *ctx)
 	torture_suite_add_1smb2_test(suite, "CREATE_IF", test_doc_create_if);
 	torture_suite_add_1smb2_test(suite, "CREATE_IF Existing", test_doc_create_if_exist);
 	torture_suite_add_1smb2_test(suite, "FIND_and_set_DOC", test_doc_find_and_set_doc);
-	torture_suite_add_1smb2_test(suite,  "READONLY", test_doc_read_only);
+	torture_suite_add_1smb2_test(suite, "READONLY", test_doc_read_only);
+	torture_suite_add_1smb2_test(suite, "BUG14427", test_doc_bug14427);
 
 	suite->description = talloc_strdup(suite, "SMB2-Delete-on-Close-Perms tests");
 
-- 
2.26.2


From a6005fb5155a7c7886b179e7672b198a55e69380 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Thu, 2 Jul 2020 12:06:28 +0200
Subject: [PATCH 2/3] s3:smbd: reformat if statement for caching in vfs_ChDir()

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14427

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
(cherry picked from commit b2b5ae090ee8796609eb0b5794bc4e62c24414ef)
---
 source3/smbd/vfs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
index 96067e45005..7c8f99bbd41 100644
--- a/source3/smbd/vfs.c
+++ b/source3/smbd/vfs.c
@@ -879,8 +879,9 @@ int vfs_ChDir(connection_struct *conn, const struct smb_filename *smb_fname)
 		return 0;
 	}
 
-	if (*smb_fname->base_name == '/' &&
-			strcsequal(LastDir,smb_fname->base_name)) {
+	if (smb_fname->base_name[0] == '/' &&
+	    strcsequal(LastDir,smb_fname->base_name))
+	{
 		return 0;
 	}
 
-- 
2.26.2


From 735fd5fe21b4c365946806e79df668cec22b3210 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Wed, 1 Jul 2020 09:38:58 +0200
Subject: [PATCH 3/3] s3:smbd: make sure vfs_ChDir() always sets
 conn->cwd_fsp->fh->fd = AT_FDCWD

This is what all consumers of conn->cwd_fsp->fh->fd expect!

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14427

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
(cherry picked from commit f3f330f61db983f6d213a097d9a4d91b1057ecb1)
---
 selftest/knownfail.d/bug14427 |  1 -
 source3/smbd/vfs.c            | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)
 delete mode 100644 selftest/knownfail.d/bug14427

diff --git a/selftest/knownfail.d/bug14427 b/selftest/knownfail.d/bug14427
deleted file mode 100644
index e136465ef87..00000000000
--- a/selftest/knownfail.d/bug14427
+++ /dev/null
@@ -1 +0,0 @@
-^samba3.smb2.delete-on-close-perms.BUG14427
diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
index 7c8f99bbd41..411999c3856 100644
--- a/source3/smbd/vfs.c
+++ b/source3/smbd/vfs.c
@@ -876,12 +876,47 @@ int vfs_ChDir(connection_struct *conn, const struct smb_filename *smb_fname)
 	}
 
 	if (ISDOT(smb_fname->base_name)) {
+		/*
+		 * passing a '.' is a noop,
+		 * and we only expect this after
+		 * everything is initialized.
+		 *
+		 * So the first vfs_ChDir() on a given
+		 * connection_struct must not be '.'.
+		 *
+		 * Note: conn_new() sets
+		 * conn->cwd_fsp->fh->fd = -1
+		 * and vfs_ChDir() leaves with
+		 * conn->cwd_fsp->fh->fd = AT_FDCWD
+		 * on success!
+		 */
+		if (conn->cwd_fsp->fh->fd != AT_FDCWD) {
+			/*
+			 * This should never happen and
+			 * we might change this to
+			 * SMB_ASSERT() in future.
+			 */
+			DBG_ERR("Called with '.' as first operation!\n");
+			log_stack_trace();
+			errno = EINVAL;
+			return -1;
+		}
 		return 0;
 	}
 
 	if (smb_fname->base_name[0] == '/' &&
 	    strcsequal(LastDir,smb_fname->base_name))
 	{
+		/*
+		 * conn->cwd_fsp->fsp_name and the kernel
+		 * are already correct, but conn->cwd_fsp->fh->fd
+		 * might still be -1 as initialized in conn_new().
+		 *
+		 * This can happen when a client made a 2nd
+		 * tree connect to a share with the same underlying
+		 * path (may or may not the same share).
+		 */
+		conn->cwd_fsp->fh->fd = AT_FDCWD;
 		return 0;
 	}
 
-- 
2.26.2