Why are we using SMB_MALLOC_ARRAY in smb2_setinfo.c

[Jeremy Allison]

On Mon, Jun 15, 2015 at 08:42:03AM +0200, Stefan (metze) Metzmacher wrote:
> Hi Richard,
> 
> the following patch should fix the problem.

Pushed with a rename of talloc_set_destructor -> defer_rename_state_destructor
to make it compile :-).

> For 4.2 we also need commit 78fb663d4c29f9a226c26001492cbc7a9a701668
> (smbd: Fix CID 1273088 Resource leak).


> 
> Am 14.06.2015 um 19:47 schrieb Richard Sharpe:
> > Hi folks,
> > 
> > In smb2_setinfo.c: smbd_smb2_setinfo_send I see the following code in
> > the SMB2_SETINFO_FILE branch of the switch:
> > 
> >                 data = NULL;
> >                 data_size = in_input_buffer.length;
> >                 if (data_size > 0) {
> >                         data = (char *)SMB_MALLOC_ARRAY(char, data_size);
> >                         if (tevent_req_nomem(data, req)) {
> >                                 return tevent_req_post(req, ev);
> >                         }
> >                         memcpy(data, in_input_buffer.data, data_size);
> >                 }
> > 
> > ...
> > 
> > And then, a little further down there appears to be an early return
> > that can leak that memory. This is the
> > SMB2_FILE_RENAME_INFORMATION_INTERNAL case.
> > 

> From ac1678418443520f0249f95bd618efe7c3d55298 Mon Sep 17 00:00:00 2001
> From: Stefan Metzmacher <metze at samba.org>
> Date: Mon, 15 Jun 2015 08:34:12 +0200
> Subject: [PATCH] s3:smb2_setinfo: fix memory leak in the defer_rename case
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=11329
> 
> Signed-off-by: Stefan Metzmacher <metze at samba.org>
> ---
>  source3/smbd/smb2_setinfo.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/source3/smbd/smb2_setinfo.c b/source3/smbd/smb2_setinfo.c
> index 3f7bbec..930739a 100644
> --- a/source3/smbd/smb2_setinfo.c
> +++ b/source3/smbd/smb2_setinfo.c
> @@ -168,6 +168,12 @@ struct defer_rename_state {
>  	int data_size;
>  };
>  
> +static int talloc_set_destructor(struct defer_rename_state *rename_state)
> +{
> +	SAFE_FREE(rename_state->data);
> +	return 0;
> +}
> +
>  static void defer_rename_done(struct tevent_req *subreq);
>  
>  static struct tevent_req *delay_rename_for_lease_break(struct tevent_req *req,
> @@ -240,6 +246,8 @@ static struct tevent_req *delay_rename_for_lease_break(struct tevent_req *req,
>  	rename_state->data = data;
>  	rename_state->data_size = data_size;
>  
> +	talloc_set_destructor(rename_state, defer_rename_state_destructor);
> +
>  	subreq = dbwrap_record_watch_send(
>  				rename_state,
>  				ev,
> -- 
> 1.9.1
>