From 7a7bc6481e16692ca1384136c8fdb1f39c80571d Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2015 11:36:30 +0200 Subject: [PATCH 1/5] smbd: Introduce reset_delete_on_close_lck Boolean flags passed down make things more complex than necessary... BUG: https://bugzilla.samba.org/show_bug.cgi?id=11257 Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison (cherry picked from commit d75a0a589f477e4541badfc1a6ba939e281a5582) --- source3/locking/locking.c | 61 +++++++++++++++++++++++++++-------------------- source3/locking/proto.h | 2 ++ 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/source3/locking/locking.c b/source3/locking/locking.c index 221d6ee..cff9025 100644 --- a/source3/locking/locking.c +++ b/source3/locking/locking.c @@ -1081,6 +1081,27 @@ static bool add_delete_on_close_token(struct share_mode_data *d, return true; } +void reset_delete_on_close_lck(files_struct *fsp, + struct share_mode_lock *lck) +{ + struct share_mode_data *d = lck->data; + uint32_t i; + + for (i=0; inum_delete_tokens; i++) { + struct delete_token *dt = &d->delete_tokens[i]; + + if (dt->name_hash == fsp->name_hash) { + d->modified = true; + + /* Delete this entry. */ + TALLOC_FREE(dt->delete_nt_token); + TALLOC_FREE(dt->delete_token); + *dt = d->delete_tokens[d->num_delete_tokens-1]; + d->num_delete_tokens -= 1; + } + } +} + /**************************************************************************** Sets the delete on close flag over all share modes on this file. Modify the share mode entry for all files open @@ -1102,44 +1123,32 @@ void set_delete_on_close_lck(files_struct *fsp, int i; bool ret; - if (delete_on_close) { - SMB_ASSERT(nt_tok != NULL); - SMB_ASSERT(tok != NULL); - } else { + if (!delete_on_close) { SMB_ASSERT(nt_tok == NULL); SMB_ASSERT(tok == NULL); + return reset_delete_on_close_lck(fsp, lck); } + SMB_ASSERT(nt_tok != NULL); + SMB_ASSERT(tok != NULL); + for (i=0; inum_delete_tokens; i++) { struct delete_token *dt = &d->delete_tokens[i]; if (dt->name_hash == fsp->name_hash) { d->modified = true; - if (delete_on_close == false) { - /* Delete this entry. */ - TALLOC_FREE(dt->delete_nt_token); - TALLOC_FREE(dt->delete_token); - *dt = d->delete_tokens[ - d->num_delete_tokens-1]; - d->num_delete_tokens -= 1; - } else { - /* Replace this token with the - given tok. */ - TALLOC_FREE(dt->delete_nt_token); - dt->delete_nt_token = dup_nt_token(dt, nt_tok); - SMB_ASSERT(dt->delete_nt_token != NULL); - TALLOC_FREE(dt->delete_token); - dt->delete_token = copy_unix_token(dt, tok); - SMB_ASSERT(dt->delete_token != NULL); - } + + /* Replace this token with the given tok. */ + TALLOC_FREE(dt->delete_nt_token); + dt->delete_nt_token = dup_nt_token(dt, nt_tok); + SMB_ASSERT(dt->delete_nt_token != NULL); + TALLOC_FREE(dt->delete_token); + dt->delete_token = copy_unix_token(dt, tok); + SMB_ASSERT(dt->delete_token != NULL); + return; } } - if (!delete_on_close) { - /* Nothing to delete - not found. */ - return; - } - ret = add_delete_on_close_token(lck->data, fsp->name_hash, nt_tok, tok); SMB_ASSERT(ret); } diff --git a/source3/locking/proto.h b/source3/locking/proto.h index c4ea198..8a0e023 100644 --- a/source3/locking/proto.h +++ b/source3/locking/proto.h @@ -185,6 +185,8 @@ bool get_delete_on_close_token(struct share_mode_lock *lck, uint32_t name_hash, const struct security_token **pp_nt_tok, const struct security_unix_token **pp_tok); +void reset_delete_on_close_lck(files_struct *fsp, + struct share_mode_lock *lck); void set_delete_on_close_lck(files_struct *fsp, struct share_mode_lock *lck, bool delete_on_close, -- 1.9.1 From 09334e5f8894d9972e8919826b7870cc012efe45 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2015 11:38:32 +0200 Subject: [PATCH 2/5] smbd: Use reset_delete_on_close_lck directly BUG: https://bugzilla.samba.org/show_bug.cgi?id=11257 Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison (cherry picked from commit 1f3735a28c3e6cbe2ca6c8e3bf312b3ea8c1754e) --- source3/locking/locking.c | 4 +--- source3/smbd/close.c | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/source3/locking/locking.c b/source3/locking/locking.c index cff9025..e9c3039 100644 --- a/source3/locking/locking.c +++ b/source3/locking/locking.c @@ -1174,9 +1174,7 @@ bool set_delete_on_close(files_struct *fsp, bool delete_on_close, nt_tok, tok); } else { - set_delete_on_close_lck(fsp, lck, false, - NULL, - NULL); + reset_delete_on_close_lck(fsp, lck); } if (fsp->is_directory) { diff --git a/source3/smbd/close.c b/source3/smbd/close.c index fc1d380..3f02779 100644 --- a/source3/smbd/close.c +++ b/source3/smbd/close.c @@ -461,7 +461,7 @@ static NTSTATUS close_remove_share_mode(files_struct *fsp, */ fsp->delete_on_close = false; - set_delete_on_close_lck(fsp, lck, false, NULL, NULL); + reset_delete_on_close_lck(fsp, lck); done: -- 1.9.1 From 62c7813e1016a3ac4ac8c41659e8feeb1f8eabc6 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2015 11:41:45 +0200 Subject: [PATCH 3/5] smbd: Remove bool arg from set_delete_on_close_lck We now have reset_delete_on_close_lck, this was called with "true" everywhere now. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11257 Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison (cherry picked from commit b0d4a7daa149cfc9ef697dd7fae4524a35078126) --- source3/locking/locking.c | 11 +---------- source3/locking/proto.h | 1 - source3/smbd/close.c | 4 ++-- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/source3/locking/locking.c b/source3/locking/locking.c index e9c3039..bcc9bfe 100644 --- a/source3/locking/locking.c +++ b/source3/locking/locking.c @@ -1115,7 +1115,6 @@ void reset_delete_on_close_lck(files_struct *fsp, void set_delete_on_close_lck(files_struct *fsp, struct share_mode_lock *lck, - bool delete_on_close, const struct security_token *nt_tok, const struct security_unix_token *tok) { @@ -1123,12 +1122,6 @@ void set_delete_on_close_lck(files_struct *fsp, int i; bool ret; - if (!delete_on_close) { - SMB_ASSERT(nt_tok == NULL); - SMB_ASSERT(tok == NULL); - return reset_delete_on_close_lck(fsp, lck); - } - SMB_ASSERT(nt_tok != NULL); SMB_ASSERT(tok != NULL); @@ -1170,9 +1163,7 @@ bool set_delete_on_close(files_struct *fsp, bool delete_on_close, } if (delete_on_close) { - set_delete_on_close_lck(fsp, lck, true, - nt_tok, - tok); + set_delete_on_close_lck(fsp, lck, nt_tok, tok); } else { reset_delete_on_close_lck(fsp, lck); } diff --git a/source3/locking/proto.h b/source3/locking/proto.h index 8a0e023..75faa94 100644 --- a/source3/locking/proto.h +++ b/source3/locking/proto.h @@ -189,7 +189,6 @@ void reset_delete_on_close_lck(files_struct *fsp, struct share_mode_lock *lck); void set_delete_on_close_lck(files_struct *fsp, struct share_mode_lock *lck, - bool delete_on_close, const struct security_token *nt_tok, const struct security_unix_token *tok); bool set_delete_on_close(files_struct *fsp, bool delete_on_close, diff --git a/source3/smbd/close.c b/source3/smbd/close.c index 3f02779..0e75bf0 100644 --- a/source3/smbd/close.c +++ b/source3/smbd/close.c @@ -303,7 +303,7 @@ static NTSTATUS close_remove_share_mode(files_struct *fsp, became_user = True; } fsp->delete_on_close = true; - set_delete_on_close_lck(fsp, lck, True, + set_delete_on_close_lck(fsp, lck, get_current_nttok(conn), get_current_utok(conn)); if (became_user) { @@ -1083,7 +1083,7 @@ static NTSTATUS close_directory(struct smb_request *req, files_struct *fsp, } send_stat_cache_delete_message(fsp->conn->sconn->msg_ctx, fsp->fsp_name->base_name); - set_delete_on_close_lck(fsp, lck, true, + set_delete_on_close_lck(fsp, lck, get_current_nttok(fsp->conn), get_current_utok(fsp->conn)); fsp->delete_on_close = true; -- 1.9.1 From 96d6090a651a15fd13b3ea326957b59ea5d6e7d9 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 21 Apr 2015 10:16:16 +0200 Subject: [PATCH 4/5] smbd: Cancel pending notifies if the directory goes away BUG: https://bugzilla.samba.org/show_bug.cgi?id=11257 Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison (cherry picked from commit 50a1247927cb68303701a11517811deda10364f7) --- source3/librpc/idl/messaging.idl | 3 +++ source3/locking/locking.c | 31 ++++++++++++++++++++++++++++- source3/smbd/notify.c | 43 ++++++++++++++++++++++++++++++++++++++++ source3/smbd/proto.h | 3 +++ source3/smbd/service.c | 4 ++++ 5 files changed, 83 insertions(+), 1 deletion(-) diff --git a/source3/librpc/idl/messaging.idl b/source3/librpc/idl/messaging.idl index ce40a7b..847dc25 100644 --- a/source3/librpc/idl/messaging.idl +++ b/source3/librpc/idl/messaging.idl @@ -96,6 +96,9 @@ interface messaging MSG_SMB_TELL_NUM_CHILDREN = 0x0317, MSG_SMB_NUM_CHILDREN = 0x0318, + /* Cancel a notify, directory got deleted */ + MSG_SMB_NOTIFY_CANCEL_DELETED = 0x0319, + /* winbind messages */ MSG_WINBIND_FINISHED = 0x0401, MSG_WINBIND_FORGET_STATE = 0x0402, diff --git a/source3/locking/locking.c b/source3/locking/locking.c index bcc9bfe..ce595e1 100644 --- a/source3/locking/locking.c +++ b/source3/locking/locking.c @@ -46,6 +46,7 @@ #include "messages.h" #include "util_tdb.h" #include "../librpc/gen_ndr/ndr_open_files.h" +#include "librpc/gen_ndr/ndr_file_id.h" #include "locking/leases_db.h" #undef DBGC_CLASS @@ -1118,9 +1119,12 @@ void set_delete_on_close_lck(files_struct *fsp, const struct security_token *nt_tok, const struct security_unix_token *tok) { + struct messaging_context *msg_ctx = fsp->conn->sconn->msg_ctx; struct share_mode_data *d = lck->data; - int i; + uint32_t i; bool ret; + DATA_BLOB fid_blob = {}; + enum ndr_err_code ndr_err; SMB_ASSERT(nt_tok != NULL); SMB_ASSERT(tok != NULL); @@ -1144,6 +1148,31 @@ void set_delete_on_close_lck(files_struct *fsp, ret = add_delete_on_close_token(lck->data, fsp->name_hash, nt_tok, tok); SMB_ASSERT(ret); + + ndr_err = ndr_push_struct_blob(&fid_blob, talloc_tos(), &fsp->file_id, + (ndr_push_flags_fn_t)ndr_push_file_id); + if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + DEBUG(10, ("ndr_push_file_id failed: %s\n", + ndr_errstr(ndr_err))); + } + + for (i=0; inum_share_modes; i++) { + struct share_mode_entry *e = &d->share_modes[i]; + NTSTATUS status; + + status = messaging_send( + msg_ctx, e->pid, MSG_SMB_NOTIFY_CANCEL_DELETED, + &fid_blob); + + if (!NT_STATUS_IS_OK(status)) { + struct server_id_buf tmp; + DEBUG(10, ("%s: messaging_send to %s returned %s\n", + __func__, server_id_str_buf(e->pid, &tmp), + nt_errstr(status))); + } + } + + TALLOC_FREE(fid_blob.data); } bool set_delete_on_close(files_struct *fsp, bool delete_on_close, diff --git a/source3/smbd/notify.c b/source3/smbd/notify.c index 0e4969d..90b3ac0 100644 --- a/source3/smbd/notify.c +++ b/source3/smbd/notify.c @@ -23,6 +23,7 @@ #include "smbd/smbd.h" #include "smbd/globals.h" #include "../librpc/gen_ndr/ndr_notify.h" +#include "librpc/gen_ndr/ndr_file_id.h" struct notify_change_event { struct timespec when; @@ -439,6 +440,48 @@ void smbd_notify_cancel_by_smbreq(const struct smb_request *smbreq) smbd_notify_cancel_by_map(map); } +static struct files_struct *smbd_notify_cancel_deleted_fn( + struct files_struct *fsp, void *private_data) +{ + struct file_id *fid = talloc_get_type_abort( + private_data, struct file_id); + + if (file_id_equal(&fsp->file_id, fid)) { + remove_pending_change_notify_requests_by_fid( + fsp, NT_STATUS_DELETE_PENDING); + } + return NULL; +} + +void smbd_notify_cancel_deleted(struct messaging_context *msg, + void *private_data, uint32_t msg_type, + struct server_id server_id, DATA_BLOB *data) +{ + struct smbd_server_connection *sconn = talloc_get_type_abort( + private_data, struct smbd_server_connection); + struct file_id *fid; + enum ndr_err_code ndr_err; + + fid = talloc(talloc_tos(), struct file_id); + if (fid == NULL) { + DEBUG(1, ("talloc failed\n")); + return; + } + + ndr_err = ndr_pull_struct_blob_all( + data, fid, fid, (ndr_pull_flags_fn_t)ndr_pull_file_id); + if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + DEBUG(10, ("%s: ndr_pull_file_id failed: %s\n", __func__, + ndr_errstr(ndr_err))); + goto done; + } + + files_forall(sconn, smbd_notify_cancel_deleted_fn, fid); + +done: + TALLOC_FREE(fid); +} + /**************************************************************************** Delete entries by fnum from the change notify pending queue. *****************************************************************************/ diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 259a109..abfb543 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -519,6 +519,9 @@ NTSTATUS change_notify_add_request(struct smb_request *req, void (*reply_fn)(struct smb_request *req, NTSTATUS error_code, uint8_t *buf, size_t len)); +void smbd_notify_cancel_deleted(struct messaging_context *msg, + void *private_data, uint32_t msg_type, + struct server_id server_id, DATA_BLOB *data); void remove_pending_change_notify_requests_by_mid( struct smbd_server_connection *sconn, uint64_t mid); void remove_pending_change_notify_requests_by_fid(files_struct *fsp, diff --git a/source3/smbd/service.c b/source3/smbd/service.c index ada2d07..d11987e 100644 --- a/source3/smbd/service.c +++ b/source3/smbd/service.c @@ -682,6 +682,10 @@ static NTSTATUS make_connection_snum(struct smbXsrv_connection *xconn, if (sconn->notify_ctx == NULL) { sconn->notify_ctx = notify_init( sconn, sconn->msg_ctx, sconn->ev_ctx); + status = messaging_register( + sconn->msg_ctx, sconn, + MSG_SMB_NOTIFY_CANCEL_DELETED, + smbd_notify_cancel_deleted); } if (sconn->sys_notify_ctx == NULL) { sconn->sys_notify_ctx = sys_notify_context_create( -- 1.9.1 From a8215150265bcc2863f508a96df78dead0033471 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 20 Apr 2015 10:44:07 +0000 Subject: [PATCH 5/5] torture: Add smb2.notify.rmdir We need to cancel a pending FileChangeNotify with DELETE_PENDING if the directory watched is about to be deleted. I know I just deleted a bool parameter, but to me torture is different :-) BUG: https://bugzilla.samba.org/show_bug.cgi?id=11257 Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Thu Apr 23 01:36:48 CEST 2015 on sn-devel-104 (cherry picked from commit 79dc084dcb6e28211addfc5d75b817cc735d67c1) --- source4/torture/smb2/notify.c | 110 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/source4/torture/smb2/notify.c b/source4/torture/smb2/notify.c index 5589559..3c21cfe 100644 --- a/source4/torture/smb2/notify.c +++ b/source4/torture/smb2/notify.c @@ -2242,6 +2242,108 @@ done: return ret; } +static bool torture_smb2_notify_rmdir(struct torture_context *torture, + struct smb2_tree *tree1, + struct smb2_tree *tree2, + bool initial_delete_on_close) +{ + bool ret = true; + NTSTATUS status; + union smb_notify notify = {}; + union smb_setfileinfo sfinfo = {}; + union smb_open io = {}; + struct smb2_handle h = {}; + struct smb2_request *req; + + torture_comment(torture, "TESTING NOTIFY CANCEL FOR DELETED DIR\n"); + + smb2_deltree(tree1, BASEDIR); + smb2_util_rmdir(tree1, BASEDIR); + + ZERO_STRUCT(io.smb2); + io.generic.level = RAW_OPEN_SMB2; + io.smb2.in.create_flags = 0; + io.smb2.in.desired_access = SEC_FILE_ALL; + io.smb2.in.create_options = NTCREATEX_OPTIONS_DIRECTORY; + io.smb2.in.file_attributes = FILE_ATTRIBUTE_NORMAL; + io.smb2.in.share_access = + NTCREATEX_SHARE_ACCESS_READ | + NTCREATEX_SHARE_ACCESS_WRITE | + NTCREATEX_SHARE_ACCESS_DELETE ; + io.smb2.in.alloc_size = 0; + io.smb2.in.create_disposition = NTCREATEX_DISP_CREATE; + io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS; + io.smb2.in.security_flags = 0; + io.smb2.in.fname = BASEDIR; + + status = smb2_create(tree1, torture, &(io.smb2)); + CHECK_STATUS(status, NT_STATUS_OK); + h = io.smb2.out.file.handle; + + ZERO_STRUCT(notify.smb2); + notify.smb2.level = RAW_NOTIFY_SMB2; + notify.smb2.in.buffer_size = 1000; + notify.smb2.in.completion_filter = FILE_NOTIFY_CHANGE_NAME; + notify.smb2.in.file.handle = h; + notify.smb2.in.recursive = false; + + io.smb2.in.desired_access |= SEC_STD_DELETE; + io.smb2.in.create_disposition = NTCREATEX_DISP_OPEN; + req = smb2_notify_send(tree1, &(notify.smb2)); + + if (initial_delete_on_close) { + status = smb2_util_rmdir(tree2, BASEDIR); + CHECK_STATUS(status, NT_STATUS_OK); + } else { + status = smb2_create(tree2, torture, &(io.smb2)); + CHECK_STATUS(status, NT_STATUS_OK); + + sfinfo.generic.level = RAW_SFILEINFO_DISPOSITION_INFORMATION; + sfinfo.generic.in.file.handle = io.smb2.out.file.handle; + sfinfo.disposition_info.in.delete_on_close = 1; + status = smb2_setinfo_file(tree2, &sfinfo); + CHECK_STATUS(status, NT_STATUS_OK); + + smb2_util_close(tree2, io.smb2.out.file.handle); + } + + status = smb2_notify_recv(req, torture, &(notify.smb2)); + CHECK_STATUS(status, NT_STATUS_DELETE_PENDING); + +done: + + smb2_util_close(tree1, h); + smb2_deltree(tree1, BASEDIR); + + return ret; +} + +static bool torture_smb2_notify_rmdir1(struct torture_context *torture, + struct smb2_tree *tree) +{ + return torture_smb2_notify_rmdir(torture, tree, tree, false); +} + +static bool torture_smb2_notify_rmdir2(struct torture_context *torture, + struct smb2_tree *tree) +{ + return torture_smb2_notify_rmdir(torture, tree, tree, true); +} + +static bool torture_smb2_notify_rmdir3(struct torture_context *torture, + struct smb2_tree *tree1, + struct smb2_tree *tree2) +{ + return torture_smb2_notify_rmdir(torture, tree1, tree2, false); +} + +static bool torture_smb2_notify_rmdir4(struct torture_context *torture, + struct smb2_tree *tree1, + struct smb2_tree *tree2) +{ + return torture_smb2_notify_rmdir(torture, tree1, tree2, true); +} + /* basic testing of SMB2 change notify */ @@ -2267,6 +2369,14 @@ struct torture_suite *torture_smb2_notify_init(void) torture_suite_add_1smb2_test(suite, "tcp", torture_smb2_notify_tcp_disconnect); torture_suite_add_2smb2_test(suite, "rec", torture_smb2_notify_recursive); torture_suite_add_1smb2_test(suite, "overflow", torture_smb2_notify_overflow); + torture_suite_add_1smb2_test(suite, "rmdir1", + torture_smb2_notify_rmdir1); + torture_suite_add_1smb2_test(suite, "rmdir2", + torture_smb2_notify_rmdir2); + torture_suite_add_2smb2_test(suite, "rmdir3", + torture_smb2_notify_rmdir3); + torture_suite_add_2smb2_test(suite, "rmdir4", + torture_smb2_notify_rmdir4); suite->description = talloc_strdup(suite, "SMB2-NOTIFY tests"); -- 1.9.1