Fixes for major copy_file_range() issues
Subsystems (10)
v4 cover Fixes for major copy_file_range() issues
Hi Darrick,
Following is a re-work of Dave Chinner's copy_file_range() patches. This v4 patch set includes review tags and excludes the individual filesystem fixes that are not related to cross-device copy. Those patches will be sent to maintainers seperately once the dependency patch is made available on a public branch.
I did include the FUSE patch in this posting since we got an ACK from Miklos. You may take it or leave it.
Thanks,
Amir.
Changes since v3:
- Drop per filesystem patch for file_modified()/file_accessed()
- Fix wrong likely()
- Add Reviewed-by tags
Changes since v2:
- Re-order generic_remap_checks() fix patch before forking generic_copy_file_checks()
- Document @req_count helper argument (Darrick)
- Fold generic_access_check_limits() (Darrick)
- Added file_modified() helper (Darrick)
- Added xfs patch to use file_modified() helper
- Drop generic_copy_file_range_prep() helper
- Per filesystem patch for file_modified()/file_accessed()
- Post copy file_remove_privs() for ceph/generic (Darrick)
Changes since v1:
- Short read instead of EINVAL (Christoph)
- generic_file_rw_checks() helper (Darrick)
- generic_copy_file_range_prep() helper (Christoph)
- Not calling ->remap_file_range() with different sb
- Not calling ->copy_file_range() with different fs type
- Remove changes to overlayfs
- Extra fix to clone/dedupe checks
Amir Goldstein (7):
vfs: introduce generic_file_rw_checks()
vfs: remove redundant checks from generic_remap_checks()
vfs: add missing checks to copy_file_range
vfs: introduce file_modified() helper
xfs: use file_modified() helper
vfs: allow copy_file_range to copy across devices
fuse: copy_file_range needs to strip setuid bits and update timestamps
Dave Chinner (2):
vfs: introduce generic_copy_file_range()
vfs: no fallback for ->copy_file_range
fs/ceph/file.c | 23 +++++++-- fs/cifs/cifsfs.c | 4 ++ fs/fuse/file.c | 29 +++++++++-- fs/inode.c | 20 +++++++ fs/nfs/nfs4file.c | 23 +++++++-- fs/read_write.c | 126 +++++++++++++++++++++++++-------------------- fs/xfs/xfs_file.c | 15 +----- include/linux/fs.h | 9 ++++ mm/filemap.c | 110 +++++++++++++++++++++++++++++++-------- 9 files changed, 259 insertions(+), 100 deletions(-)
Signature · 1 line
2.17.1
PATCH v4 1/9 vfs: introduce generic_copy_file_range()
From: Dave Chinner <[email protected]>
Right now if vfs_copy_file_range() does not use any offload mechanism, it falls back to calling do_splice_direct(). This fails to do basic sanity checks on the files being copied. Before we start adding this necessarily functionality to the fallback path, separate it out into generic_copy_file_range().
generic_copy_file_range() has the same prototype as ->copy_file_range() so that filesystems can use it in their custom ->copy_file_range() method if they so choose.
--- fs/read_write.c | 35 ++++++++++++++++++++++++++++++++--- include/linux/fs.h | 3 +++ 2 files changed, 35 insertions(+), 3 deletions(-)
fs/read_write.c +32 -3
@@ -1565,6 +1565,36 @@ COMPAT_SYSCALL_DEFINE4(sendfile64, int, out_fd, int, in_fd, }#endif +/** +* generic_copy_file_range - copy data between two files +* @file_in: file structure to read from +* @pos_in: file offset to read from +* @file_out: file structure to write data to +* @pos_out: file offset to write data to +* @len: amount of data to copy +* @flags: copy flags +* +* This is a generic filesystem helper to copy data from one file to another. +* It has no constraints on the source or destination file owners - the files +* can belong to different superblocks and different filesystem types. Short +* copies are allowed. +* +* This should be called from the @file_out filesystem, as per the +* ->copy_file_range() method. +* +* Returns the number of bytes copied or a negative error indicating the +* failure. +*/ + +ssize_t generic_copy_file_range (struct file *file_in,loff_t pos_in, +struct file *file_out,loff_t pos_out, +size_t len,unsigned int flags) +{ +return do_splice_direct (file_in, &pos_in, file_out, &pos_out, + len >MAX_RW_COUNT ?MAX_RW_COUNT : len,0 ); +} +EXPORT_SYMBOL (generic_copy_file_range); +/* * copy_file_range() differs from regular file read and write in that it * specifically allows return partial success. When it does so is up to
@@ -1632,9 +1662,8 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in, goto done; } - ret =do_splice_direct (file_in, &pos_in, file_out, &pos_out, - len >MAX_RW_COUNT ?MAX_RW_COUNT : len,0 ); - + ret =generic_copy_file_range (file_in, pos_in, file_out, pos_out, len, + flags); done:if (ret >0 ) {fsnotify_access (file_in);
include/linux/fs.h +3 -0
@@ -1889,6 +1889,9 @@ extern ssize_t vfs_readv(struct file *, const struct iovec __user *, unsigned long, loff_t *, rwf_t);extern ssize_t vfs_copy_file_range (struct file *,loff_t ,struct file *,loff_t ,size_t ,unsigned int ); +extern ssize_t generic_copy_file_range (struct file *file_in,loff_t pos_in, +struct file *file_out,loff_t pos_out, +size_t len,unsigned int flags); extern int generic_remap_file_range_prep(struct file *file_in, loff_t pos_in, struct file *file_out, loff_t pos_out, loff_t *count,
Signature · 1 line
2.17.1
PATCH v4 2/9 vfs: no fallback for ->copy_file_range
In reply to Amir GoldsteinFrom: Dave Chinner <[email protected]>
Now that we have generic_copy_file_range(), remove it as a fallback case when offloads fail. This puts the responsibility for executing fallbacks on the filesystems that implement ->copy_file_range and allows us to add operational validity checks to generic_copy_file_range().
Rework vfs_copy_file_range() to call a new do_copy_file_range() helper to execute the copying callout, and move calls to generic_file_copy_range() into filesystem methods where they currently return failures.
[Amir] overlayfs is not responsible of executing the fallback. It is the responsibility of the underlying filesystem.
--- fs/ceph/file.c | 21 ++++++++++++++++++--- fs/cifs/cifsfs.c | 4 ++++ fs/fuse/file.c | 21 ++++++++++++++++++--- fs/nfs/nfs4file.c | 20 +++++++++++++++++--- fs/read_write.c | 25 ++++++++++++++++--------- 5 files changed, 73 insertions(+), 18 deletions(-)
fs/ceph/file.c +18 -3
@@ -1889,9 +1889,9 @@ static int is_file_size_ok(struct inode *src_inode, struct inode *dst_inode,return 0 ; } -static ssize_t ceph_copy_file_range (struct file *src_file,loff_t src_off, -struct file *dst_file,loff_t dst_off, -size_t len,unsigned int flags) +static ssize_t __ceph_copy_file_range (struct file *src_file,loff_t src_off, +struct file *dst_file,loff_t dst_off, +size_t len,unsigned int flags) {struct inode *src_inode =file_inode (src_file);struct inode *dst_inode =file_inode (dst_file);
@@ -2100,6 +2100,21 @@ static ssize_t ceph_copy_file_range(struct file *src_file, loff_t src_off,return ret; } +static ssize_t ceph_copy_file_range (struct file *src_file,loff_t src_off, +struct file *dst_file,loff_t dst_off, +size_t len,unsigned int flags) +{ +ssize_t ret; + + ret =__ceph_copy_file_range (src_file, src_off, dst_file, dst_off, + len, flags); + +if (ret == -EOPNOTSUPP ) + ret =generic_copy_file_range (src_file, src_off, dst_file, + dst_off, len, flags); +return ret; +} +const struct file_operations ceph_file_fops = { .open = ceph_open, .release = ceph_release,
fs/cifs/cifsfs.c +4 -0
@@ -1148,6 +1148,10 @@ static ssize_t cifs_copy_file_range(struct file *src_file, loff_t off, rc =cifs_file_copychunk_range (xid, src_file, off, dst_file, destoff, len, flags);free_xid (xid); + +if (rc == -EOPNOTSUPP ) + rc =generic_copy_file_range (src_file, off, dst_file, + destoff, len, flags);return rc; }
fs/fuse/file.c +18 -3
@@ -3097,9 +3097,9 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,return err; } -static ssize_t fuse_copy_file_range (struct file *file_in,loff_t pos_in, -struct file *file_out,loff_t pos_out, -size_t len,unsigned int flags) +static ssize_t __fuse_copy_file_range (struct file *file_in,loff_t pos_in, +struct file *file_out,loff_t pos_out, +size_t len,unsigned int flags) {struct fuse_file *ff_in = file_in->private_data;struct fuse_file *ff_out = file_out->private_data;
@@ -3173,6 +3173,21 @@ static ssize_t fuse_copy_file_range(struct file *file_in, loff_t pos_in,return err; } +static ssize_t fuse_copy_file_range (struct file *src_file,loff_t src_off, +struct file *dst_file,loff_t dst_off, +size_t len,unsigned int flags) +{ +ssize_t ret; + + ret =__fuse_copy_file_range (src_file, src_off, dst_file, dst_off, + len, flags); + +if (ret == -EOPNOTSUPP ) + ret =generic_copy_file_range (src_file, src_off, dst_file, + dst_off, len, flags); +return ret; +} +static const struct file_operations fuse_file_operations = { .llseek = fuse_file_llseek, .read_iter = fuse_file_read_iter,
fs/nfs/nfs4file.c +17 -3
@@ -129,9 +129,9 @@ nfs4_file_flush(struct file *file, fl_owner_t id) }#ifdef CONFIG_NFS_V4_2 -static ssize_t nfs4_copy_file_range (struct file *file_in,loff_t pos_in, -struct file *file_out,loff_t pos_out, -size_t count,unsigned int flags) +static ssize_t __nfs4_copy_file_range (struct file *file_in,loff_t pos_in, +struct file *file_out,loff_t pos_out, +size_t count,unsigned int flags) {if (!nfs_server_capable (file_inode (file_out),NFS_CAP_COPY ))return -EOPNOTSUPP ;
@@ -140,6 +140,20 @@ static ssize_t nfs4_copy_file_range(struct file *file_in, loff_t pos_in,return nfs42_proc_copy (file_in, pos_in, file_out, pos_out, count); } +static ssize_t nfs4_copy_file_range (struct file *file_in,loff_t pos_in, +struct file *file_out,loff_t pos_out, +size_t count,unsigned int flags) +{ +ssize_t ret; + + ret =__nfs4_copy_file_range (file_in, pos_in, file_out, pos_out, count, + flags); +if (ret == -EOPNOTSUPP ) + ret =generic_copy_file_range (file_in, pos_in, file_out, + pos_out, count, flags); +return ret; +} +static loff_t nfs4_file_llseek (struct file *filep,loff_t offset,int whence) {loff_t ret;
fs/read_write.c +16 -9
@@ -1595,6 +1595,19 @@ ssize_t generic_copy_file_range(struct file *file_in, loff_t pos_in, }EXPORT_SYMBOL (generic_copy_file_range); +static ssize_t do_copy_file_range (struct file *file_in,loff_t pos_in, +struct file *file_out,loff_t pos_out, +size_t len,unsigned int flags) +{ +if (file_out->f_op->copy_file_range) +return file_out->f_op->copy_file_range (file_in, pos_in, + file_out, pos_out, + len, flags); + +return generic_copy_file_range (file_in, pos_in, file_out, pos_out, len, + flags); +} +/* * copy_file_range() differs from regular file read and write in that it * specifically allows return partial success. When it does so is up to
@@ -1655,15 +1668,9 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in, } } -if (file_out->f_op->copy_file_range) { - ret = file_out->f_op->copy_file_range (file_in, pos_in, file_out, - pos_out, len, flags); -if (ret != -EOPNOTSUPP ) - goto done; - } - - ret =generic_copy_file_range (file_in, pos_in, file_out, pos_out, len, - flags); + ret =do_copy_file_range (file_in, pos_in, file_out, pos_out, len, + flags); +WARN_ON_ONCE (ret == -EOPNOTSUPP ); done:if (ret >0 ) {fsnotify_access (file_in);
Signature · 1 line
2.17.1
PATCH v4 3/9 vfs: introduce generic_file_rw_checks()
In reply to Amir GoldsteinFactor out helper with some checks on in/out file that are common to clone_file_range and copy_file_range.
--- fs/read_write.c | 38 +++++++++++--------------------------- include/linux/fs.h | 1 + mm/filemap.c | 24 ++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 27 deletions(-)
fs/read_write.c +11 -27
@@ -1617,17 +1617,18 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in, struct file *file_out, loff_t pos_out, size_t len, unsigned int flags) { - struct inode *inode_in = file_inode(file_in); -struct inode *inode_out =file_inode (file_out); ssize_t ret;if (flags !=0 )return -EINVAL ; -if (S_ISDIR (inode_in->i_mode) ||S_ISDIR (inode_out->i_mode)) -return -EISDIR ; -if (!S_ISREG (inode_in->i_mode) || !S_ISREG (inode_out->i_mode)) -return -EINVAL ; +/* this could be relaxed once a method supports cross-fs copies */ +if (file_inode (file_in)->i_sb !=file_inode (file_out)->i_sb) +return -EXDEV ; + + ret =generic_file_rw_checks (file_in, file_out); +if (unlikely (ret)) +return ret; ret =rw_verify_area (READ , file_in, &pos_in, len);if (unlikely (ret))
@@ -1637,15 +1638,6 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,if (unlikely (ret))return ret; -if (!(file_in->f_mode &FMODE_READ ) || - !(file_out->f_mode &FMODE_WRITE ) || - (file_out->f_flags &O_APPEND )) -return -EBADF ; - -/* this could be relaxed once a method supports cross-fs copies */ -if (inode_in->i_sb != inode_out->i_sb) -return -EXDEV ; -if (len ==0 )return 0 ;
@@ -2013,29 +2005,21 @@ loff_t do_clone_file_range(struct file *file_in, loff_t pos_in, struct file *file_out, loff_t pos_out, loff_t len, unsigned int remap_flags) { - struct inode *inode_in = file_inode(file_in); -struct inode *inode_out =file_inode (file_out); loff_t ret;WARN_ON_ONCE (remap_flags &REMAP_FILE_DEDUP ); -if (S_ISDIR (inode_in->i_mode) ||S_ISDIR (inode_out->i_mode)) -return -EISDIR ; -if (!S_ISREG (inode_in->i_mode) || !S_ISREG (inode_out->i_mode)) -return -EINVAL ; -/* * FICLONE/FICLONERANGE ioctls enforce that src and dest files are on * the same mount. Practically, they only need to be on the same file * system. */ -if (inode_in->i_sb != inode_out->i_sb) +if (file_inode (file_in)->i_sb !=file_inode (file_out)->i_sb)return -EXDEV ; -if (!(file_in->f_mode &FMODE_READ ) || - !(file_out->f_mode &FMODE_WRITE ) || - (file_out->f_flags &O_APPEND )) -return -EBADF ; + ret =generic_file_rw_checks (file_in, file_out); +if (ret <0 ) +return ret;if (!file_in->f_op->remap_file_range)return -EOPNOTSUPP ;
include/linux/fs.h +1 -0
@@ -3049,6 +3049,7 @@ extern ssize_t generic_write_checks(struct kiocb *, struct iov_iter *);extern int generic_remap_checks (struct file *file_in,loff_t pos_in,struct file *file_out,loff_t pos_out,loff_t *count,unsigned int remap_flags); +extern int generic_file_rw_checks (struct file *file_in,struct file *file_out);extern ssize_t generic_file_read_iter (struct kiocb *,struct iov_iter *);extern ssize_t __generic_file_write_iter (struct kiocb *,struct iov_iter *);extern ssize_t generic_file_write_iter (struct kiocb *,struct iov_iter *);
mm/filemap.c +24 -0
@@ -3041,6 +3041,30 @@ int generic_remap_checks(struct file *file_in, loff_t pos_in,return 0 ; } + +/* +* Performs common checks before doing a file copy/clone +* from @file_in to @file_out. +*/ +int generic_file_rw_checks (struct file *file_in,struct file *file_out) +{ +struct inode *inode_in =file_inode (file_in); +struct inode *inode_out =file_inode (file_out); + +/* Don't copy dirs, pipes, sockets... */ +if (S_ISDIR (inode_in->i_mode) ||S_ISDIR (inode_out->i_mode)) +return -EISDIR ; +if (!S_ISREG (inode_in->i_mode) || !S_ISREG (inode_out->i_mode)) +return -EINVAL ; + +if (!(file_in->f_mode &FMODE_READ ) || + !(file_out->f_mode &FMODE_WRITE ) || + (file_out->f_flags &O_APPEND )) +return -EBADF ; + +return 0 ; +} +int pagecache_write_begin (struct file *file,struct address_space *mapping,loff_t pos,unsigned len,unsigned flags,struct page **pagep,void **fsdata)
Signature · 1 line
2.17.1
PATCH v4 4/9 vfs: remove redundant checks from generic_remap_checks()
In reply to Amir GoldsteinThe access limit checks on input file range in generic_remap_checks() are redundant because the input file size is guaranteed to be within limits and pos+len are already checked to be within input file size.
Beyond the fact that the check cannot fail, if it would have failed, it could return -EFBIG for input file range error. There is no precedent for that. -EFBIG is returned in syscalls that would change file length.
With that call removed, we can fold generic_access_check_limits() into generic_write_check_limits().
--- mm/filemap.c | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-)
mm/filemap.c +12 -21
@@ -2895,24 +2895,11 @@ EXPORT_SYMBOL(read_cache_page_gfp);* LFS limits. If pos is under the limit it becomes a short access. If it * exceeds the limit we return -EFBIG. */ -static int generic_access_check_limits (struct file *file,loff_t pos, -loff_t *count) -{ -struct inode *inode = file->f_mapping->host; -loff_t max_size = inode->i_sb->s_maxbytes; - -if (!(file->f_flags &O_LARGEFILE )) - max_size =MAX_NON_LFS ; - -if (unlikely (pos >= max_size)) -return -EFBIG ; - *count =min (*count, max_size - pos); -return 0 ; -} -static int generic_write_check_limits (struct file *file,loff_t pos,loff_t *count) { +struct inode *inode = file->f_mapping->host; +loff_t max_size = inode->i_sb->s_maxbytes;loff_t limit =rlimit (RLIMIT_FSIZE );if (limit !=RLIM_INFINITY ) {
@@ -2923,7 +2910,15 @@ static int generic_write_check_limits(struct file *file, loff_t pos, *count =min (*count, limit - pos); } -return generic_access_check_limits (file, pos, count); +if (!(file->f_flags &O_LARGEFILE )) + max_size =MAX_NON_LFS ; + +if (unlikely (pos >= max_size)) +return -EFBIG ; + + *count =min (*count, max_size - pos); + +return 0 ; }/*
@@ -2963,7 +2958,7 @@ EXPORT_SYMBOL(generic_write_checks);/* * Performs necessary checks before doing a clone. * -* Can adjust amount of bytes to clone. +* Can adjust amount of bytes to clone via @req_count argument. * Returns appropriate error code that caller should return or * zero in case the clone should be allowed. */
@@ -3001,10 +2996,6 @@ int generic_remap_checks(struct file *file_in, loff_t pos_in,return -EINVAL ; count =min (count, size_in - (uint64_t )pos_in); - ret =generic_access_check_limits (file_in, pos_in, &count); -if (ret) -return ret; - ret =generic_write_check_limits (file_out, pos_out, &count);if (ret)return ret;
Signature · 1 line
2.17.1
PATCH v4 5/9 vfs: add missing checks to copy_file_range
In reply to Amir GoldsteinLike the clone and dedupe interfaces we've recently fixed, the copy_file_range() implementation is missing basic sanity, limits and boundary condition tests on the parameters that are passed to it from userspace. Create a new "generic_copy_file_checks()" function modelled on the generic_remap_checks() function to provide this missing functionality.
[Amir] Shorten copy length instead of checking pos_in limits because input file size already abides by the limits.
--- fs/read_write.c | 3 ++- include/linux/fs.h | 3 +++ mm/filemap.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-)
fs/read_write.c +2 -1
@@ -1626,7 +1626,8 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,if (file_inode (file_in)->i_sb !=file_inode (file_out)->i_sb)return -EXDEV ; - ret =generic_file_rw_checks (file_in, file_out); + ret =generic_copy_file_checks (file_in, pos_in, file_out, pos_out, &len, + flags);if (unlikely (ret))return ret;
include/linux/fs.h +3 -0
@@ -3050,6 +3050,9 @@ extern int generic_remap_checks(struct file *file_in, loff_t pos_in, struct file *file_out, loff_t pos_out, loff_t *count, unsigned int remap_flags);extern int generic_file_rw_checks (struct file *file_in,struct file *file_out); +extern int generic_copy_file_checks (struct file *file_in,loff_t pos_in, +struct file *file_out,loff_t pos_out, +size_t *count,unsigned int flags);extern ssize_t generic_file_read_iter (struct kiocb *,struct iov_iter *);extern ssize_t __generic_file_write_iter (struct kiocb *,struct iov_iter *);extern ssize_t generic_file_write_iter (struct kiocb *,struct iov_iter *);
mm/filemap.c +53 -0
59 lines not on this page · raw message
Signature · 1 line
2.17.1
PATCH v4 6/9 vfs: introduce file_modified() helper
In reply to Amir GoldsteinThe combination of file_remove_privs() and file_update_mtime() is quite common in filesystem ->write_iter() methods.
Modelled after the helper file_accessed(), introduce file_modified() and use it from generic_remap_file_range_prep().
Note that the order of calling file_remove_privs() before file_update_mtime() in the helper was matched to the more common order by filesystems and not the current order in generic_remap_file_range_prep().
--- fs/inode.c | 20 ++++++++++++++++++++ fs/read_write.c | 21 +++------------------ include/linux/fs.h | 2 ++ 3 files changed, 25 insertions(+), 18 deletions(-)
fs/inode.c +20 -0
@@ -1899,6 +1899,26 @@ int file_update_time(struct file *file) }EXPORT_SYMBOL (file_update_time); +/* Caller must hold the file's inode lock */ +int file_modified (struct file *file) +{ +int err; + +/* +* Clear the security bits if the process is not being run by root. +* This keeps people from modifying setuid and setgid binaries. +*/ + err =file_remove_privs (file); +if (err) +return err; + +if (unlikely (file->f_mode &FMODE_NOCMTIME )) +return 0 ; + +return file_update_time (file); +} +EXPORT_SYMBOL (file_modified); +int inode_needs_sync (struct inode *inode) {if (IS_SYNC (inode))
fs/read_write.c +3 -18
@@ -1980,25 +1980,10 @@ int generic_remap_file_range_prep(struct file *file_in, loff_t pos_in,return ret;/* If can't alter the file contents, we're done. */ -if (!(remap_flags &REMAP_FILE_DEDUP )) { -/* Update the timestamps, since we can alter file contents. */ -if (!(file_out->f_mode &FMODE_NOCMTIME )) { - ret =file_update_time (file_out); -if (ret) -return ret; - } +if (!(remap_flags &REMAP_FILE_DEDUP )) + ret =file_modified (file_out); -/* -* Clear the security bits if the process is not being run by -* root. This keeps people from modifying setuid and setgid -* binaries. -*/ - ret =file_remove_privs (file_out); -if (ret) -return ret; - } - -return 0 ; +return ret; }EXPORT_SYMBOL (generic_remap_file_range_prep);
include/linux/fs.h +2 -0
@@ -2177,6 +2177,8 @@ static inline void file_accessed(struct file *file)touch_atime (&file->f_path); } +extern int file_modified (struct file *file); +int sync_inode (struct inode *inode,struct writeback_control *wbc);int sync_inode_metadata (struct inode *inode,int wait);
Signature · 1 line
2.17.1
PATCH v4 7/9 xfs: use file_modified() helper
In reply to Amir GoldsteinNote that by using the helper, the order of calling file_remove_privs() after file_update_mtime() in xfs_file_aio_write_checks() has changed.
--- fs/xfs/xfs_file.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-)
fs/xfs/xfs_file.c +1 -14
@@ -367,20 +367,7 @@ xfs_file_aio_write_checks(* lock above. Eventually we should look into a way to avoid * the pointless lock roundtrip. */ -if (likely (!(file->f_mode &FMODE_NOCMTIME ))) { - error =file_update_time (file); -if (error) -return error; - } - -/* -* If we're writing the file then make sure to clear the setuid and -* setgid bits if the process is not being run by root. This keeps -* people from modifying setuid and setgid binaries. -*/ -if (!IS_NOSEC (inode)) -return file_remove_privs (file); -return 0 ; +return file_modified (file); } static int
Signature · 1 line
2.17.1
PATCH v4 8/9 vfs: allow copy_file_range to copy across devices
In reply to Amir GoldsteinWe want to enable cross-filesystem copy_file_range functionality where possible, so push the "same superblock only" checks down to the individual filesystem callouts so they can make their own decisions about cross-superblock copy offload and fallack to generic_copy_file_range() for cross-superblock copy.
[Amir] We do not call ->remap_file_range() in case the inodes are not on the same sb and do not call ->copy_file_range() in case the inodes are not on the same filesystem type.
This changes behavior of the copy_file_range(2) syscall, which will now allow cross filesystem in-kernel copy. CIFS already supports cross-superblock copy, between two shares to the same server. This functionality will now be available via the copy_file_range(2) syscall.
--- fs/ceph/file.c | 4 +++- fs/cifs/cifsfs.c | 2 +- fs/fuse/file.c | 5 ++++- fs/nfs/nfs4file.c | 5 ++++- fs/read_write.c | 20 ++++++++++++++------ 5 files changed, 26 insertions(+), 10 deletions(-)
fs/ceph/file.c +3 -1
@@ -1909,6 +1909,8 @@ static ssize_t __ceph_copy_file_range(struct file *src_file, loff_t src_off,if (src_inode == dst_inode)return -EINVAL ; +if (src_inode->i_sb != dst_inode->i_sb) +return -EXDEV ;if (ceph_snap (dst_inode) !=CEPH_NOSNAP )return -EROFS ;
@@ -2109,7 +2111,7 @@ static ssize_t ceph_copy_file_range(struct file *src_file, loff_t src_off, ret =__ceph_copy_file_range (src_file, src_off, dst_file, dst_off, len, flags); -if (ret == -EOPNOTSUPP ) +if (ret == -EOPNOTSUPP || ret == -EXDEV ) ret =generic_copy_file_range (src_file, src_off, dst_file, dst_off, len, flags);return ret;
fs/cifs/cifsfs.c +1 -1
@@ -1149,7 +1149,7 @@ static ssize_t cifs_copy_file_range(struct file *src_file, loff_t off, len, flags);free_xid (xid); -if (rc == -EOPNOTSUPP ) +if (rc == -EOPNOTSUPP || rc == -EXDEV ) rc =generic_copy_file_range (src_file, off, dst_file, destoff, len, flags);return rc;
fs/fuse/file.c +4 -1
@@ -3126,6 +3126,9 @@ static ssize_t __fuse_copy_file_range(struct file *file_in, loff_t pos_in,if (fc->no_copy_file_range)return -EOPNOTSUPP ; +if (file_inode (file_in)->i_sb !=file_inode (file_out)->i_sb) +return -EXDEV ; +inode_lock (inode_out);if (fc->writeback_cache) {
@@ -3182,7 +3185,7 @@ static ssize_t fuse_copy_file_range(struct file *src_file, loff_t src_off, ret =__fuse_copy_file_range (src_file, src_off, dst_file, dst_off, len, flags); -if (ret == -EOPNOTSUPP ) +if (ret == -EOPNOTSUPP || ret == -EXDEV ) ret =generic_copy_file_range (src_file, src_off, dst_file, dst_off, len, flags);return ret;
fs/nfs/nfs4file.c +4 -1
@@ -133,6 +133,9 @@ static ssize_t __nfs4_copy_file_range(struct file *file_in, loff_t pos_in, struct file *file_out, loff_t pos_out, size_t count, unsigned int flags) { + /* Only offload copy if superblock is the same */ + if (file_inode(file_in)->i_sb != file_inode(file_out)->i_sb) + return -EXDEV;if (!nfs_server_capable (file_inode (file_out),NFS_CAP_COPY ))return -EOPNOTSUPP ;if (file_inode (file_in) ==file_inode (file_out))
@@ -148,7 +151,7 @@ static ssize_t nfs4_copy_file_range(struct file *file_in, loff_t pos_in, ret =__nfs4_copy_file_range (file_in, pos_in, file_out, pos_out, count, flags); -if (ret == -EOPNOTSUPP ) +if (ret == -EOPNOTSUPP || ret == -EXDEV ) ret =generic_copy_file_range (file_in, pos_in, file_out, pos_out, count, flags);return ret;
fs/read_write.c +14 -6
@@ -1599,7 +1599,18 @@ static ssize_t do_copy_file_range(struct file *file_in, loff_t pos_in, struct file *file_out, loff_t pos_out, size_t len, unsigned int flags) { - if (file_out->f_op->copy_file_range) +/* +* Although we now allow filesystems to handle cross sb copy, passing +* an inode of the wrong filesystem type to filesystem operation can +* often result in an attempt to dereference the wrong concrete inode +* struct, so avoid doing that until we really have a good reason. +* The incentive for passing inode from different sb to filesystem is +* NFS cross server copy and for that use case, enforcing same +* filesystem type is acceptable. +*/ +if (file_out->f_op->copy_file_range && +file_inode (file_in)->i_sb->s_type == +file_inode (file_out)->i_sb->s_type)return file_out->f_op->copy_file_range (file_in, pos_in, file_out, pos_out, len, flags);
@@ -1622,10 +1633,6 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,if (flags !=0 )return -EINVAL ; -/* this could be relaxed once a method supports cross-fs copies */ -if (file_inode (file_in)->i_sb !=file_inode (file_out)->i_sb) -return -EXDEV ; - ret =generic_copy_file_checks (file_in, pos_in, file_out, pos_out, &len, flags);if (unlikely (ret))
@@ -1648,7 +1655,8 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,* Try cloning first, this is supported by more file systems, and * more efficient if both clone and copy are supported (e.g. NFS). */ -if (file_in->f_op->remap_file_range) { +if (file_in->f_op->remap_file_range && +file_inode (file_in)->i_sb ==file_inode (file_out)->i_sb) {loff_t cloned; cloned = file_in->f_op->remap_file_range(file_in, pos_in,
Signature · 1 line
2.17.1
PATCH v4 9/9 fuse: copy_file_range needs to strip setuid bits and update timestamps
In reply to Amir GoldsteinLike ->write_iter(), we update mtime and strip setuid of dst file before copy and like ->read_iter(), we update atime of src file after copy.
--- fs/fuse/file.c | 5 +++++ 1 file changed, 5 insertions(+)
fs/fuse/file.c +5 -0
@@ -3131,6 +3131,10 @@ static ssize_t __fuse_copy_file_range(struct file *file_in, loff_t pos_in,inode_lock (inode_out); + err =file_modified (file_out); +if (err) + goto out; +if (fc->writeback_cache) { err =filemap_write_and_wait_range (inode_out->i_mapping, pos_out, pos_out + len);
@@ -3172,6 +3176,7 @@ static ssize_t __fuse_copy_file_range(struct file *file_in, loff_t pos_in,clear_bit (FUSE_I_SIZE_UNSTABLE , &fi_out->state);inode_unlock (inode_out); +file_accessed (file_in);return err; }
Signature · 1 line
2.17.1