On Tue, May 28, 2019 at 7:31 PM Darrick J. Wong <darrick.wong@xxxxxxxxxx> wrote: > > On Tue, May 28, 2019 at 07:26:29PM +0300, Amir Goldstein wrote: > > On Tue, May 28, 2019 at 7:18 PM Darrick J. Wong <darrick.wong@xxxxxxxxxx> wrote: > > > > > > On Sun, May 26, 2019 at 09:10:55AM +0300, Amir Goldstein wrote: > > > > Like 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. > > > > > > > > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> > > > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > > > > --- > > > > fs/read_write.c | 3 ++- > > > > include/linux/fs.h | 3 +++ > > > > mm/filemap.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++ > > > > 3 files changed, 58 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/fs/read_write.c b/fs/read_write.c > > > > index f1900bdb3127..b0fb1176b628 100644 > > > > --- a/fs/read_write.c > > > > +++ b/fs/read_write.c > > > > @@ -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; > > > > > > > > diff --git a/include/linux/fs.h b/include/linux/fs.h > > > > index 89b9b73eb581..e4d382c4342a 100644 > > > > --- a/include/linux/fs.h > > > > +++ b/include/linux/fs.h > > > > @@ -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 *); > > > > diff --git a/mm/filemap.c b/mm/filemap.c > > > > index 798aac92cd76..1852fbf08eeb 100644 > > > > --- a/mm/filemap.c > > > > +++ b/mm/filemap.c > > > > @@ -3064,6 +3064,59 @@ int generic_file_rw_checks(struct file *file_in, struct file *file_out) > > > > return 0; > > > > } > > > > > > > > +/* > > > > + * Performs necessary checks before doing a file copy > > > > + * > > > > + * Can adjust amount of bytes to copy > > > > > > That's the @req_count parameter, correct? > > > > Correct. Same as generic_remap_checks() > > Ok. Would you mind updating the comment? > OK. Thanks, Amir.