From 2dd11d418a8e02a26d99103a648e5c89f22f6741 Mon Sep 17 00:00:00 2001 From: Anoop C S Date: Thu, 9 Aug 2018 12:28:41 +0530 Subject: [PATCH 1/3] s3/libsmb: Explicitly set delete_on_close token for rmdir The current implementation of `rmdir` hopes to get the directory deleted on closing last open handle when FILE_DELETE_ON_CLOSE is set on it. But for non-empty directories Windows doesn't error out during an open call. Following that we internally refuse to set initial delete_on_close while opening a non-empty directory. This prevents us from trying to delete the directory when last open handle is closed. Instead of relying on FILE_DELETE_ON_CLOSE during an open we explicitly set delete_on_close token on directory handle once it is available. This ensures that NT_STATUS_DIRECTORY_NOT_EMPTY is returned for `rmdir` on non-empty directories while closing open directory handle. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13204 Signed-off-by: Anoop C S Reviewed-by: Jeremy Allison Reviewed-by: Andreas Schneider (cherry picked from commit 6b68e3eca631c04d6d57c489daf60f64732fc86d) --- source3/libsmb/cli_smb2_fnum.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c index c397b29b381..7b2f39c02e8 100644 --- a/source3/libsmb/cli_smb2_fnum.c +++ b/source3/libsmb/cli_smb2_fnum.c @@ -682,7 +682,7 @@ NTSTATUS cli_smb2_rmdir(struct cli_state *cli, const char *dname) FILE_ATTRIBUTE_DIRECTORY, /* file attributes */ FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access */ FILE_OPEN, /* create_disposition */ - FILE_DIRECTORY_FILE|FILE_DELETE_ON_CLOSE, /* create_options */ + FILE_DIRECTORY_FILE, /* create_options */ &fnum, NULL); @@ -710,6 +710,13 @@ NTSTATUS cli_smb2_rmdir(struct cli_state *cli, const char *dname) if (!NT_STATUS_IS_OK(status)) { return status; } + + status = cli_smb2_delete_on_close(cli, fnum, true); + if (!NT_STATUS_IS_OK(status)) { + cli_smb2_close_fnum(cli, fnum); + return status; + } + return cli_smb2_close_fnum(cli, fnum); } -- 2.17.1 From dc93568d0e162d87fe453413aac5dc5967cd5138 Mon Sep 17 00:00:00 2001 From: Anoop C S Date: Thu, 9 Aug 2018 20:02:05 +0530 Subject: [PATCH 2/3] s4/torture: Add new test for DELETE_ON_CLOSE on non-empty directories BUG: https://bugzilla.samba.org/show_bug.cgi?id=13204 Signed-off-by: Anoop C S Reviewed-by: Jeremy Allison Reviewed-by: Andreas Schneider (cherry picked from commit 6a7f11746c9cc3cdc5307e540bdd1f3f10fed05b) --- source4/torture/basic/delete.c | 87 ++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/source4/torture/basic/delete.c b/source4/torture/basic/delete.c index 45c530288c9..a59f492e36a 100644 --- a/source4/torture/basic/delete.c +++ b/source4/torture/basic/delete.c @@ -2101,6 +2101,92 @@ static bool deltest20b(struct torture_context *tctx, struct smbcli_state *cli1, return correct; } +/* Test 20c */ +/* Along the lines of deltest20 we try to open a non-empty directory with delete + * on close set and subsequent close to verify its presence in the tree. + */ +static bool deltest20c(struct torture_context *tctx, struct smbcli_state *cli1, struct smbcli_state *cli2) +{ + int fnum1 = -1; + int dnum1 = -1; + int ret; + char *fullname; + + del_clean_area(cli1, cli2); + + smbcli_deltree(cli1->tree, dname); + + /* Firstly open and create with all access */ + dnum1 = smbcli_nt_create_full(cli1->tree, dname, 0, + SEC_FILE_READ_DATA| + SEC_FILE_WRITE_DATA| + SEC_STD_DELETE, + FILE_ATTRIBUTE_DIRECTORY, + NTCREATEX_SHARE_ACCESS_READ| + NTCREATEX_SHARE_ACCESS_WRITE| + NTCREATEX_SHARE_ACCESS_DELETE, + NTCREATEX_DISP_CREATE, + NTCREATEX_OPTIONS_DIRECTORY, 0); + torture_assert(tctx, dnum1 != -1, talloc_asprintf(tctx, "open of %s failed: %s", + dname, smbcli_errstr(cli1->tree))); + + /* And close - just to create the directory */ + smbcli_close(cli1->tree, dnum1); + + ret = asprintf(&fullname, "\\%s%s", dname, fname); + torture_assert(tctx, ret != -1, "asprintf failed"); + + /* Open and create with all access */ + fnum1 = smbcli_nt_create_full(cli1->tree, fullname, 0, + SEC_RIGHTS_FILE_ALL, + FILE_ATTRIBUTE_NORMAL, + NTCREATEX_SHARE_ACCESS_READ| + NTCREATEX_SHARE_ACCESS_WRITE| + NTCREATEX_SHARE_ACCESS_DELETE, + NTCREATEX_DISP_CREATE, + 0, 0); + torture_assert(tctx, fnum1 != -1, talloc_asprintf(tctx, "open of %s failed: %s", + fname, smbcli_errstr(cli1->tree))); + + /* And close - just to create the file. */ + smbcli_close(cli1->tree, fnum1); + + /* Open with all access, but add delete on close */ + dnum1 = smbcli_nt_create_full(cli1->tree, dname, 0, + SEC_FILE_READ_DATA| + SEC_FILE_WRITE_DATA| + SEC_STD_DELETE, + FILE_ATTRIBUTE_DIRECTORY, + NTCREATEX_SHARE_ACCESS_READ| + NTCREATEX_SHARE_ACCESS_WRITE| + NTCREATEX_SHARE_ACCESS_DELETE, + NTCREATEX_DISP_OPEN, + NTCREATEX_OPTIONS_DIRECTORY|NTCREATEX_OPTIONS_DELETE_ON_CLOSE, 0); + /* Should work */ + torture_assert(tctx, dnum1 != -1, talloc_asprintf(tctx, "open of %s failed: %s", + dname, smbcli_errstr(cli1->tree))); + + smbcli_close(cli1->tree, dnum1); + + /* Try to open again */ + dnum1 = smbcli_nt_create_full(cli1->tree, dname, 0, + SEC_FILE_READ_DATA| + SEC_FILE_WRITE_DATA| + SEC_STD_DELETE, + FILE_ATTRIBUTE_DIRECTORY, + NTCREATEX_SHARE_ACCESS_READ| + NTCREATEX_SHARE_ACCESS_WRITE| + NTCREATEX_SHARE_ACCESS_DELETE, + NTCREATEX_DISP_OPEN, + NTCREATEX_OPTIONS_DIRECTORY, 0); + /* Directory should be still present*/ + torture_assert(tctx, dnum1 != -1, talloc_asprintf(tctx, "open of %s failed: %s", + dname, smbcli_errstr(cli1->tree))); + + smbcli_close(cli1->tree, dnum1); + + return true; +} /* Test 21 ... */ static bool deltest21(struct torture_context *tctx) @@ -2526,6 +2612,7 @@ struct torture_suite *torture_test_delete(TALLOC_CTX *ctx) torture_suite_add_2smb_test(suite, "deltest20", deltest20); torture_suite_add_2smb_test(suite, "deltest20a", deltest20a); torture_suite_add_2smb_test(suite, "deltest20b", deltest20b); + torture_suite_add_2smb_test(suite, "deltest20c", deltest20c); torture_suite_add_simple_test(suite, "deltest21", deltest21); torture_suite_add_simple_test(suite, "deltest22", deltest22); torture_suite_add_2smb_test(suite, "deltest23", deltest23); -- 2.17.1 From 2951f0dc9059215841dcb65534b1b8c52c769773 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Aug 2018 10:02:26 -0700 Subject: [PATCH 3/3] s3: tests: smbclient. Regression test to ensure we get NT_STATUS_DIRECTORY_NOT_EMPTY on rmdir. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13204 Signed-off-by: Jeremy Allison Reviewed-by: Andreas Schneider Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Fri Aug 10 21:08:14 CEST 2018 on sn-devel-144 (cherry picked from commit bca400847f2fcc3dd1398e166c1964cb88822071) --- source3/script/tests/test_smbclient_s3.sh | 42 +++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh index 706023b7d19..77cb728a25a 100755 --- a/source3/script/tests/test_smbclient_s3.sh +++ b/source3/script/tests/test_smbclient_s3.sh @@ -1669,6 +1669,44 @@ EOF fi } +# Test smbclient non-empty rmdir command +test_del_nedir() +{ + tmpfile=$PREFIX/smbclient_interactive_prompt_commands + del_nedir="$LOCAL_PATH/del_nedir" + + rm -rf $del_nedir + mkdir $del_nedir + touch $del_nedir/afile + cat > $tmpfile <