[PATCH] Fix valgrind read-after-free error in cli_smb2_close_fnum_recv().
jra at samba.org
Wed Nov 29 18:13:00 UTC 2017
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)
But if you really want your version, I'm good with
anything that removes the valgrind error really :-).
> 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() 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)
More information about the samba-technical