The branch, master has been updated
via e0bf930 s3:smb2_notify: fix use after free on long living notify
requests
from 0535f73 ctdb:vacuum: move retrieval of freelist to after vacuum run
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit e0bf930f23fe20ee00d0006a5f6c2ba1a8f592a0
Author: Stefan Metzmacher <me...@samba.org>
Date: Thu Jan 30 16:12:44 2014 +0100
s3:smb2_notify: fix use after free on long living notify requests
This is a hack, but it should fix the bug:
change_notify_add_request() talloc moves smb_request away,
which is not expected by the smb2_notify.c code...
smbd_smb2_notify_reply() uses tevent_req_defer_callback()
(in older versions an immediate event) to defer the response.
This is needed as change_notify_reply() will do more things
after calling reply_fn() (smbd_smb2_notify_reply is this case)
and often change_notify_remove_request() is called after
change_notify_reply().
change_notify_remove_request() implicitly free's the smb_request
that was passed to change_notify_add_request().
smbd_smb2_fake_smb_request() added the smb_request as smb2req->smb1req,
which is expected to be available after smbd_smb2_notify_recv() returned.
The long term solution would be the following interface:
struct tevent_req *change_notify_request_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
struct files_struct *fsp,
uint32_t max_length,
uint32_t filter,
bool recursive);
NTSTATUS change_notify_request_recv(struct tevent_req *req,
TALLOC_CTX *mem_ctx,
DATA_BLOB *buffer);
Bug: https://bugzilla.samba.org/show_bug.cgi?id=10442
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Reviewed-by: Andreas Schneider <a...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
Autobuild-User(master): Stefan Metzmacher <me...@samba.org>
Autobuild-Date(master): Fri Feb 14 11:18:15 CET 2014 on sn-devel-104
-----------------------------------------------------------------------
Summary of changes:
source3/smbd/smb2_notify.c | 55 ++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 55 insertions(+), 0 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/smbd/smb2_notify.c b/source3/smbd/smb2_notify.c
index 77399ef..228346e 100644
--- a/source3/smbd/smb2_notify.c
+++ b/source3/smbd/smb2_notify.c
@@ -28,6 +28,8 @@
struct smbd_smb2_notify_state {
struct smbd_smb2_request *smb2req;
struct smb_request *smbreq;
+ bool has_request;
+ bool skip_reply;
NTSTATUS status;
DATA_BLOB out_output_buffer;
};
@@ -160,6 +162,44 @@ static void smbd_smb2_notify_reply(struct smb_request
*smbreq,
uint8_t *buf, size_t len);
static bool smbd_smb2_notify_cancel(struct tevent_req *req);
+static int smbd_smb2_notify_state_destructor(struct smbd_smb2_notify_state
*state)
+{
+ if (!state->has_request) {
+ return 0;
+ }
+
+ state->skip_reply = true;
+ smbd_notify_cancel_by_smbreq(state->smbreq);
+ return 0;
+}
+
+static int smbd_smb2_notify_smbreq_destructor(struct smb_request *smbreq)
+{
+ struct tevent_req *req = talloc_get_type_abort(smbreq->async_priv,
+ struct tevent_req);
+ struct smbd_smb2_notify_state *state = tevent_req_data(req,
+ struct smbd_smb2_notify_state);
+
+ /*
+ * Our temporary parent from change_notify_add_request()
+ * goes away.
+ */
+ state->has_request = false;
+
+ /*
+ * move it back to its original parent,
+ * which means we no longer need the destructor
+ * to protect it.
+ */
+ talloc_steal(smbreq->smb2req, smbreq);
+ talloc_set_destructor(smbreq, NULL);
+
+ /*
+ * We want to keep smbreq!
+ */
+ return -1;
+}
+
static struct tevent_req *smbd_smb2_notify_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
struct smbd_smb2_request
*smb2req,
@@ -183,6 +223,7 @@ static struct tevent_req *smbd_smb2_notify_send(TALLOC_CTX
*mem_ctx,
state->smb2req = smb2req;
state->status = NT_STATUS_INTERNAL_ERROR;
state->out_output_buffer = data_blob_null;
+ talloc_set_destructor(state, smbd_smb2_notify_state_destructor);
DEBUG(10,("smbd_smb2_notify_send: %s - %s\n",
fsp_str_dbg(fsp), fsp_fnum_dbg(fsp)));
@@ -266,6 +307,16 @@ static struct tevent_req *smbd_smb2_notify_send(TALLOC_CTX
*mem_ctx,
return tevent_req_post(req, ev);
}
+ /*
+ * This is a HACK!
+ *
+ * change_notify_add_request() talloc_moves()
+ * smbreq away from us, so we need a destructor
+ * which moves it back at the end.
+ */
+ state->has_request = true;
+ talloc_set_destructor(smbreq, smbd_smb2_notify_smbreq_destructor);
+
/* allow this request to be canceled */
tevent_req_set_cancel_fn(req, smbd_smb2_notify_cancel);
@@ -281,6 +332,10 @@ static void smbd_smb2_notify_reply(struct smb_request
*smbreq,
struct smbd_smb2_notify_state *state = tevent_req_data(req,
struct smbd_smb2_notify_state);
+ if (state->skip_reply) {
+ return;
+ }
+
state->status = error_code;
if (!NT_STATUS_IS_OK(error_code)) {
/* nothing */
--
Samba Shared Repository
