Samba server bug: CopyChunk from one share to a different share on same server

Samba server bug: CopyChunk from one share to a different share on same server

[Steve French]

Tried a quick experiment using cloner (David's tool - xfstests/src/cloner)
to copy a file from share1 to share2 on the same target server (using
CopyChunk and SMB3).  Works fine for Windows (tried Windows 8.1) but failed
for Samba 4.2.

This is one of the more important cases to support (copying a file from one
share to another)

Samba returns STATUS_OBJECT_NAME_NOT_FOUND on FSCTL_SRV_COPYCHUNK_WRITE
unless source and target are on the same share.  I didn't try it with
vfs_btrfs (the test system was running ext4, and both exports are on the
same volume on the server).

Was testing the change below where I relax the copy offload check as
follows (to allow cross share copy chunk as Windows does)

diff --git a/fs/cifs/ioctl.c b/fs/cifs/ioctl.c
index 28a77bf..35cf990 100644
--- a/fs/cifs/ioctl.c
+++ b/fs/cifs/ioctl.c
@@ -85,9 +85,14 @@ static long cifs_ioctl_clone(unsigned int xid, struct
file *dst_file,
     src_tcon = tlink_tcon(smb_file_src->tlink);
     target_tcon = tlink_tcon(smb_file_target->tlink);

-    /* check if source and target are on same tree connection */
-    if (src_tcon != target_tcon) {
-        cifs_dbg(VFS, "file copy src and target on different volume\n");
+    /* check source and target on same server (or volume if dup_extents) */
+    if (dup_extents && (src_tcon != target_tcon)) {
+        cifs_dbg(VFS, "source and target of copy not on same share\n");
+        goto out_fput;
+    }
+
+    if (!dup_extents && (src_tcon->ses != target_tcon->ses)) {
+        cifs_dbg(VFS, "source and target of copy not on same server\n");
         goto out_fput;
     }


-- 
Thanks,

Steve

Samba server bug: CopyChunk from one share to a different share on same server

[Steve French]

Tried a quick experiment using cloner (David's tool in
xfstests/src/cloner.c) to copy a file from share1 to share2 on the same
target server (using CopyChunk and SMB3).  Works fine for Windows (tried
Windows 8.1 as server) but failed for Samba 4.2.

This is one of the more important cases to support (copying a file from one
share to another).

Samba returned STATUS_OBJECT_NAME_NOT_FOUND on FSCTL_SRV_COPYCHUNK_WRITE
unless source and target are on the same share.  I didn't try it with
vfs_btrfs (the test system was running ext4, and both exports are on the
same volume on the server).

Was testing the change below where I relax the copy offload check in
cifs.ko as follows (to allow cross share copy chunk as Windows often does)

diff --git a/fs/cifs/ioctl.c b/fs/cifs/ioctl.c
index 28a77bf..35cf990 100644
--- a/fs/cifs/ioctl.c
+++ b/fs/cifs/ioctl.c
@@ -85,9 +85,14 @@ static long cifs_ioctl_clone(unsigned int xid, struct
file *dst_file,
     src_tcon = tlink_tcon(smb_file_src->tlink);
     target_tcon = tlink_tcon(smb_file_target->tlink);

-    /* check if source and target are on same tree connection */
-    if (src_tcon != target_tcon) {
-        cifs_dbg(VFS, "file copy src and target on different volume\n");
+    /* check source and target on same server (or volume if dup_extents) */
+    if (dup_extents && (src_tcon != target_tcon)) {
+        cifs_dbg(VFS, "source and target of copy not on same share\n");
+        goto out_fput;
+    }
+
+    if (!dup_extents && (src_tcon->ses != target_tcon->ses)) {
+        cifs_dbg(VFS, "source and target of copy not on same server\n");
         goto out_fput;
     }

-- 
Thanks,

Steve

Samba server bug: CopyChunk from one share to a different share on same server

[Steve French]

Tried a quick experiment using cloner (David's tool in
xfstests/src/cloner.c) to copy a file from share1 to share2 on the
same target server (using CopyChunk and SMB3).  Works fine for Windows
(tried Windows 8.1 as server) but failed for Samba 4.2.

This is one of the more important cases to support (copying a file
from one share to another).

Samba returned STATUS_OBJECT_NAME_NOT_FOUND on
FSCTL_SRV_COPYCHUNK_WRITE unless source and target are on the same
share.  I didn't try it with vfs_btrfs (the test system was running
ext4, and both exports are on the same volume on the server).

Was testing the change below where I relax the copy offload check in
cifs.ko as follows (to allow cross share copy chunk as Windows often
does)

diff --git a/fs/cifs/ioctl.c b/fs/cifs/ioctl.c
index 28a77bf..35cf990 100644
--- a/fs/cifs/ioctl.c
+++ b/fs/cifs/ioctl.c
@@ -85,9 +85,14 @@ static long cifs_ioctl_clone(unsigned int xid,
struct file *dst_file,
     src_tcon = tlink_tcon(smb_file_src->tlink);
     target_tcon = tlink_tcon(smb_file_target->tlink);

-    /* check if source and target are on same tree connection */
-    if (src_tcon != target_tcon) {
-        cifs_dbg(VFS, "file copy src and target on different volume\n");
+    /* check source and target on same server (or volume if dup_extents) */
+    if (dup_extents && (src_tcon != target_tcon)) {
+        cifs_dbg(VFS, "source and target of copy not on same share\n");
+        goto out_fput;
+    }
+
+    if (!dup_extents && (src_tcon->ses != target_tcon->ses)) {
+        cifs_dbg(VFS, "source and target of copy not on same server\n");
         goto out_fput;
     }

-- 
Thanks,

Steve

Re: Samba server bug: CopyChunk from one share to a different share on same server

[David Disseldorp]

On Sun, 8 Nov 2015 20:52:16 -0600, Steve French wrote:

> Tried a quick experiment using cloner (David's tool - xfstests/src/cloner)
> to copy a file from share1 to share2 on the same target server (using
> CopyChunk and SMB3).  Works fine for Windows (tried Windows 8.1) but failed
> for Samba 4.2.
> 
> This is one of the more important cases to support (copying a file from one
> share to another)
> 
> Samba returns STATUS_OBJECT_NAME_NOT_FOUND on FSCTL_SRV_COPYCHUNK_WRITE
> unless source and target are on the same share.  I didn't try it with
> vfs_btrfs (the test system was running ext4, and both exports are on the
> same volume on the server).

It's no different across Samba VFS modules - this behaviour is intended.
The persistent/volatile handle is used to generate the copy-chunk resume
key. file_fsp_get() is currently used to lookup the source fsp using the
destination file connection context.

> Was testing the change below where I relax the copy offload check as
> follows (to allow cross share copy chunk as Windows does)
> 
> diff --git a/fs/cifs/ioctl.c b/fs/cifs/ioctl.c
> index 28a77bf..35cf990 100644
> --- a/fs/cifs/ioctl.c
> +++ b/fs/cifs/ioctl.c
> @@ -85,9 +85,14 @@ static long cifs_ioctl_clone(unsigned int xid, struct
> file *dst_file,
>      src_tcon = tlink_tcon(smb_file_src->tlink);
>      target_tcon = tlink_tcon(smb_file_target->tlink);
> 
> -    /* check if source and target are on same tree connection */
> -    if (src_tcon != target_tcon) {
> -        cifs_dbg(VFS, "file copy src and target on different volume\n");
> +    /* check source and target on same server (or volume if dup_extents) */
> +    if (dup_extents && (src_tcon != target_tcon)) {
> +        cifs_dbg(VFS, "source and target of copy not on same share\n");
> +        goto out_fput;
> +    }
> +
> +    if (!dup_extents && (src_tcon->ses != target_tcon->ses)) {
> +        cifs_dbg(VFS, "source and target of copy not on same server\n");
>          goto out_fput;
>      }
> 
> 

This change looks fine to me.
Reviewed-by: David Disseldorp <ddiss-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

Cheers, David