[SCM] Samba Shared Repository - branch v4-0-test updated

Karolin Seeger kseeger at samba.org
Sun Feb 16 11:19:09 MST 2014


The branch, v4-0-test has been updated
       via  c400dd0 s3:smb2_notify: fix use after free on long living notify requests
      from  c10bc88 s3: modules: streaminfo: As we have no VFS function SMB_VFS_LLISTXATTR we can't cope with a symlink when lp_posix_pathnames() is true.

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v4-0-test


- Log -----------------------------------------------------------------
commit c400dd0f92688d9baece6dcd3d66d3245127c352
Author: Stefan Metzmacher <metze at 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 <metze at samba.org>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Stefan Metzmacher <metze at samba.org>
    Autobuild-Date(master): Fri Feb 14 11:18:15 CET 2014 on sn-devel-104
    (cherry picked from commit e0bf930f23fe20ee00d0006a5f6c2ba1a8f592a0)
    
    Autobuild-User(v4-0-test): Karolin Seeger <kseeger at samba.org>
    Autobuild-Date(v4-0-test): Sun Feb 16 19:18:59 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 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 */


-- 
Samba Shared Repository


More information about the samba-cvs mailing list