[PATCH] Fix valgrind read-after-free error in cli_smb2_close_fnum_recv().

[Jeremy Allison]

On Wed, Nov 29, 2017 at 07:06:08PM +0100, Ralph Böhme wrote:
> On Wed, Nov 29, 2017 at 09:33:43AM -0800, Jeremy Allison wrote:
> > cli_smb2_close_fnum_recv() uses tevent_req_simple_recv_ntstatus(req), which
> > frees req, then uses the state pointer which was owned by req.
> > 
> > Please review and push.
> 
> looks complicated. What about the attached version?

Yeah, I actually coded that up first and rejected it :-), because
it still leaves the tevent_req_simple_recv_ntstatus(req)
inside, which (silently) frees the data inside req.

There's no indication in that API that req->state is
gone, so I wanted to leave tevent_req_simple_recv_ntstatus(req)
only for cases that look like:

NTSTATUS XXX_recv(struct tevent_req *req)
{
	return tevent_req_simple_recv_ntstatus(req);
}

But if you really want your version, I'm good with
anything that removes the valgrind error really :-).

Jeremy.

> -- 
> Ralph Boehme, Samba Team       https://samba.org/
> Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

> From da7e28bf7d57ef974a88825a3d78444bac111a2b Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Wed, 29 Nov 2017 19:03:40 +0100
> Subject: [PATCH] s3: libsmb: Fix valgrind read-after-free error in
>  cli_smb2_close_fnum_recv().
> 
> cli_smb2_close_fnum_recv() uses tevent_req_simple_recv_ntstatus(req), which
> frees req, then uses the state pointer which was owned by req.
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13171
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/libsmb/cli_smb2_fnum.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c
> index 628b17b293b..a827817b45e 100644
> --- a/source3/libsmb/cli_smb2_fnum.c
> +++ b/source3/libsmb/cli_smb2_fnum.c
> @@ -449,9 +449,10 @@ NTSTATUS cli_smb2_close_fnum_recv(struct tevent_req *req)
>  {
>  	struct cli_smb2_close_fnum_state *state = tevent_req_data(
>  		req, struct cli_smb2_close_fnum_state);
> -	NTSTATUS status = tevent_req_simple_recv_ntstatus(req);
> -	state->cli->raw_status = status;
> -	return status;
> +	struct cli_state *cli = state->cli;
> +
> +	cli->raw_status  = tevent_req_simple_recv_ntstatus(req);
> +	return cli->raw_status;
>  }
>  
>  NTSTATUS cli_smb2_close_fnum(struct cli_state *cli, uint16_t fnum)
> -- 
> 2.13.6
>