The Samba-Bugzilla – Attachment 16951 Details for
Bug 14892
SIGSEGV in rmdir_internals/synthetic_pathref - dirfsp is used uninitialized in rmdir_internals().
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am fix for 4.15.next
bug-14892-4.15.patch (text/plain), 11.24 KB, created by
Jeremy Allison
on 2021-11-04 16:46:53 UTC
(
hide
)
Description:
git-am fix for 4.15.next
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2021-11-04 16:46:53 UTC
Size:
11.24 KB
patch
obsolete
>From f4622de3a6021f970054f58e310dabab600872a8 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 2 Nov 2021 10:44:44 -0700 >Subject: [PATCH 1/3] s3: smbd: dirfsp is being used uninitialized inside > rmdir_internals(). >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >Not caught be the tests in bugs 14878, 14879 as can_delete_directory_fsp() >doesn't have the same bug. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14892 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> > >Autobuild-User(master): Ralph Böhme <slow@samba.org> >Autobuild-Date(master): Wed Nov 3 14:33:49 UTC 2021 on sn-devel-184 > >(cherry picked from commit bbdcd66c048fee39629aeff450b50d049806e2f7) >--- > source3/smbd/close.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > >diff --git a/source3/smbd/close.c b/source3/smbd/close.c >index 7178257efcc..89e18d979ed 100644 >--- a/source3/smbd/close.c >+++ b/source3/smbd/close.c >@@ -1055,6 +1055,8 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) > goto err; > } > >+ dirfsp = dir_hnd_fetch_fsp(dir_hnd); >+ > while ((dname = ReadDirName(dir_hnd, &dirpos, &st, &talloced)) != NULL) { > struct smb_filename *smb_dname_full = NULL; > struct smb_filename *direntry_fname = NULL; >@@ -1203,7 +1205,6 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp) > > /* Do a recursive delete. */ > RewindDir(dir_hnd,&dirpos); >- dirfsp = dir_hnd_fetch_fsp(dir_hnd); > > while ((dname = ReadDirName(dir_hnd, &dirpos, &st, &talloced)) != NULL) { > struct smb_filename *direntry_fname = NULL; >-- >2.30.2 > > >From 4ca5e2e900aa7e38c3678d2026df7cfbabe0c702 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 3 Nov 2021 16:50:10 -0700 >Subject: [PATCH 2/3] 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@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit adfad6390962022277cc6aacaa388af86e46b71c) >--- > selftest/knownfail.d/del_on_close_nonempty | 1 + > source3/selftest/tests.py | 15 +++ > source3/torture/proto.h | 1 + > source3/torture/test_smb2.c | 136 +++++++++++++++++++++ > source3/torture/torture.c | 4 + > 5 files changed, 157 insertions(+) > create mode 100644 selftest/knownfail.d/del_on_close_nonempty > >diff --git a/selftest/knownfail.d/del_on_close_nonempty b/selftest/knownfail.d/del_on_close_nonempty >new file mode 100644 >index 00000000000..7109b993f91 >--- /dev/null >+++ b/selftest/knownfail.d/del_on_close_nonempty >@@ -0,0 +1 @@ >+^samba3.smbtorture_s3.plain.SMB2-DEL-ON-CLOSE-NONEMPTY.smbtorture\(fileserver\) >diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py >index e39009635a6..60b3f18eded 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/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, >-- >2.30.2 > > >From b4b4675beef95ec57b28f62fc9de9b084d2bf5fb Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 3 Nov 2021 19:02:36 -0700 >Subject: [PATCH 3/3] s3: smbd: Ensure in the directory scanning loops inside > rmdir_internals() we don't overwrite the 'ret' variable. >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >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@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> > >Autobuild-User(master): Ralph Böhme <slow@samba.org> >Autobuild-Date(master): Thu Nov 4 09:10:27 UTC 2021 on sn-devel-184 > >(cherry picked from commit 141f3f5f9a5ef556cc7864b2afbf8ad48b7ebe77) >--- > selftest/knownfail.d/del_on_close_nonempty | 1 - > source3/smbd/close.c | 13 +++++++------ > 2 files changed, 7 insertions(+), 7 deletions(-) > delete mode 100644 selftest/knownfail.d/del_on_close_nonempty > >diff --git a/selftest/knownfail.d/del_on_close_nonempty b/selftest/knownfail.d/del_on_close_nonempty >deleted file mode 100644 >index 7109b993f91..00000000000 >--- a/selftest/knownfail.d/del_on_close_nonempty >+++ /dev/null >@@ -1 +0,0 @@ >-^samba3.smbtorture_s3.plain.SMB2-DEL-ON-CLOSE-NONEMPTY.smbtorture\(fileserver\) >diff --git a/source3/smbd/close.c b/source3/smbd/close.c >index 89e18d979ed..0e5f1958fa1 100644 >--- a/source3/smbd/close.c >+++ b/source3/smbd/close.c >@@ -1061,6 +1061,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); >@@ -1095,8 +1096,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); >@@ -1139,8 +1140,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" >@@ -1243,8 +1244,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; > } > >-- >2.30.2 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
slow
:
review+
Actions:
View
Attachments on
bug 14892
:
16921
|
16941
| 16951