[PATCH]: s3: smbd: Fix delete-on-close after smb2_find
Jeremy Allison
jra at samba.org
Fri Nov 3 15:39:35 UTC 2017
On Fri, Nov 03, 2017 at 04:10:23PM +0100, Ralph Wuerthner via samba-technical wrote:
> Hi!
>
> On a customer system I came recently across the following Samba panic:
>
> [2017/11/03 14:27:35.930242, 0, pid=446, effective(12000500,
> 12000513), real(12000500, 0)]
> ../source3/lib/util.c:791(smb_panic_s3)
> PANIC (pid 446): assert failed: dirp->fsp->dptr->dir_hnd == dirp
> [2017/11/03 14:27:35.930722, 0, pid=446, effective(12000500,
> 12000513), real(12000500, 0)]
> ../source3/lib/util.c:902(log_stack_trace)
> BACKTRACE: 27 stack frames:
> #0 /usr/lpp/mmfs/lib64/libsmbconf.so.0(log_stack_trace+0x1a)
> [0x7f3a05013e7a]
> #1 /usr/lpp/mmfs/lib64/libsmbconf.so.0(smb_panic_s3+0x20)
> [0x7f3a05013f50]
> #2 /usr/lpp/mmfs/lib64/libsamba-util.so.0(smb_panic+0x2f)
> [0x7f3a07919bdf]
> #3 /usr/lpp/mmfs/lib64/samba/libsmbd-base-samba4.so(+0xbb127)
> [0x7f3a07461127]
> #4 /usr/lpp/mmfs/lib64/samba/libtalloc.so.2(_talloc_free+0x440)
> [0x7f3a06d8fed0]
> #5 /usr/lpp/mmfs/lib64/samba/libsmbd-base-samba4.so(can_delete_directory_fsp+0x137)
> [0x7f3a07464357]
> #6 /usr/lpp/mmfs/lib64/samba/libsmbd-base-samba4.so(can_set_delete_on_close+0x168)
> [0x7f3a074e85b8]
> #7 /usr/lpp/mmfs/lib64/samba/libsmbd-base-samba4.so(+0xf4b73)
> [0x7f3a0749ab73]
> #8 /usr/lpp/mmfs/lib64/samba/libsmbd-base-samba4.so(smbd_do_setfilepathinfo+0x169b)
> [0x7f3a074ab0ab]
> #9 /usr/lpp/mmfs/lib64/samba/libsmbd-base-samba4.so(smbd_smb2_request_process_setinfo+0x630)
> [0x7f3a07504c30]
> #10 /usr/lpp/mmfs/lib64/samba/libsmbd-base-samba4.so(smbd_smb2_request_dispatch+0xcb5)
> [0x7f3a074ecb05]
> #11 /usr/lpp/mmfs/lib64/samba/libsmbd-base-samba4.so(+0x148f22)
> [0x7f3a074eef22]
> #12 /usr/lpp/mmfs/lib64/samba/libtevent.so.0(+0x9c9b) [0x7f3a06984c9b]
> #13 /usr/lpp/mmfs/lib64/samba/libtevent.so.0(+0x8167) [0x7f3a06983167]
> #14
> /usr/lpp/mmfs/lib64/samba/libtevent.so.0(_tevent_loop_once+0x8d)
> [0x7f3a0697f31d]
> #15 /usr/lpp/mmfs/lib64/samba/libtevent.so.0(tevent_common_loop_wait+0x1b)
> [0x7f3a0697f4bb]
> #16 /usr/lpp/mmfs/lib64/samba/libtevent.so.0(+0x8107) [0x7f3a06983107]
> #17
> /usr/lpp/mmfs/lib64/samba/libsmbd-base-samba4.so(smbd_process+0x721)
> [0x7f3a074db991]
> #18 /usr/lpp/mmfs/bin/smbd(+0xb588) [0x7f3a07fbc588]
> #19 /usr/lpp/mmfs/lib64/samba/libtevent.so.0(+0x9c9b) [0x7f3a06984c9b]
> #20 /usr/lpp/mmfs/lib64/samba/libtevent.so.0(+0x8167) [0x7f3a06983167]
> #21
> /usr/lpp/mmfs/lib64/samba/libtevent.so.0(_tevent_loop_once+0x8d)
> [0x7f3a0697f31d]
> #22 /usr/lpp/mmfs/lib64/samba/libtevent.so.0(tevent_common_loop_wait+0x1b)
> [0x7f3a0697f4bb]
> #23 /usr/lpp/mmfs/lib64/samba/libtevent.so.0(+0x8107) [0x7f3a06983107]
> #24 /usr/lpp/mmfs/bin/smbd(main+0x1580) [0x7f3a07fb8fc0]
> #25 /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f3a03accb35]
> #26 /usr/lpp/mmfs/bin/smbd(+0x82c9) [0x7f3a07fb92c9]
>
> I was able to recreate the panic on my test system with the attached
> 'smb2.delete-on-close-perms.FIND and set DOC.FIND and set DOC'
> smbtorture test. Please see also my proposed fix. Both patches apply
> on master.
Thanks *VERY* much for the test.
I'm trying to avoid adding calls that use by-pathname semantics,
so I'll take a look at the patch to see if we can fix it another
way first.
Thanks !
Jeremy.
>
> Ralph Wuerthner
>
> From 634694af4c2c42206adaf49e8231f6555a1de4eb Mon Sep 17 00:00:00 2001
> From: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
> Date: Wed, 1 Nov 2017 14:13:25 +0100
> Subject: [PATCH 1/2] s3: smbd: Fix delete-on-close after smb2_find
>
> Both dptr_create() and can_delete_directory_fsp() are calling OpenDir_fsp()
> to get a directory handle. This is causing an issue when delete-on-close is
> set after smb2_find because both directory handle instances share the same
> underlying file descriptor. In addition the SMB_ASSERT() in destructor
> smb_Dir_destructor() gets triggered.
> To avoid this use OpenDir() instead of OpenDir_fsp().
>
> Signed-off-by: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
> ---
> source3/smbd/dir.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
> index 8d83fba..8e591ed 100644
> --- a/source3/smbd/dir.c
> +++ b/source3/smbd/dir.c
> @@ -2132,9 +2132,9 @@ NTSTATUS can_delete_directory_fsp(files_struct *fsp)
> char *talloced = NULL;
> SMB_STRUCT_STAT st;
> struct connection_struct *conn = fsp->conn;
> - struct smb_Dir *dir_hnd = OpenDir_fsp(talloc_tos(),
> + struct smb_Dir *dir_hnd = OpenDir(talloc_tos(),
> conn,
> - fsp,
> + fsp->fsp_name,
> NULL,
> 0);
>
> --
> 2.7.4
>
>
> From 8399e0abd3cf8482fd7168ed017d03b9dff4e437 Mon Sep 17 00:00:00 2001
> From: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
> Date: Fri, 27 Oct 2017 14:59:32 +0200
> Subject: [PATCH 2/2] Add FIND and set DOC test case.
>
> Signed-off-by: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
> ---
> source4/torture/smb2/delete-on-close.c | 67 ++++++++++++++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
>
> diff --git a/source4/torture/smb2/delete-on-close.c b/source4/torture/smb2/delete-on-close.c
> index 44ef33e..b7c41e9 100644
> --- a/source4/torture/smb2/delete-on-close.c
> +++ b/source4/torture/smb2/delete-on-close.c
> @@ -516,6 +516,72 @@ static bool test_doc_create_if_exist(struct torture_context *tctx, struct smb2_t
> return true;
> }
>
> +static bool test_doc_find_and_set_doc(struct torture_context *tctx, struct smb2_tree *tree)
> +{
> + struct smb2_create io;
> + struct smb2_find find;
> + NTSTATUS status;
> + union smb_search_data *d;
> + union smb_setfileinfo sfinfo;
> + unsigned int count;
> + uint32_t perms = 0;
> + int i;
> +
> + perms = SEC_STD_SYNCHRONIZE | SEC_STD_READ_CONTROL | SEC_STD_DELETE |
> + SEC_DIR_WRITE_ATTRIBUTE | SEC_DIR_READ_ATTRIBUTE |
> + SEC_DIR_WRITE_EA | SEC_FILE_APPEND_DATA |
> + SEC_FILE_WRITE_DATA | SEC_DIR_LIST;
> +
> + /* File should not exist for this first test, so make sure */
> + set_dir_delete_perms(tctx, tree);
> +
> + smb2_deltree(tree, DNAME);
> +
> + create_dir(tctx, tree);
> +
> + torture_comment(tctx, "FIND and delete directory\n");
> + torture_comment(tctx, "We expect NT_STATUS_OK\n");
> +
> + /* open the directory first */
> + ZERO_STRUCT(io);
> + io.in.desired_access = perms;
> + io.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY;
> + io.in.create_disposition = NTCREATEX_DISP_OPEN_IF;
> + io.in.share_access = NTCREATEX_SHARE_ACCESS_READ |
> + NTCREATEX_SHARE_ACCESS_DELETE;
> + io.in.create_options = NTCREATEX_OPTIONS_DIRECTORY;
> + io.in.fname = DNAME;
> +
> + status = smb2_create(tree, tctx, &io);
> + CHECK_STATUS(status, NT_STATUS_OK);
> +
> + /* list directory */
> + ZERO_STRUCT(find);
> + find.in.file.handle = io.out.file.handle;
> + find.in.pattern = "*";
> + find.in.continue_flags = SMB2_CONTINUE_FLAG_SINGLE;
> + find.in.max_response_size = 0x100;
> + find.in.level = SMB2_FIND_BOTH_DIRECTORY_INFO;
> +
> + /* start enumeration on directory */
> + status = smb2_find_level(tree, tree, &find, &count, &d);
> + CHECK_STATUS(status, NT_STATUS_OK);
> +
> + /* set delete-on-close */
> + ZERO_STRUCT(sfinfo);
> + sfinfo.generic.level = RAW_SFILEINFO_DISPOSITION_INFORMATION;
> + sfinfo.disposition_info.in.delete_on_close = 1;
> + sfinfo.generic.in.file.handle = io.out.file.handle;
> + status = smb2_setinfo_file(tree, &sfinfo);
> + CHECK_STATUS(status, NT_STATUS_OK);
> +
> + /* close directory */
> + status = smb2_util_close(tree, io.out.file.handle);
> + CHECK_STATUS(status, NT_STATUS_OK);
> + return true;
> +}
> +
> +
> /*
> * Extreme testing of Delete On Close and permissions
> */
> @@ -529,6 +595,7 @@ struct torture_suite *torture_smb2_doc_init(TALLOC_CTX *ctx)
> torture_suite_add_1smb2_test(suite, "CREATE Existing", test_doc_create_exist);
> 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);
>
> suite->description = talloc_strdup(suite, "SMB2-Delete-on-Close-Perms tests");
>
> --
> 2.7.4
>
>
More information about the samba-technical
mailing list