From 222b16ac61329dc819ab5b9ccd3276c5a1a01c8f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher 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 Reviewed-by: Ralph Boehme (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 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 Reviewed-by: Ralph Boehme (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 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 Reviewed-by: Ralph Boehme (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