> On Mar 3, 2017, at 3:47 PM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote: > > On Thu, Mar 02, 2017 at 08:22:21AM -0800, Christoph Hellwig wrote: >> On Thu, Mar 02, 2017 at 11:01:05AM -0500, Olga Kornievskaia wrote: >>> + if (pos_in >= i_size_read(inode_in)) >>> + return -EINVAL; >> >> That's not how the syscall is supposed to work, we'd rather do a >> short read^^^^^copy. > > That's what I think too, but then is COPY(2) wrong?: > > EINVAL Requested range extends beyond the end of the source > file; or the flags argument is not 0. > > Also, copy_file_range can be implemented by ->clone_file_range, where > these kinds of checks make more sense, I think; e.g. from btrfs: > > ret = -EINVAL; > if (off + len > src->i_size || off + len < off) > goto out_unlock; > > Well, so the caller just has to be prepared for either behavior, I > guess, but that may make it more complicated to use. > I’m still rather very confused again by the comment and what it is proposing. There are two checks to consider for the validity of the arguments 1. If the offset of the source file is beyond the end of the source file. 2. If the offset + len is beyond the end of the file. I read that the man page is talking about #2. This is actually what the NFSv4.2 spec required for the COPY too but we’ve been discussing that it should be a short read instead. This patch address is to address case #1. As far as I can tell it applies to all file systems. Are you suggesting that the checks for the validity of the arguments do not belong in VFS but instead should be in each of the underlying file systems? Not all vfs_copy_file_range() are implemented via clone_file_range(). At least I hope that “inter” NFSv4.2 COPY will also use vfs_file_copy_range() and it would not be calling clone(). > --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
