[SCM] Samba Shared Repository - branch master updated

Ralph Böhme slow at samba.org
Thu Nov 4 09:11:02 UTC 2021


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 <jra at 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 <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    
    Autobuild-User(master): Ralph Böhme <slow at samba.org>
    Autobuild-Date(master): Thu Nov  4 09:10:27 UTC 2021 on sn-devel-184

commit adfad6390962022277cc6aacaa388af86e46b71c
Author: Jeremy Allison <jra at 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 <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at 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



More information about the samba-cvs mailing list