The branch, master has been updated
via 141f3f5f9a5 s3: smbd: Ensure in the directory scanning loops inside
rmdir_internals() we don't overwrite the 'ret' variable.
via adfad639096 s3: smbtorture3: Add test for setting delete on close
on a directory, then creating a file within to see if delete succeeds.
from b919798f575 smbd: early out in is_visible_fsp()
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 141f3f5f9a5ef556cc7864b2afbf8ad48b7ebe77
Author: Jeremy Allison <j...@samba.org>
Date: Wed Nov 3 19:02:36 2021 -0700
s3: smbd: Ensure in the directory scanning loops inside rmdir_internals()
we don't overwrite the 'ret' variable.
If we overwrite with ret=0, we return NT_STATUS_OK even when we goto err.
This function should be restructured to use NT_STATUS internally,
and make 'int ret' transitory, but that's a patch for another
time.
Remove knownfail.
BUG: BUG: https://bugzilla.samba.org/show_bug.cgi?id=14892
Signed-off-by: Jeremy Allison <j...@samba.org>
Reviewed-by: Ralph Boehme <s...@samba.org>
Autobuild-User(master): Ralph Böhme <s...@samba.org>
Autobuild-Date(master): Thu Nov 4 09:10:27 UTC 2021 on sn-devel-184
commit adfad6390962022277cc6aacaa388af86e46b71c
Author: Jeremy Allison <j...@samba.org>
Date: Wed Nov 3 16:50:10 2021 -0700
s3: smbtorture3: Add test for setting delete on close on a directory, then
creating a file within to see if delete succeeds.
Exposes an existing problem where "ret" is overwritten
in the directory scan.
Add knownfail.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14892
Signed-off-by: Jeremy Allison <j...@samba.org>
Reviewed-by: Ralph Boehme <s...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
source3/selftest/tests.py | 15 +++++
source3/smbd/close.c | 13 +++--
source3/torture/proto.h | 1 +
source3/torture/test_smb2.c | 136 ++++++++++++++++++++++++++++++++++++++++++++
source3/torture/torture.c | 4 ++
5 files changed, 163 insertions(+), 6 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 5aba11c11b1..41ed728a03e 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -256,6 +256,21 @@ plantestsuite("samba3.smbtorture_s3.plain.%s" %
"SMB2-LIST-DIR-ASYNC",
smbtorture3,
"",
"-l $LOCAL_PATH"])
+#
+# SMB2-DEL-ON-CLOSE-NONEMPTY needs to run against a special fileserver share
veto_files_delete
+#
+plantestsuite("samba3.smbtorture_s3.plain.%s" % "SMB2-DEL-ON-CLOSE-NONEMPTY",
+ "fileserver",
+ [os.path.join(samba3srcdir,
+ "script/tests/test_smbtorture_s3.sh"),
+ 'SMB2-DEL-ON-CLOSE-NONEMPTY',
+ '//$SERVER_IP/veto_files_delete',
+ '$USERNAME',
+ '$PASSWORD',
+ smbtorture3,
+ "",
+ "-l $LOCAL_PATH"])
+
shares = [
diff --git a/source3/smbd/close.c b/source3/smbd/close.c
index ad10215a4fa..e6272376739 100644
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -1058,6 +1058,7 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct
files_struct *fsp)
struct smb_filename *smb_dname_full = NULL;
struct smb_filename *direntry_fname = NULL;
char *fullname = NULL;
+ int retval;
if (ISDOT(dname) || ISDOTDOT(dname)) {
TALLOC_FREE(talloced);
@@ -1092,8 +1093,8 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct
files_struct *fsp)
goto err;
}
- ret = SMB_VFS_LSTAT(conn, smb_dname_full);
- if (ret != 0) {
+ retval = SMB_VFS_LSTAT(conn, smb_dname_full);
+ if (retval != 0) {
int saved_errno = errno;
TALLOC_FREE(talloced);
TALLOC_FREE(fullname);
@@ -1136,8 +1137,8 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct
files_struct *fsp)
}
/* Not a DFS link - could it be a dangling symlink ? */
- ret = SMB_VFS_STAT(conn, smb_dname_full);
- if (ret == -1 && (errno == ENOENT || errno == ELOOP)) {
+ retval = SMB_VFS_STAT(conn, smb_dname_full);
+ if (retval == -1 && (errno == ENOENT || errno ==
ELOOP)) {
/*
* Dangling symlink.
* Allow delete as "delete veto files = yes"
@@ -1240,8 +1241,8 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct
files_struct *fsp)
* Todo: use SMB_VFS_STATX() once that's available.
*/
- ret = SMB_VFS_LSTAT(conn, smb_dname_full);
- if (ret != 0) {
+ retval = SMB_VFS_LSTAT(conn, smb_dname_full);
+ if (retval != 0) {
goto err_break;
}
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index 4db267c92b0..65fa17523d8 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -120,6 +120,7 @@ bool run_smb2_sacl(int dummy);
bool run_smb2_quota1(int dummy);
bool run_smb2_stream_acl(int dummy);
bool run_list_dir_async_test(int dummy);
+bool run_delete_on_close_non_empty(int dummy);
bool run_chain3(int dummy);
bool run_local_conv_auth_info(int dummy);
bool run_local_sprintf_append(int dummy);
diff --git a/source3/torture/test_smb2.c b/source3/torture/test_smb2.c
index b186ea4edac..0fac5125c08 100644
--- a/source3/torture/test_smb2.c
+++ b/source3/torture/test_smb2.c
@@ -3228,3 +3228,139 @@ bool run_list_dir_async_test(int dummy)
(void)cli_rmdir(cli, dname);
return ret;
}
+
+/*
+ * Test delete a directory fails if a file is created
+ * in a directory after the delete on close is set.
+ * BUG: https://bugzilla.samba.org/show_bug.cgi?id=14892
+ */
+
+bool run_delete_on_close_non_empty(int dummy)
+{
+ struct cli_state *cli = NULL;
+ NTSTATUS status;
+ const char *dname = "DEL_ON_CLOSE_DIR";
+ const char *fname = "DEL_ON_CLOSE_DIR\\testfile";
+ uint16_t fnum = (uint16_t)-1;
+ uint16_t fnum1 = (uint16_t)-1;
+ bool ret = false;
+
+ printf("SMB2 delete on close nonempty\n");
+
+ if (!torture_init_connection(&cli)) {
+ return false;
+ }
+
+ status = smbXcli_negprot(cli->conn,
+ cli->timeout,
+ PROTOCOL_SMB2_02,
+ PROTOCOL_SMB3_11);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("smbXcli_negprot returned %s\n", nt_errstr(status));
+ return false;
+ }
+
+ status = cli_session_setup_creds(cli, torture_creds);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_session_setup returned %s\n", nt_errstr(status));
+ return false;
+ }
+
+ status = cli_tree_connect(cli, share, "?????", NULL);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_tree_connect returned %s\n", nt_errstr(status));
+ return false;
+ }
+
+ /* Ensure directory doesn't exist. */
+ (void)cli_unlink(cli,
+ fname,
+ FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
+ (void)cli_rmdir(cli, dname);
+
+ /* Create target directory. */
+ status = cli_ntcreate(cli,
+ dname,
+ 0,
+ DELETE_ACCESS|FILE_READ_DATA,
+ FILE_ATTRIBUTE_DIRECTORY,
+ FILE_SHARE_READ|
+ FILE_SHARE_WRITE|
+ FILE_SHARE_DELETE,
+ FILE_CREATE,
+ FILE_DIRECTORY_FILE,
+ 0,
+ &fnum,
+ NULL);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_ntcreate for directory %s returned %s\n",
+ dname,
+ nt_errstr(status));
+ goto out;
+ }
+
+ /* Now set the delete on close bit. */
+ status = cli_nt_delete_on_close(cli, fnum, 1);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_cli_nt_delete_on_close set for directory "
+ "%s returned %s\n",
+ dname,
+ nt_errstr(status));
+ goto out;
+ }
+
+ /* Create file inside target directory. */
+ /*
+ * NB. On Windows this will return NT_STATUS_DELETE_PENDING. Only on
+ * Samba will this succeed by default (the option "check parent
+ * directory delete on close" configures behaviour), but we're using
+ * this to test a race condition.
+ */
+ status = cli_ntcreate(cli,
+ fname,
+ 0,
+ FILE_READ_DATA,
+ FILE_ATTRIBUTE_NORMAL,
+ FILE_SHARE_READ|
+ FILE_SHARE_WRITE|
+ FILE_SHARE_DELETE,
+ FILE_CREATE,
+ 0,
+ 0,
+ &fnum1,
+ NULL);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_ntcreate for file %s returned %s\n",
+ fname,
+ nt_errstr(status));
+ goto out;
+ }
+ cli_close(cli, fnum1);
+ fnum1 = (uint16_t)-1;
+
+ /* Now the close should fail. */
+ status = cli_close(cli, fnum);
+ if (!NT_STATUS_EQUAL(status, NT_STATUS_DIRECTORY_NOT_EMPTY)) {
+ printf("cli_close for directory %s returned %s\n",
+ dname,
+ nt_errstr(status));
+ goto out;
+ }
+
+ ret = true;
+
+ out:
+
+ if (fnum1 != (uint16_t)-1) {
+ cli_close(cli, fnum1);
+ }
+ if (fnum != (uint16_t)-1) {
+ cli_nt_delete_on_close(cli, fnum, 0);
+ cli_close(cli, fnum);
+ }
+ (void)cli_unlink(cli,
+ fname,
+ FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
+ (void)cli_rmdir(cli, dname);
+ return ret;
+}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 79a9c65073c..197e1990e16 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -15249,6 +15249,10 @@ static struct {
.name = "SMB2-LIST-DIR-ASYNC",
.fn = run_list_dir_async_test,
},
+ {
+ .name = "SMB2-DEL-ON-CLOSE-NONEMPTY",
+ .fn = run_delete_on_close_non_empty,
+ },
{
.name = "CLEANUP1",
.fn = run_cleanup1,
--
Samba Shared Repository
