[PATCH] Fix libsmb renaming over existing files

[Jeremy Allison]

On Tue, Dec 13, 2016 at 08:13:06AM +0200, Uri Simchoni wrote:
> 
> One comment + one patch to apply on top (mea cupla...)
> Otherwise RB+ me.

Thanks !

> > From 61236af38d52a063d2b3b46d0b1c1f625d10a67d Mon Sep 17 00:00:00 2001
> > From: Jeremy Allison <jra at samba.org>
> > Date: Mon, 12 Dec 2016 15:52:11 -0800
> > Subject: [PATCH] s3: libsmb: Ensure SMB2 operations correctly set
> >  cli->raw_status.
> > 
> > Needs to be done even on success (cli_is_error() checks if
> > cli->raw_status was NT_STATUS_OK).
> > 
> > BUG: https://bugzilla.samba.org/show_bug.cgi?id=12468
> > 
> > Signed-off-by: Jeremy Allison <jra at samba.org>
> > ---
> >  source3/libsmb/cli_smb2_fnum.c | 57 +++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 51 insertions(+), 6 deletions(-)
> > 
> > diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c
> > index 5a96b05..346af89 100644
> > --- a/source3/libsmb/cli_smb2_fnum.c
> > +++ b/source3/libsmb/cli_smb2_fnum.c
> > @@ -886,6 +886,9 @@ NTSTATUS cli_smb2_list(struct cli_state *cli,
> >  	if (fnum != 0xffff) {
> >  		cli_smb2_close_fnum(cli, fnum);
> >  	}
> > +
> > +	cli->raw_status = status;
> > +
> >  	TALLOC_FREE(subframe);
> >  	TALLOC_FREE(frame);
> >  	return status;
> > @@ -957,7 +960,7 @@ NTSTATUS cli_smb2_qpathinfo_basic(struct cli_state *cli,
> >  		return status;
> >  	}
> >  
> > -	cli_smb2_close_fnum(cli, fnum);
> > +	status = cli_smb2_close_fnum(cli, fnum);
> >  
> >  	ZERO_STRUCTP(sbuf);
> >  
> > @@ -967,7 +970,7 @@ NTSTATUS cli_smb2_qpathinfo_basic(struct cli_state *cli,
> >  	sbuf->st_ex_size = cr.end_of_file;
> >  	*attributes = cr.file_attributes;
> >  
> > -	return NT_STATUS_OK;
> > +	return status;
> >  }
> >  
> 
> This doesn't seem to set raw_status, and also, do you really want to
> fail an otherwise successful query call because the close failed?

The cli_smb2_close_fnum() itself sets cli->raw_status on
both success or fail. And yeah, when an operation does open/op/close
I think we should return an error if the op succeeded but the close
failed.