On Sat, 16 Nov 2013 01:14:19 -0600
Steve French <[email protected]> wrote:

>  fs/cifs/smb2ops.c | 89 
> ++++++++++++++++++++++++++++++++++++++++++++++++-------
>  fs/cifs/smb2pdu.c |  2 +-
>  2 files changed, 79 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 11dde4b..7a21447 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -532,7 +532,10 @@ smb2_clone_range(const unsigned int xid,
>       int rc;
>       unsigned int ret_data_len;
>       struct copychunk_ioctl *pcchunk;
> -     char *retbuf = NULL;
> +     struct copychunk_ioctl_rsp *retbuf = NULL;
> +     struct cifs_tcon *tcon;
> +     int chunks_copied = 0;
> +     bool chunk_sizes_updated = false;
>  
>       pcchunk = kmalloc(sizeof(struct copychunk_ioctl), GFP_KERNEL);
>  
> @@ -552,22 +555,86 @@ smb2_clone_range(const unsigned int xid,

The SMB2_request_res_key() error path above here should be fixed to free
pcchunk.

>       /* For now array only one chunk long, will make more flexible later */
>       pcchunk->ChunkCount = __constant_cpu_to_le32(1);
>       pcchunk->Reserved = 0;
> -     pcchunk->SourceOffset = cpu_to_le64(src_off);
> -     pcchunk->TargetOffset = cpu_to_le64(dest_off);
> -     pcchunk->Length = cpu_to_le32(len);
>       pcchunk->Reserved2 = 0;
>  
> +     tcon = tlink_tcon(trgtfile->tlink);
> +
> +     while (len > 0) {
> +             pcchunk->SourceOffset = cpu_to_le64(src_off);
> +             pcchunk->TargetOffset = cpu_to_le64(dest_off);
> +             pcchunk->Length =
> +                     cpu_to_le32(min_t(u32, len, tcon->max_bytes_chunk));
> +
>       /* Request that server copy to target from src file identified by key */

Comment should be indented here too.

> -     rc = SMB2_ioctl(xid, tlink_tcon(trgtfile->tlink),
> -                     trgtfile->fid.persistent_fid,
> +             rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
>                       trgtfile->fid.volatile_fid, FSCTL_SRV_COPYCHUNK_WRITE,
>                       true /* is_fsctl */, (char *)pcchunk,
> -                     sizeof(struct copychunk_ioctl), &retbuf, &ret_data_len);
> -
> -     /* BB need to special case rc = EINVAL to alter chunk size */
> -
> -     cifs_dbg(FYI, "rc %d data length out %d\n", rc, ret_data_len);
> +                     sizeof(struct copychunk_ioctl), (char **)&retbuf,
> +                     &ret_data_len);
> +             if (rc == 0) {
> +                     if (ret_data_len != sizeof(struct copychunk_ioctl_rsp))
> +                             goto cchunk_out;
> +                     if (retbuf->TotalBytesWritten == 0) {
> +                             cifs_dbg(FYI, "no bytes copied\n");
> +                             goto cchunk_out;
> +                     }

These two error cases should set rc non-zero, it may not be the last
copy-chunk for this clone.
...

> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -1214,7 +1214,7 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon 
> *tcon, u64 persistent_fid,
>       rc = SendReceive2(xid, ses, iov, num_iovecs, &resp_buftype, 0);
>       rsp = (struct smb2_ioctl_rsp *)iov[0].iov_base;
>  
> -     if (rc != 0) {
> +     if ((rc != 0) && (rc != -EINVAL)) {

This should probably be FSCTL_SRV_COPYCHUNK[_WRITE] specific. See MS-SMB2
  3.3.4.4   Sending an Error Response
  When the server is responding with a failure to any command sent by the
  client, the response message MUST be constructed as described here. An
  error code other than one of the following indicates a failure:
  ... 

  STATUS_INVALID_PARAMETER in an FSCTL_SRV_COPYCHUNK or
  FSCTL_SRV_COPYCHUNK_WRITE response, when returning an
  SRV_COPYCHUNK_RESPONSE as described in section 3.3.5.15.6.2.


>               if (tcon)
>                       cifs_stats_fail_inc(tcon, SMB2_IOCTL_HE);
>               goto ioctl_exit;

Cheers, David
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to