[PATCH] Fix libsmb renaming over existing files

Jeremy Allison jra at samba.org
Mon Dec 12 23:58:52 UTC 2016


On Mon, Dec 12, 2016 at 12:53:16PM -0800, Jeremy Allison wrote:
> On Mon, Dec 12, 2016 at 09:11:09PM +0100, Stefan Metzmacher wrote:
> > Until then I think it's better to catch this generically
> > in the caller.
> > 
> > E.g. in cli_rename() and a lot of others we have the following
> > pattern at the beginning:
> > 
> >         if (smbXcli_conn_protocol(cli->conn) >= PROTOCOL_SMB2_02) {
> >                 return cli_smb2_rename(cli,
> >                                         fname_src,
> >                                         fname_dst);
> >         }
> > 
> > I think we should change this to:
> > 
> >         if (smbXcli_conn_protocol(cli->conn) >= PROTOCOL_SMB2_02) {
> >                 cli->raw_status = cli_smb2_rename(cli,
> >                                         fname_src,
> >                                         fname_dst);
> > 		return cli->raw_status;
> >         }
> > 
> > 
> > Otherwise we'll miss a lot of places within the cli_smb2_* functions
> > where we miss to set cli->raw_status.
> 
> Unfortunately there are places inside source3/libsmb/cli_smb2_fnum.c
> where cli->raw_status gets set by an intermediate function. The
> classic case is where a pathname function calls ntcreate()/operation/close
> as SMB2 only has handle-based ops. The close call sets cli->raw_status
> so with just your suggestion we lose the raw_status on the 'operation'
> that we actually wanted the error code from.
> 
> That's why Volker's patch sets cli->raw_status *after* the close
> is done, to overwrite whatever the close set it to.
> 
> Right now inside source3/libsmb/cli_smb2_fnum.c cli->raw_status
> always gets set in:
> 
> cli_smb2_create_fnum_recv()
> cli_smb2_close_fnum_recv()
> cli_smb2_read_recv()
> cli_smb2_write_recv()
> cli_smb2_writeall_recv()
> cli_smb2_splice_recv()
> 
> And gets set only on error in:
> 
> cli_smb2_get_fs_attr_info()
> 
> Unfortunately we can't just set it on error due to
> upper level functions such as:
> 
> cli_is_error()
> 
> which expect cli->raw_status to be NT_STATUS_OK
> if the last operation succeeded.
> 
> So I think the lower-level patch is needed for
> now.

And here it is. Please review and push if happy (passes
make test here).

Jeremy.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s3-libsmb-Ensure-SMB2-operations-correctly-set-cli-r.patch
Type: text/x-diff
Size: 6093 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20161212/ef7de493/0001-s3-libsmb-Ensure-SMB2-operations-correctly-set-cli-r.diff>


More information about the samba-technical mailing list