Why are we using SMB_MALLOC_ARRAY in smb2_setinfo.c
Jeremy Allison
jra at samba.org
Tue Jun 16 10:52:10 MDT 2015
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
>
More information about the samba-technical
mailing list