[PATCH] Fix notify bug
Volker Lendecke
Volker.Lendecke at SerNet.DE
Wed Apr 22 07:53:47 MDT 2015
Hi!
Attached find patches that fix a bug in our filechangenotify code. We
have to cancel notifies with DELETE_PENDING if the directory
watched goes away. See frame 28 in win8.cap.
Review&push appreciated!
Thanks,
Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: win8.cap
Type: application/vnd.tcpdump.pcap
Size: 7090 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150422/08e878a6/attachment.bin>
-------------- next part --------------
From 00d435881cf040219dc3407bb3ca2758b9669e1d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
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...
Signed-off-by: Volker Lendecke <vl at samba.org>
---
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; i<d->num_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; i<d->num_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 4ac701d32fe1cd3f0230843139dd365fc0e814fe Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 21 Apr 2015 11:38:32 +0200
Subject: [PATCH 2/5] smbd: Use reset_delete_on_close_lck directly
Signed-off-by: Volker Lendecke <vl at samba.org>
---
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 4fbd442f..415e50e 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 33062519ea5cce94070e3699b8995e5a8e0aa4c3 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
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.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
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 415e50e..09be2e7 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) {
@@ -1076,7 +1076,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 7a1e5baac726329404438626e12e564cd9726bdb Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 21 Apr 2015 10:16:16 +0200
Subject: [PATCH 4/5] smbd: Cancel pending notifies if the directory goes away
Signed-off-by: Volker Lendecke <vl at samba.org>
---
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/librpc/idl/messaging.idl b/librpc/idl/messaging.idl
index 04dfa1e..2b902ec 100644
--- a/librpc/idl/messaging.idl
+++ b/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; i<d->num_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 5ac8c0c..8cb44df 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;
@@ -418,6 +419,48 @@ void smbd_notify_cancel_by_smbreq(const struct smb_request *smbreq)
change_notify_remove_request(sconn, map->req);
}
+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 f01bbbd..a5144d5 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -517,6 +517,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 30ea93595cef76828b08c0535ad60919aa30ce42 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
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 :-)
Signed-off-by: Volker Lendecke <vl at samba.org>
---
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 bbdc223..0f572b6 100644
--- a/source4/torture/smb2/notify.c
+++ b/source4/torture/smb2/notify.c
@@ -2015,6 +2015,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
*/
@@ -2037,6 +2139,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
More information about the samba-technical
mailing list