From 4dda9c0a47aceb562e13e4722da5c894693ab5e8 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 30 Jan 2014 16:12:44 +0100 Subject: [PATCH] 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 Reviewed-by: Andreas Schneider Reviewed-by: Jeremy Allison Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Fri Feb 14 11:18:15 CET 2014 on sn-devel-104 (cherry picked from commit e0bf930f23fe20ee00d0006a5f6c2ba1a8f592a0) --- source3/smbd/smb2_notify.c | 55 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/source3/smbd/smb2_notify.c b/source3/smbd/smb2_notify.c index 81aa615..c35acc5 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 */ -- 1.7.9.5