cifs: copy_file_range needs to strip setuid bits and update timestamps
Subsystems (3)
PATCH 1/1 cifs: copy_file_range needs to strip setuid bits and update timestamps
cifs has both source and destination inodes locked throughout the copy. Like ->write_iter(), we update mtime and strip setuid bits of destination file before copy and like ->read_iter(), we update atime of source file after copy.
---
Hi Steve,
Please apply this patch to you cifs branch after merging Darrick's copy-file-range-fixes branch from:
git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git
Thanks,
Amir.
fs/cifs/cifsfs.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
fs/cifs/cifsfs.c +8 -3
@@ -1096,6 +1096,10 @@ ssize_t cifs_file_copychunk_range(unsigned int xid, goto out; } + rc = -EOPNOTSUPP ; +if (!target_tcon->ses->server->ops->copychunk_range) + goto out; +/* * Note: cifs case is easier than btrfs since server responsible for * checks for proper open modes and file type and if it wants
@@ -1107,11 +1111,12 @@ ssize_t cifs_file_copychunk_range(unsigned int xid,/* should we flush first and last page first */ truncate_inode_pages (&target_inode->i_data,0 ); -if (target_tcon->ses->server->ops->copychunk_range) + rc =file_modified (dst_file); +if (!rc) rc = target_tcon->ses->server->ops->copychunk_range (xid, smb_file_src, smb_file_target, off, len, destoff); -else - rc = -EOPNOTSUPP ; + +file_accessed (src_file);/* force revalidate of size and timestamps of target file now * that target is updated on the server
Signature · 1 line
2.17.1
Looks good in my testing so far - but also want to do a little more testing with the copy_file_range xfstest cases because your patches fixed one additional test (not cross mount copy) so we can understand why it fixed that test case.
Quoted 55 lines · Amir Goldstein
cifs has both source and destination inodes locked throughout the copy. Like ->write_iter(), we update mtime and strip setuid bits of destination file before copy and like ->read_iter(), we update atime of source file after copy.
Signed-off-by Amir Goldstein <[email protected]>---
Hi Steve,
Please apply this patch to you cifs branch after merging Darrick's copy-file-range-fixes branch from:
git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.gitThanks,
Amir.fs/cifs/cifsfs.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)fs/cifs/cifsfs.c +8
-4@@ -1096,6 +1096,10 @@ ssize_t cifs_file_copychunk_range(unsigned int xid, goto out; } + rc = -EOPNOTSUPP ; +if (!target_tcon->ses->server->ops->copychunk_range) + goto out; +/* * Note: cifs case is easier than btrfs since server responsible for * checks for proper open modes and file type and if it wants @@ -1107,11 +1111,12 @@ ssize_t cifs_file_copychunk_range(unsigned int xid,/* should we flush first and last page first */ truncate_inode_pages (&target_inode->i_data,0 ); -if (target_tcon->ses->server->ops->copychunk_range) + rc =file_modified (dst_file); +if (!rc) rc = target_tcon->ses->server->ops->copychunk_range (xid, smb_file_src, smb_file_target, off, len, destoff); -else - rc = -EOPNOTSUPP ; + +file_accessed (src_file);/* force revalidate of size and timestamps of target file now * that target is updated on the server -- 2.17.1
Signature · 3 lines
Thanks, Steve
Quoted 5 lines · Steve French
Looks good in my testing so far - but also want to do a little more testing with the
copy_file_rangexfstest cases because your patches fixed one additional test (not cross mount copy) so we can understand why it fixed that test case.
I know which of my patches fixed generic/43[01].
It was 96e6e8f4a68d ("vfs: add missing checks to copy_file_range") More specifically this code:
/* Shorten the copy to EOF */
size_in = i_size_read(inode_in);
if (pos_in >= size_in)
count = 0;
else
count = min(count, size_in - (uint64_t)pos_in);If CIFS sends an out of range value of copy length to Windows server, server replies with an error. That is inconsistent with the semantics of copy_file_range(2) syscall, which expects "short copy", hence need to shorten length before passing on to server.
I verified with Aurelien that this is the case and I was under the impression that he was going to create a similar local fix to cifs code for stable. I thought he told you, so I forgot to report back myself.
Thanks,
Amir.