Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On Fri, Apr 9, 2021 at 4:39 PM Luis Henriques wrote: > > Nicolas Boichat writes: > > > On Wed, Feb 24, 2021 at 6:44 PM Nicolas Boichat > > wrote: > >> > >> On Wed, Feb 24, 2021 at 6:22 PM Luis Henriques wrote: > >> > > >> > On Tue, Feb 23, 2021 at 08:00:54PM -0500, Olga Kornievskaia wrote: > >> > > On Mon, Feb 22, 2021 at 5:25 AM Luis Henriques > >> > > wrote: > >> > > > > >> > > > A regression has been reported by Nicolas Boichat, found while using > >> > > > the > >> > > > copy_file_range syscall to copy a tracefs file. Before commit > >> > > > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") > >> > > > the > >> > > > kernel would return -EXDEV to userspace when trying to copy a file > >> > > > across > >> > > > different filesystems. After this commit, the syscall doesn't fail > >> > > > anymore > >> > > > and instead returns zero (zero bytes copied), as this file's content > >> > > > is > >> > > > generated on-the-fly and thus reports a size of zero. > >> > > > > >> > > > This patch restores some cross-filesystem copy restrictions that > >> > > > existed > >> > > > prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy > >> > > > across > >> > > > devices"). Filesystems are still allowed to fall-back to the VFS > >> > > > generic_copy_file_range() implementation, but that has now to be done > >> > > > explicitly. > >> > > > > >> > > > nfsd is also modified to fall-back into generic_copy_file_range() in > >> > > > case > >> > > > vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. > >> > > > > >> > > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across > >> > > > devices") > >> > > > Link: > >> > > > https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/ > >> > > > Link: > >> > > > https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx+BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/ > >> > > > Link: > >> > > > https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/ > >> > > > Reported-by: Nicolas Boichat > >> > > > Signed-off-by: Luis Henriques > >> > > > >> > > I tested v8 and I believe it works for NFS. > >> > > >> > Thanks a lot for the testing. And to everyone else for reviews, > >> > feedback,... and patience. > >> > >> Thanks so much to you!!! > >> > >> Works here, you can add my > >> Tested-by: Nicolas Boichat > > > > What happened to this patch? It does not seem to have been picked up > > yet? Any reason why? > > Hmm... good question. I'm not actually sure who would be picking it. Al, > maybe...? > Darrick, Would you mind taking this through your tree in case Al doesn't pick it up? Thanks, Amir.
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
Nicolas Boichat writes: > On Wed, Feb 24, 2021 at 6:44 PM Nicolas Boichat wrote: >> >> On Wed, Feb 24, 2021 at 6:22 PM Luis Henriques wrote: >> > >> > On Tue, Feb 23, 2021 at 08:00:54PM -0500, Olga Kornievskaia wrote: >> > > On Mon, Feb 22, 2021 at 5:25 AM Luis Henriques >> > > wrote: >> > > > >> > > > A regression has been reported by Nicolas Boichat, found while using >> > > > the >> > > > copy_file_range syscall to copy a tracefs file. Before commit >> > > > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the >> > > > kernel would return -EXDEV to userspace when trying to copy a file >> > > > across >> > > > different filesystems. After this commit, the syscall doesn't fail >> > > > anymore >> > > > and instead returns zero (zero bytes copied), as this file's content is >> > > > generated on-the-fly and thus reports a size of zero. >> > > > >> > > > This patch restores some cross-filesystem copy restrictions that >> > > > existed >> > > > prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy >> > > > across >> > > > devices"). Filesystems are still allowed to fall-back to the VFS >> > > > generic_copy_file_range() implementation, but that has now to be done >> > > > explicitly. >> > > > >> > > > nfsd is also modified to fall-back into generic_copy_file_range() in >> > > > case >> > > > vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. >> > > > >> > > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across >> > > > devices") >> > > > Link: >> > > > https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/ >> > > > Link: >> > > > https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx+BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/ >> > > > Link: >> > > > https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/ >> > > > Reported-by: Nicolas Boichat >> > > > Signed-off-by: Luis Henriques >> > > >> > > I tested v8 and I believe it works for NFS. >> > >> > Thanks a lot for the testing. And to everyone else for reviews, >> > feedback,... and patience. >> >> Thanks so much to you!!! >> >> Works here, you can add my >> Tested-by: Nicolas Boichat > > What happened to this patch? It does not seem to have been picked up > yet? Any reason why? Hmm... good question. I'm not actually sure who would be picking it. Al, maybe...? Cheers, -- Luis > >> > >> > I'll now go look into the manpage and see what needs to be changed. >> > >> > Cheers, >> > -- >> > Luís
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On Wed, Feb 24, 2021 at 6:44 PM Nicolas Boichat wrote: > > On Wed, Feb 24, 2021 at 6:22 PM Luis Henriques wrote: > > > > On Tue, Feb 23, 2021 at 08:00:54PM -0500, Olga Kornievskaia wrote: > > > On Mon, Feb 22, 2021 at 5:25 AM Luis Henriques wrote: > > > > > > > > A regression has been reported by Nicolas Boichat, found while using the > > > > copy_file_range syscall to copy a tracefs file. Before commit > > > > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the > > > > kernel would return -EXDEV to userspace when trying to copy a file > > > > across > > > > different filesystems. After this commit, the syscall doesn't fail > > > > anymore > > > > and instead returns zero (zero bytes copied), as this file's content is > > > > generated on-the-fly and thus reports a size of zero. > > > > > > > > This patch restores some cross-filesystem copy restrictions that existed > > > > prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across > > > > devices"). Filesystems are still allowed to fall-back to the VFS > > > > generic_copy_file_range() implementation, but that has now to be done > > > > explicitly. > > > > > > > > nfsd is also modified to fall-back into generic_copy_file_range() in > > > > case > > > > vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. > > > > > > > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across > > > > devices") > > > > Link: > > > > https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/ > > > > Link: > > > > https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx+BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/ > > > > Link: > > > > https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/ > > > > Reported-by: Nicolas Boichat > > > > Signed-off-by: Luis Henriques > > > > > > I tested v8 and I believe it works for NFS. > > > > Thanks a lot for the testing. And to everyone else for reviews, > > feedback,... and patience. > > Thanks so much to you!!! > > Works here, you can add my > Tested-by: Nicolas Boichat What happened to this patch? It does not seem to have been picked up yet? Any reason why? > > > > I'll now go look into the manpage and see what needs to be changed. > > > > Cheers, > > -- > > Luís
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On Wed, Feb 24, 2021 at 6:22 PM Luis Henriques wrote: > > On Tue, Feb 23, 2021 at 08:00:54PM -0500, Olga Kornievskaia wrote: > > On Mon, Feb 22, 2021 at 5:25 AM Luis Henriques wrote: > > > > > > A regression has been reported by Nicolas Boichat, found while using the > > > copy_file_range syscall to copy a tracefs file. Before commit > > > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the > > > kernel would return -EXDEV to userspace when trying to copy a file across > > > different filesystems. After this commit, the syscall doesn't fail > > > anymore > > > and instead returns zero (zero bytes copied), as this file's content is > > > generated on-the-fly and thus reports a size of zero. > > > > > > This patch restores some cross-filesystem copy restrictions that existed > > > prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across > > > devices"). Filesystems are still allowed to fall-back to the VFS > > > generic_copy_file_range() implementation, but that has now to be done > > > explicitly. > > > > > > nfsd is also modified to fall-back into generic_copy_file_range() in case > > > vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. > > > > > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") > > > Link: > > > https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/ > > > Link: > > > https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx+BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/ > > > Link: > > > https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/ > > > Reported-by: Nicolas Boichat > > > Signed-off-by: Luis Henriques > > > > I tested v8 and I believe it works for NFS. > > Thanks a lot for the testing. And to everyone else for reviews, > feedback,... and patience. Thanks so much to you!!! Works here, you can add my Tested-by: Nicolas Boichat > > I'll now go look into the manpage and see what needs to be changed. > > Cheers, > -- > Luís
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On Tue, Feb 23, 2021 at 08:00:54PM -0500, Olga Kornievskaia wrote: > On Mon, Feb 22, 2021 at 5:25 AM Luis Henriques wrote: > > > > A regression has been reported by Nicolas Boichat, found while using the > > copy_file_range syscall to copy a tracefs file. Before commit > > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the > > kernel would return -EXDEV to userspace when trying to copy a file across > > different filesystems. After this commit, the syscall doesn't fail anymore > > and instead returns zero (zero bytes copied), as this file's content is > > generated on-the-fly and thus reports a size of zero. > > > > This patch restores some cross-filesystem copy restrictions that existed > > prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across > > devices"). Filesystems are still allowed to fall-back to the VFS > > generic_copy_file_range() implementation, but that has now to be done > > explicitly. > > > > nfsd is also modified to fall-back into generic_copy_file_range() in case > > vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. > > > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") > > Link: > > https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/ > > Link: > > https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx+BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/ > > Link: > > https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/ > > Reported-by: Nicolas Boichat > > Signed-off-by: Luis Henriques > > I tested v8 and I believe it works for NFS. Thanks a lot for the testing. And to everyone else for reviews, feedback,... and patience. I'll now go look into the manpage and see what needs to be changed. Cheers, -- Luís
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On 2/23/21 9:33 AM, Amir Goldstein wrote: On Tue, Feb 23, 2021 at 7:31 PM wrote: On 2/23/21 8:57 AM, dai@oracle.com wrote: On 2/23/21 8:47 AM, Amir Goldstein wrote: On Tue, Feb 23, 2021 at 6:02 PM wrote: On 2/23/21 7:29 AM, dai@oracle.com wrote: On 2/23/21 2:32 AM, Luis Henriques wrote: On Mon, Feb 22, 2021 at 08:25:27AM -0800, dai@oracle.com wrote: On 2/22/21 2:24 AM, Luis Henriques wrote: A regression has been reported by Nicolas Boichat, found while using the copy_file_range syscall to copy a tracefs file. Before commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the kernel would return -EXDEV to userspace when trying to copy a file across different filesystems. After this commit, the syscall doesn't fail anymore and instead returns zero (zero bytes copied), as this file's content is generated on-the-fly and thus reports a size of zero. This patch restores some cross-filesystem copy restrictions that existed prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices"). Filesystems are still allowed to fall-back to the VFS generic_copy_file_range() implementation, but that has now to be done explicitly. nfsd is also modified to fall-back into generic_copy_file_range() in case vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmi49dC6w$ Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx*BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/__;Kw!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmgCmMHzA$ Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmzqItkrQ$ Reported-by: Nicolas Boichat Signed-off-by: Luis Henriques --- Changes since v7 - set 'ret' to '-EOPNOTSUPP' before the clone 'if' statement so that the error returned is always related to the 'copy' operation Changes since v6 - restored i_sb checks for the clone operation Changes since v5 - check if ->copy_file_range is NULL before calling it Changes since v4 - nfsd falls-back to generic_copy_file_range() only *if* it gets -EOPNOTSUPP or -EXDEV. Changes since v3 - dropped the COPY_FILE_SPLICE flag - kept the f_op's checks early in generic_copy_file_checks, implementing Amir's suggestions - modified nfsd to use generic_copy_file_range() Changes since v2 - do all the required checks earlier, in generic_copy_file_checks(), adding new checks for ->remap_file_range - new COPY_FILE_SPLICE flag - don't remove filesystem's fallback to generic_copy_file_range() - updated commit changelog (and subject) Changes since v1 (after Amir review) - restored do_copy_file_range() helper - return -EOPNOTSUPP if fs doesn't implement CFR - updated commit description fs/nfsd/vfs.c | 8 +++- fs/read_write.c | 49 - 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 04937e51de56..23dab0fa9087 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -568,6 +568,7 @@ __be32 nfsd4_clone_file_range(struct nfsd_file *nf_src, u64 src_pos, ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst, u64 dst_pos, u64 count) { +ssize_t ret; /* * Limit copy to 4MB to prevent indefinitely blocking an nfsd @@ -578,7 +579,12 @@ ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst, * limit like this and pipeline multiple COPY requests. */ count = min_t(u64, count, 1 << 22); -return vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); +ret = vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); + +if (ret == -EOPNOTSUPP || ret == -EXDEV) +ret = generic_copy_file_range(src, src_pos, dst, dst_pos, + count, 0); +return ret; } __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh *fhp, diff --git a/fs/read_write.c b/fs/read_write.c index 75f764b43418..5a26297fd410 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -1388,28 +1388,6 @@ 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) -{ -/* - * Although we now allow filesystems to handle cross sb copy, passing - * a file of the wrong filesystem type to filesystem driver can result - * in
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On Mon, Feb 22, 2021 at 5:25 AM Luis Henriques wrote: > > A regression has been reported by Nicolas Boichat, found while using the > copy_file_range syscall to copy a tracefs file. Before commit > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the > kernel would return -EXDEV to userspace when trying to copy a file across > different filesystems. After this commit, the syscall doesn't fail anymore > and instead returns zero (zero bytes copied), as this file's content is > generated on-the-fly and thus reports a size of zero. > > This patch restores some cross-filesystem copy restrictions that existed > prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across > devices"). Filesystems are still allowed to fall-back to the VFS > generic_copy_file_range() implementation, but that has now to be done > explicitly. > > nfsd is also modified to fall-back into generic_copy_file_range() in case > vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") > Link: > https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/ > Link: > https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx+BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/ > Link: > https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/ > Reported-by: Nicolas Boichat > Signed-off-by: Luis Henriques I tested v8 and I believe it works for NFS. > --- > Changes since v7 > - set 'ret' to '-EOPNOTSUPP' before the clone 'if' statement so that the > error returned is always related to the 'copy' operation > Changes since v6 > - restored i_sb checks for the clone operation > Changes since v5 > - check if ->copy_file_range is NULL before calling it > Changes since v4 > - nfsd falls-back to generic_copy_file_range() only *if* it gets -EOPNOTSUPP > or -EXDEV. > Changes since v3 > - dropped the COPY_FILE_SPLICE flag > - kept the f_op's checks early in generic_copy_file_checks, implementing > Amir's suggestions > - modified nfsd to use generic_copy_file_range() > Changes since v2 > - do all the required checks earlier, in generic_copy_file_checks(), > adding new checks for ->remap_file_range > - new COPY_FILE_SPLICE flag > - don't remove filesystem's fallback to generic_copy_file_range() > - updated commit changelog (and subject) > Changes since v1 (after Amir review) > - restored do_copy_file_range() helper > - return -EOPNOTSUPP if fs doesn't implement CFR > - updated commit description > > fs/nfsd/vfs.c | 8 +++- > fs/read_write.c | 49 - > 2 files changed, 31 insertions(+), 26 deletions(-) > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c > index 04937e51de56..23dab0fa9087 100644 > --- a/fs/nfsd/vfs.c > +++ b/fs/nfsd/vfs.c > @@ -568,6 +568,7 @@ __be32 nfsd4_clone_file_range(struct nfsd_file *nf_src, > u64 src_pos, > ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst, > u64 dst_pos, u64 count) > { > + ssize_t ret; > > /* > * Limit copy to 4MB to prevent indefinitely blocking an nfsd > @@ -578,7 +579,12 @@ ssize_t nfsd_copy_file_range(struct file *src, u64 > src_pos, struct file *dst, > * limit like this and pipeline multiple COPY requests. > */ > count = min_t(u64, count, 1 << 22); > - return vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); > + ret = vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); > + > + if (ret == -EOPNOTSUPP || ret == -EXDEV) > + ret = generic_copy_file_range(src, src_pos, dst, dst_pos, > + count, 0); > + return ret; > } > > __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh *fhp, > diff --git a/fs/read_write.c b/fs/read_write.c > index 75f764b43418..5a26297fd410 100644 > --- a/fs/read_write.c > +++ b/fs/read_write.c > @@ -1388,28 +1388,6 @@ 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) > -{ > - /* > -* Although we now allow filesystems to handle cross sb copy, passing > -* a file of the wrong filesystem type to filesystem driver can result > -* in an attempt to dereference the wrong type of ->private_data, so > -* avoid doing that until we really have a good reason. NFS defines > -* several different file_system_type structures, but they all end up > -* using the same ->copy_file_range() function pointer. > -*/ > - if (file_out->f_op->copy_file_range && > - file_out->f_op->copy_file_range == file_in
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On Tue, Feb 23, 2021 at 08:57:38AM -0800, dai@oracle.com wrote: > > On 2/23/21 8:47 AM, Amir Goldstein wrote: > > On Tue, Feb 23, 2021 at 6:02 PM wrote: > > > > > > On 2/23/21 7:29 AM, dai@oracle.com wrote: > > > > On 2/23/21 2:32 AM, Luis Henriques wrote: > > > > > On Mon, Feb 22, 2021 at 08:25:27AM -0800, dai@oracle.com wrote: > > > > > > On 2/22/21 2:24 AM, Luis Henriques wrote: > > > > > > > A regression has been reported by Nicolas Boichat, found while > > > > > > > using the > > > > > > > copy_file_range syscall to copy a tracefs file. Before commit > > > > > > > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across > > > > > > > devices") the > > > > > > > kernel would return -EXDEV to userspace when trying to copy a file > > > > > > > across > > > > > > > different filesystems. After this commit, the syscall doesn't > > > > > > > fail > > > > > > > anymore > > > > > > > and instead returns zero (zero bytes copied), as this file's > > > > > > > content is > > > > > > > generated on-the-fly and thus reports a size of zero. > > > > > > > > > > > > > > This patch restores some cross-filesystem copy restrictions that > > > > > > > existed > > > > > > > prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy > > > > > > > across > > > > > > > devices"). Filesystems are still allowed to fall-back to the VFS > > > > > > > generic_copy_file_range() implementation, but that has now to be > > > > > > > done > > > > > > > explicitly. > > > > > > > > > > > > > > nfsd is also modified to fall-back into generic_copy_file_range() > > > > > > > in case > > > > > > > vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. > > > > > > > > > > > > > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across > > > > > > > devices") > > > > > > > Link: > > > > > > > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmi49dC6w$ > > > > > > > Link: > > > > > > > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx*BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/__;Kw!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmgCmMHzA$ > > > > > > > Link: > > > > > > > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmzqItkrQ$ > > > > > > > Reported-by: Nicolas Boichat > > > > > > > Signed-off-by: Luis Henriques > > > > > > > --- > > > > > > > Changes since v7 > > > > > > > - set 'ret' to '-EOPNOTSUPP' before the clone 'if' statement so > > > > > > > that the > > > > > > > error returned is always related to the 'copy' operation > > > > > > > Changes since v6 > > > > > > > - restored i_sb checks for the clone operation > > > > > > > Changes since v5 > > > > > > > - check if ->copy_file_range is NULL before calling it > > > > > > > Changes since v4 > > > > > > > - nfsd falls-back to generic_copy_file_range() only *if* it gets > > > > > > > -EOPNOTSUPP > > > > > > > or -EXDEV. > > > > > > > Changes since v3 > > > > > > > - dropped the COPY_FILE_SPLICE flag > > > > > > > - kept the f_op's checks early in generic_copy_file_checks, > > > > > > > implementing > > > > > > > Amir's suggestions > > > > > > > - modified nfsd to use generic_copy_file_range() > > > > > > > Changes since v2 > > > > > > > - do all the required checks earlier, in > > > > > > > generic_copy_file_checks(), > > > > > > > adding new checks for ->remap_file_range > > > > > > > - new COPY_FILE_SPLICE flag > > > > > > > - don't remove filesystem's fallback to generic_copy_file_range() > > > > > > > - updated commit changelog (and subject) > > > > > > > Changes since v1 (after Amir review) > > > > > > > - restored do_copy_file_range() helper > > > > > > > - return -EOPNOTSUPP if fs doesn't implement CFR > > > > > > > - updated commit description > > > > > > > > > > > > > > fs/nfsd/vfs.c | 8 +++- > > > > > > > fs/read_write.c | 49 > > > > > > > - > > > > > > > 2 files changed, 31 insertions(+), 26 deletions(-) > > > > > > > > > > > > > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c > > > > > > > index 04937e51de56..23dab0fa9087 100644 > > > > > > > --- a/fs/nfsd/vfs.c > > > > > > > +++ b/fs/nfsd/vfs.c > > > > > > > @@ -568,6 +568,7 @@ __be32 nfsd4_clone_file_range(struct nfsd_file > > > > > > > *nf_src, u64 src_pos, > > > > > > > ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, > > > > > > > struct file *dst, > > > > > > > u64 dst_pos, u64 count) > > > > > > > { > > > > > > > +ssize_t ret; > > > > > > > /* > > > > > > > * Limit copy to 4MB to prevent indefinitely blocking an > > > > > > > nfsd > > > > > > > @@ -578,7 +579,12 @@ ssize_t nfsd_copy_
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On Tue, Feb 23, 2021 at 7:31 PM wrote: > > On 2/23/21 8:57 AM, dai@oracle.com wrote: > > > On 2/23/21 8:47 AM, Amir Goldstein wrote: > > On Tue, Feb 23, 2021 at 6:02 PM wrote: > > > On 2/23/21 7:29 AM, dai@oracle.com wrote: > > On 2/23/21 2:32 AM, Luis Henriques wrote: > > On Mon, Feb 22, 2021 at 08:25:27AM -0800, dai@oracle.com wrote: > > On 2/22/21 2:24 AM, Luis Henriques wrote: > > A regression has been reported by Nicolas Boichat, found while > using the > copy_file_range syscall to copy a tracefs file. Before commit > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the > kernel would return -EXDEV to userspace when trying to copy a file > across > different filesystems. After this commit, the syscall doesn't fail > anymore > and instead returns zero (zero bytes copied), as this file's > content is > generated on-the-fly and thus reports a size of zero. > > This patch restores some cross-filesystem copy restrictions that > existed > prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy > across > devices"). Filesystems are still allowed to fall-back to the VFS > generic_copy_file_range() implementation, but that has now to be done > explicitly. > > nfsd is also modified to fall-back into generic_copy_file_range() > in case > vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across > devices") > Link: > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmi49dC6w$ > Link: > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx*BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/__;Kw!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmgCmMHzA$ > Link: > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmzqItkrQ$ > Reported-by: Nicolas Boichat > Signed-off-by: Luis Henriques > --- > Changes since v7 > - set 'ret' to '-EOPNOTSUPP' before the clone 'if' statement so > that the > error returned is always related to the 'copy' operation > Changes since v6 > - restored i_sb checks for the clone operation > Changes since v5 > - check if ->copy_file_range is NULL before calling it > Changes since v4 > - nfsd falls-back to generic_copy_file_range() only *if* it gets > -EOPNOTSUPP > or -EXDEV. > Changes since v3 > - dropped the COPY_FILE_SPLICE flag > - kept the f_op's checks early in generic_copy_file_checks, > implementing > Amir's suggestions > - modified nfsd to use generic_copy_file_range() > Changes since v2 > - do all the required checks earlier, in generic_copy_file_checks(), > adding new checks for ->remap_file_range > - new COPY_FILE_SPLICE flag > - don't remove filesystem's fallback to generic_copy_file_range() > - updated commit changelog (and subject) > Changes since v1 (after Amir review) > - restored do_copy_file_range() helper > - return -EOPNOTSUPP if fs doesn't implement CFR > - updated commit description > > fs/nfsd/vfs.c | 8 +++- > fs/read_write.c | 49 > - > 2 files changed, 31 insertions(+), 26 deletions(-) > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c > index 04937e51de56..23dab0fa9087 100644 > --- a/fs/nfsd/vfs.c > +++ b/fs/nfsd/vfs.c > @@ -568,6 +568,7 @@ __be32 nfsd4_clone_file_range(struct nfsd_file > *nf_src, u64 src_pos, > ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, > struct file *dst, > u64 dst_pos, u64 count) > { > +ssize_t ret; > /* > * Limit copy to 4MB to prevent indefinitely blocking an nfsd > @@ -578,7 +579,12 @@ ssize_t nfsd_copy_file_range(struct file *src, > u64 src_pos, struct file *dst, > * limit like this and pipeline multiple COPY requests. > */ > count = min_t(u64, count, 1 << 22); > -return vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); > +ret = vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); > + > +if (ret == -EOPNOTSUPP || ret == -EXDEV) > +ret = generic_copy_file_range(src, src_pos, dst, dst_pos, > + count, 0); > +return ret; > } > __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh > *fhp, > diff --git a/fs/read_write.c b/fs/read_write.c > index 75f764b43418..5a26297fd410 100644 > --- a/fs/read_write.c > +++ b/fs/read_write.c > @@ -1388,28 +1388,6 @@ 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
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On Tue, Feb 23, 2021 at 11:03 AM wrote: > > > On 2/23/21 7:29 AM, dai@oracle.com wrote: > > > > On 2/23/21 2:32 AM, Luis Henriques wrote: > >> On Mon, Feb 22, 2021 at 08:25:27AM -0800, dai@oracle.com wrote: > >>> On 2/22/21 2:24 AM, Luis Henriques wrote: > A regression has been reported by Nicolas Boichat, found while > using the > copy_file_range syscall to copy a tracefs file. Before commit > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the > kernel would return -EXDEV to userspace when trying to copy a file > across > different filesystems. After this commit, the syscall doesn't fail > anymore > and instead returns zero (zero bytes copied), as this file's > content is > generated on-the-fly and thus reports a size of zero. > > This patch restores some cross-filesystem copy restrictions that > existed > prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy > across > devices"). Filesystems are still allowed to fall-back to the VFS > generic_copy_file_range() implementation, but that has now to be done > explicitly. > > nfsd is also modified to fall-back into generic_copy_file_range() > in case > vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across > devices") > Link: > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmi49dC6w$ > Link: > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx*BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/__;Kw!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmgCmMHzA$ > Link: > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmzqItkrQ$ > Reported-by: Nicolas Boichat > Signed-off-by: Luis Henriques > --- > Changes since v7 > - set 'ret' to '-EOPNOTSUPP' before the clone 'if' statement so > that the > error returned is always related to the 'copy' operation > Changes since v6 > - restored i_sb checks for the clone operation > Changes since v5 > - check if ->copy_file_range is NULL before calling it > Changes since v4 > - nfsd falls-back to generic_copy_file_range() only *if* it gets > -EOPNOTSUPP > or -EXDEV. > Changes since v3 > - dropped the COPY_FILE_SPLICE flag > - kept the f_op's checks early in generic_copy_file_checks, > implementing > Amir's suggestions > - modified nfsd to use generic_copy_file_range() > Changes since v2 > - do all the required checks earlier, in generic_copy_file_checks(), > adding new checks for ->remap_file_range > - new COPY_FILE_SPLICE flag > - don't remove filesystem's fallback to generic_copy_file_range() > - updated commit changelog (and subject) > Changes since v1 (after Amir review) > - restored do_copy_file_range() helper > - return -EOPNOTSUPP if fs doesn't implement CFR > - updated commit description > > fs/nfsd/vfs.c | 8 +++- > fs/read_write.c | 49 > - > 2 files changed, 31 insertions(+), 26 deletions(-) > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c > index 04937e51de56..23dab0fa9087 100644 > --- a/fs/nfsd/vfs.c > +++ b/fs/nfsd/vfs.c > @@ -568,6 +568,7 @@ __be32 nfsd4_clone_file_range(struct nfsd_file > *nf_src, u64 src_pos, > ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, > struct file *dst, > u64 dst_pos, u64 count) > { > +ssize_t ret; > /* > * Limit copy to 4MB to prevent indefinitely blocking an nfsd > @@ -578,7 +579,12 @@ ssize_t nfsd_copy_file_range(struct file *src, > u64 src_pos, struct file *dst, > * limit like this and pipeline multiple COPY requests. > */ > count = min_t(u64, count, 1 << 22); > -return vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); > +ret = vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); > + > +if (ret == -EOPNOTSUPP || ret == -EXDEV) > +ret = generic_copy_file_range(src, src_pos, dst, dst_pos, > + count, 0); > +return ret; > } > __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh > *fhp, > diff --git a/fs/read_write.c b/fs/read_write.c > index 75f764b43418..5a26297fd410 100644 > --- a/fs/read_write.c > +++ b/f
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On 2/23/21 8:47 AM, Amir Goldstein wrote: On Tue, Feb 23, 2021 at 6:02 PM wrote: On 2/23/21 7:29 AM, dai@oracle.com wrote: On 2/23/21 2:32 AM, Luis Henriques wrote: On Mon, Feb 22, 2021 at 08:25:27AM -0800, dai@oracle.com wrote: On 2/22/21 2:24 AM, Luis Henriques wrote: A regression has been reported by Nicolas Boichat, found while using the copy_file_range syscall to copy a tracefs file. Before commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the kernel would return -EXDEV to userspace when trying to copy a file across different filesystems. After this commit, the syscall doesn't fail anymore and instead returns zero (zero bytes copied), as this file's content is generated on-the-fly and thus reports a size of zero. This patch restores some cross-filesystem copy restrictions that existed prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices"). Filesystems are still allowed to fall-back to the VFS generic_copy_file_range() implementation, but that has now to be done explicitly. nfsd is also modified to fall-back into generic_copy_file_range() in case vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmi49dC6w$ Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx*BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/__;Kw!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmgCmMHzA$ Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmzqItkrQ$ Reported-by: Nicolas Boichat Signed-off-by: Luis Henriques --- Changes since v7 - set 'ret' to '-EOPNOTSUPP' before the clone 'if' statement so that the error returned is always related to the 'copy' operation Changes since v6 - restored i_sb checks for the clone operation Changes since v5 - check if ->copy_file_range is NULL before calling it Changes since v4 - nfsd falls-back to generic_copy_file_range() only *if* it gets -EOPNOTSUPP or -EXDEV. Changes since v3 - dropped the COPY_FILE_SPLICE flag - kept the f_op's checks early in generic_copy_file_checks, implementing Amir's suggestions - modified nfsd to use generic_copy_file_range() Changes since v2 - do all the required checks earlier, in generic_copy_file_checks(), adding new checks for ->remap_file_range - new COPY_FILE_SPLICE flag - don't remove filesystem's fallback to generic_copy_file_range() - updated commit changelog (and subject) Changes since v1 (after Amir review) - restored do_copy_file_range() helper - return -EOPNOTSUPP if fs doesn't implement CFR - updated commit description fs/nfsd/vfs.c | 8 +++- fs/read_write.c | 49 - 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 04937e51de56..23dab0fa9087 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -568,6 +568,7 @@ __be32 nfsd4_clone_file_range(struct nfsd_file *nf_src, u64 src_pos, ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst, u64 dst_pos, u64 count) { +ssize_t ret; /* * Limit copy to 4MB to prevent indefinitely blocking an nfsd @@ -578,7 +579,12 @@ ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst, * limit like this and pipeline multiple COPY requests. */ count = min_t(u64, count, 1 << 22); -return vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); +ret = vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); + +if (ret == -EOPNOTSUPP || ret == -EXDEV) +ret = generic_copy_file_range(src, src_pos, dst, dst_pos, + count, 0); +return ret; } __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh *fhp, diff --git a/fs/read_write.c b/fs/read_write.c index 75f764b43418..5a26297fd410 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -1388,28 +1388,6 @@ 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) -{ -/* - * Although we now allow filesystems to handle cross sb copy, passing - * a file of the wrong filesystem type to filesystem driver can result - * in an attempt to dereference the wrong type of ->private_data, so - * avoid doing that until we really have a good reason. NFS defines - * sev
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On Tue, Feb 23, 2021 at 6:02 PM wrote: > > > On 2/23/21 7:29 AM, dai@oracle.com wrote: > > > > On 2/23/21 2:32 AM, Luis Henriques wrote: > >> On Mon, Feb 22, 2021 at 08:25:27AM -0800, dai@oracle.com wrote: > >>> On 2/22/21 2:24 AM, Luis Henriques wrote: > A regression has been reported by Nicolas Boichat, found while > using the > copy_file_range syscall to copy a tracefs file. Before commit > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the > kernel would return -EXDEV to userspace when trying to copy a file > across > different filesystems. After this commit, the syscall doesn't fail > anymore > and instead returns zero (zero bytes copied), as this file's > content is > generated on-the-fly and thus reports a size of zero. > > This patch restores some cross-filesystem copy restrictions that > existed > prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy > across > devices"). Filesystems are still allowed to fall-back to the VFS > generic_copy_file_range() implementation, but that has now to be done > explicitly. > > nfsd is also modified to fall-back into generic_copy_file_range() > in case > vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across > devices") > Link: > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmi49dC6w$ > Link: > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx*BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/__;Kw!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmgCmMHzA$ > Link: > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmzqItkrQ$ > Reported-by: Nicolas Boichat > Signed-off-by: Luis Henriques > --- > Changes since v7 > - set 'ret' to '-EOPNOTSUPP' before the clone 'if' statement so > that the > error returned is always related to the 'copy' operation > Changes since v6 > - restored i_sb checks for the clone operation > Changes since v5 > - check if ->copy_file_range is NULL before calling it > Changes since v4 > - nfsd falls-back to generic_copy_file_range() only *if* it gets > -EOPNOTSUPP > or -EXDEV. > Changes since v3 > - dropped the COPY_FILE_SPLICE flag > - kept the f_op's checks early in generic_copy_file_checks, > implementing > Amir's suggestions > - modified nfsd to use generic_copy_file_range() > Changes since v2 > - do all the required checks earlier, in generic_copy_file_checks(), > adding new checks for ->remap_file_range > - new COPY_FILE_SPLICE flag > - don't remove filesystem's fallback to generic_copy_file_range() > - updated commit changelog (and subject) > Changes since v1 (after Amir review) > - restored do_copy_file_range() helper > - return -EOPNOTSUPP if fs doesn't implement CFR > - updated commit description > > fs/nfsd/vfs.c | 8 +++- > fs/read_write.c | 49 > - > 2 files changed, 31 insertions(+), 26 deletions(-) > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c > index 04937e51de56..23dab0fa9087 100644 > --- a/fs/nfsd/vfs.c > +++ b/fs/nfsd/vfs.c > @@ -568,6 +568,7 @@ __be32 nfsd4_clone_file_range(struct nfsd_file > *nf_src, u64 src_pos, > ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, > struct file *dst, > u64 dst_pos, u64 count) > { > +ssize_t ret; > /* > * Limit copy to 4MB to prevent indefinitely blocking an nfsd > @@ -578,7 +579,12 @@ ssize_t nfsd_copy_file_range(struct file *src, > u64 src_pos, struct file *dst, > * limit like this and pipeline multiple COPY requests. > */ > count = min_t(u64, count, 1 << 22); > -return vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); > +ret = vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); > + > +if (ret == -EOPNOTSUPP || ret == -EXDEV) > +ret = generic_copy_file_range(src, src_pos, dst, dst_pos, > + count, 0); > +return ret; > } > __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh > *fhp, > diff --git a/fs/read_write.c b/fs/read_write.c > index 75f764b43418..5a26297fd410 100644 > --- a/fs/read_write.c > +++ b/fs
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On 2/23/21 7:29 AM, dai@oracle.com wrote: On 2/23/21 2:32 AM, Luis Henriques wrote: On Mon, Feb 22, 2021 at 08:25:27AM -0800, dai@oracle.com wrote: On 2/22/21 2:24 AM, Luis Henriques wrote: A regression has been reported by Nicolas Boichat, found while using the copy_file_range syscall to copy a tracefs file. Before commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the kernel would return -EXDEV to userspace when trying to copy a file across different filesystems. After this commit, the syscall doesn't fail anymore and instead returns zero (zero bytes copied), as this file's content is generated on-the-fly and thus reports a size of zero. This patch restores some cross-filesystem copy restrictions that existed prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices"). Filesystems are still allowed to fall-back to the VFS generic_copy_file_range() implementation, but that has now to be done explicitly. nfsd is also modified to fall-back into generic_copy_file_range() in case vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmi49dC6w$ Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx*BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/__;Kw!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmgCmMHzA$ Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmzqItkrQ$ Reported-by: Nicolas Boichat Signed-off-by: Luis Henriques --- Changes since v7 - set 'ret' to '-EOPNOTSUPP' before the clone 'if' statement so that the error returned is always related to the 'copy' operation Changes since v6 - restored i_sb checks for the clone operation Changes since v5 - check if ->copy_file_range is NULL before calling it Changes since v4 - nfsd falls-back to generic_copy_file_range() only *if* it gets -EOPNOTSUPP or -EXDEV. Changes since v3 - dropped the COPY_FILE_SPLICE flag - kept the f_op's checks early in generic_copy_file_checks, implementing Amir's suggestions - modified nfsd to use generic_copy_file_range() Changes since v2 - do all the required checks earlier, in generic_copy_file_checks(), adding new checks for ->remap_file_range - new COPY_FILE_SPLICE flag - don't remove filesystem's fallback to generic_copy_file_range() - updated commit changelog (and subject) Changes since v1 (after Amir review) - restored do_copy_file_range() helper - return -EOPNOTSUPP if fs doesn't implement CFR - updated commit description fs/nfsd/vfs.c | 8 +++- fs/read_write.c | 49 - 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 04937e51de56..23dab0fa9087 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -568,6 +568,7 @@ __be32 nfsd4_clone_file_range(struct nfsd_file *nf_src, u64 src_pos, ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst, u64 dst_pos, u64 count) { + ssize_t ret; /* * Limit copy to 4MB to prevent indefinitely blocking an nfsd @@ -578,7 +579,12 @@ ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst, * limit like this and pipeline multiple COPY requests. */ count = min_t(u64, count, 1 << 22); - return vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); + ret = vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); + + if (ret == -EOPNOTSUPP || ret == -EXDEV) + ret = generic_copy_file_range(src, src_pos, dst, dst_pos, + count, 0); + return ret; } __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh *fhp, diff --git a/fs/read_write.c b/fs/read_write.c index 75f764b43418..5a26297fd410 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -1388,28 +1388,6 @@ 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) -{ - /* - * Although we now allow filesystems to handle cross sb copy, passing - * a file of the wrong filesystem type to filesystem driver can result - * in an attempt to dereference the wrong type of ->private_data, so - * avoid doing that until we really have a good reason. NFS defines - * several different file_system_type structures, but they all end up
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On 2/23/21 2:32 AM, Luis Henriques wrote: On Mon, Feb 22, 2021 at 08:25:27AM -0800, dai@oracle.com wrote: On 2/22/21 2:24 AM, Luis Henriques wrote: A regression has been reported by Nicolas Boichat, found while using the copy_file_range syscall to copy a tracefs file. Before commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the kernel would return -EXDEV to userspace when trying to copy a file across different filesystems. After this commit, the syscall doesn't fail anymore and instead returns zero (zero bytes copied), as this file's content is generated on-the-fly and thus reports a size of zero. This patch restores some cross-filesystem copy restrictions that existed prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices"). Filesystems are still allowed to fall-back to the VFS generic_copy_file_range() implementation, but that has now to be done explicitly. nfsd is also modified to fall-back into generic_copy_file_range() in case vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmi49dC6w$ Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx*BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/__;Kw!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmgCmMHzA$ Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmzqItkrQ$ Reported-by: Nicolas Boichat Signed-off-by: Luis Henriques --- Changes since v7 - set 'ret' to '-EOPNOTSUPP' before the clone 'if' statement so that the error returned is always related to the 'copy' operation Changes since v6 - restored i_sb checks for the clone operation Changes since v5 - check if ->copy_file_range is NULL before calling it Changes since v4 - nfsd falls-back to generic_copy_file_range() only *if* it gets -EOPNOTSUPP or -EXDEV. Changes since v3 - dropped the COPY_FILE_SPLICE flag - kept the f_op's checks early in generic_copy_file_checks, implementing Amir's suggestions - modified nfsd to use generic_copy_file_range() Changes since v2 - do all the required checks earlier, in generic_copy_file_checks(), adding new checks for ->remap_file_range - new COPY_FILE_SPLICE flag - don't remove filesystem's fallback to generic_copy_file_range() - updated commit changelog (and subject) Changes since v1 (after Amir review) - restored do_copy_file_range() helper - return -EOPNOTSUPP if fs doesn't implement CFR - updated commit description fs/nfsd/vfs.c | 8 +++- fs/read_write.c | 49 - 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 04937e51de56..23dab0fa9087 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -568,6 +568,7 @@ __be32 nfsd4_clone_file_range(struct nfsd_file *nf_src, u64 src_pos, ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst, u64 dst_pos, u64 count) { + ssize_t ret; /* * Limit copy to 4MB to prevent indefinitely blocking an nfsd @@ -578,7 +579,12 @@ ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst, * limit like this and pipeline multiple COPY requests. */ count = min_t(u64, count, 1 << 22); - return vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); + ret = vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); + + if (ret == -EOPNOTSUPP || ret == -EXDEV) + ret = generic_copy_file_range(src, src_pos, dst, dst_pos, + count, 0); + return ret; } __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh *fhp, diff --git a/fs/read_write.c b/fs/read_write.c index 75f764b43418..5a26297fd410 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -1388,28 +1388,6 @@ 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) -{ - /* -* Although we now allow filesystems to handle cross sb copy, passing -* a file of the wrong filesystem type to filesystem driver can result -* in an attempt to dereference the wrong type of ->private_data, so -* avoid doing that until we really have a good reason. NFS defines -* several different file_system_type structures
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On Tue, Feb 23, 2021 at 12:31 PM Luis Henriques wrote: > > On Mon, Feb 22, 2021 at 08:25:27AM -0800, dai@oracle.com wrote: > > > > On 2/22/21 2:24 AM, Luis Henriques wrote: > > > A regression has been reported by Nicolas Boichat, found while using the > > > copy_file_range syscall to copy a tracefs file. Before commit > > > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the > > > kernel would return -EXDEV to userspace when trying to copy a file across > > > different filesystems. After this commit, the syscall doesn't fail > > > anymore > > > and instead returns zero (zero bytes copied), as this file's content is > > > generated on-the-fly and thus reports a size of zero. > > > > > > This patch restores some cross-filesystem copy restrictions that existed > > > prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across > > > devices"). Filesystems are still allowed to fall-back to the VFS > > > generic_copy_file_range() implementation, but that has now to be done > > > explicitly. > > > > > > nfsd is also modified to fall-back into generic_copy_file_range() in case > > > vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. > > > > > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") > > > Link: > > > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmi49dC6w$ > > > Link: > > > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx*BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/__;Kw!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmgCmMHzA$ > > > Link: > > > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmzqItkrQ$ > > > Reported-by: Nicolas Boichat > > > Signed-off-by: Luis Henriques > > > --- > > > Changes since v7 > > > - set 'ret' to '-EOPNOTSUPP' before the clone 'if' statement so that the > > >error returned is always related to the 'copy' operation > > > Changes since v6 > > > - restored i_sb checks for the clone operation > > > Changes since v5 > > > - check if ->copy_file_range is NULL before calling it > > > Changes since v4 > > > - nfsd falls-back to generic_copy_file_range() only *if* it gets > > > -EOPNOTSUPP > > >or -EXDEV. > > > Changes since v3 > > > - dropped the COPY_FILE_SPLICE flag > > > - kept the f_op's checks early in generic_copy_file_checks, implementing > > >Amir's suggestions > > > - modified nfsd to use generic_copy_file_range() > > > Changes since v2 > > > - do all the required checks earlier, in generic_copy_file_checks(), > > >adding new checks for ->remap_file_range > > > - new COPY_FILE_SPLICE flag > > > - don't remove filesystem's fallback to generic_copy_file_range() > > > - updated commit changelog (and subject) > > > Changes since v1 (after Amir review) > > > - restored do_copy_file_range() helper > > > - return -EOPNOTSUPP if fs doesn't implement CFR > > > - updated commit description > > > > > > fs/nfsd/vfs.c | 8 +++- > > > fs/read_write.c | 49 - > > > 2 files changed, 31 insertions(+), 26 deletions(-) > > > > > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c > > > index 04937e51de56..23dab0fa9087 100644 > > > --- a/fs/nfsd/vfs.c > > > +++ b/fs/nfsd/vfs.c > > > @@ -568,6 +568,7 @@ __be32 nfsd4_clone_file_range(struct nfsd_file > > > *nf_src, u64 src_pos, > > > ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file > > > *dst, > > > u64 dst_pos, u64 count) > > > { > > > + ssize_t ret; > > > /* > > > * Limit copy to 4MB to prevent indefinitely blocking an nfsd > > > @@ -578,7 +579,12 @@ ssize_t nfsd_copy_file_range(struct file *src, u64 > > > src_pos, struct file *dst, > > > * limit like this and pipeline multiple COPY requests. > > > */ > > > count = min_t(u64, count, 1 << 22); > > > - return vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); > > > + ret = vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); > > > + > > > + if (ret == -EOPNOTSUPP || ret == -EXDEV) > > > + ret = generic_copy_file_range(src, src_pos, dst, dst_pos, > > > + count, 0); > > > + return ret; > > > } > > > __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh *fhp, > > > diff --git a/fs/read_write.c b/fs/read_write.c > > > index 75f764b43418..5a26297fd410 100644 > > > --- a/fs/read_write.c > > > +++ b/fs/read_write.c > > > @@ -1388,28 +1388,6 @@ 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_i
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On Mon, Feb 22, 2021 at 08:25:27AM -0800, dai@oracle.com wrote: > > On 2/22/21 2:24 AM, Luis Henriques wrote: > > A regression has been reported by Nicolas Boichat, found while using the > > copy_file_range syscall to copy a tracefs file. Before commit > > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the > > kernel would return -EXDEV to userspace when trying to copy a file across > > different filesystems. After this commit, the syscall doesn't fail anymore > > and instead returns zero (zero bytes copied), as this file's content is > > generated on-the-fly and thus reports a size of zero. > > > > This patch restores some cross-filesystem copy restrictions that existed > > prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across > > devices"). Filesystems are still allowed to fall-back to the VFS > > generic_copy_file_range() implementation, but that has now to be done > > explicitly. > > > > nfsd is also modified to fall-back into generic_copy_file_range() in case > > vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. > > > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") > > Link: > > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmi49dC6w$ > > Link: > > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx*BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/__;Kw!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmgCmMHzA$ > > Link: > > https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmzqItkrQ$ > > Reported-by: Nicolas Boichat > > Signed-off-by: Luis Henriques > > --- > > Changes since v7 > > - set 'ret' to '-EOPNOTSUPP' before the clone 'if' statement so that the > >error returned is always related to the 'copy' operation > > Changes since v6 > > - restored i_sb checks for the clone operation > > Changes since v5 > > - check if ->copy_file_range is NULL before calling it > > Changes since v4 > > - nfsd falls-back to generic_copy_file_range() only *if* it gets -EOPNOTSUPP > >or -EXDEV. > > Changes since v3 > > - dropped the COPY_FILE_SPLICE flag > > - kept the f_op's checks early in generic_copy_file_checks, implementing > >Amir's suggestions > > - modified nfsd to use generic_copy_file_range() > > Changes since v2 > > - do all the required checks earlier, in generic_copy_file_checks(), > >adding new checks for ->remap_file_range > > - new COPY_FILE_SPLICE flag > > - don't remove filesystem's fallback to generic_copy_file_range() > > - updated commit changelog (and subject) > > Changes since v1 (after Amir review) > > - restored do_copy_file_range() helper > > - return -EOPNOTSUPP if fs doesn't implement CFR > > - updated commit description > > > > fs/nfsd/vfs.c | 8 +++- > > fs/read_write.c | 49 - > > 2 files changed, 31 insertions(+), 26 deletions(-) > > > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c > > index 04937e51de56..23dab0fa9087 100644 > > --- a/fs/nfsd/vfs.c > > +++ b/fs/nfsd/vfs.c > > @@ -568,6 +568,7 @@ __be32 nfsd4_clone_file_range(struct nfsd_file *nf_src, > > u64 src_pos, > > ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file > > *dst, > > u64 dst_pos, u64 count) > > { > > + ssize_t ret; > > /* > > * Limit copy to 4MB to prevent indefinitely blocking an nfsd > > @@ -578,7 +579,12 @@ ssize_t nfsd_copy_file_range(struct file *src, u64 > > src_pos, struct file *dst, > > * limit like this and pipeline multiple COPY requests. > > */ > > count = min_t(u64, count, 1 << 22); > > - return vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); > > + ret = vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); > > + > > + if (ret == -EOPNOTSUPP || ret == -EXDEV) > > + ret = generic_copy_file_range(src, src_pos, dst, dst_pos, > > + count, 0); > > + return ret; > > } > > __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh *fhp, > > diff --git a/fs/read_write.c b/fs/read_write.c > > index 75f764b43418..5a26297fd410 100644 > > --- a/fs/read_write.c > > +++ b/fs/read_write.c > > @@ -1388,28 +1388,6 @@ 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) > > -{ > > - /* > > -* Although we now allow filesystems to handle cross sb copy, passing > > -* a file of
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On 2/22/21 2:24 AM, Luis Henriques wrote: A regression has been reported by Nicolas Boichat, found while using the copy_file_range syscall to copy a tracefs file. Before commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the kernel would return -EXDEV to userspace when trying to copy a file across different filesystems. After this commit, the syscall doesn't fail anymore and instead returns zero (zero bytes copied), as this file's content is generated on-the-fly and thus reports a size of zero. This patch restores some cross-filesystem copy restrictions that existed prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices"). Filesystems are still allowed to fall-back to the VFS generic_copy_file_range() implementation, but that has now to be done explicitly. nfsd is also modified to fall-back into generic_copy_file_range() in case vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmi49dC6w$ Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx*BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/__;Kw!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmgCmMHzA$ Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/__;!!GqivPVa7Brio!P1UWThiSkxbjfjFQWNYJmCxGEkiLFyvHjH6cS-G1ZTt1z-TeqwGQgQmzqItkrQ$ Reported-by: Nicolas Boichat Signed-off-by: Luis Henriques --- Changes since v7 - set 'ret' to '-EOPNOTSUPP' before the clone 'if' statement so that the error returned is always related to the 'copy' operation Changes since v6 - restored i_sb checks for the clone operation Changes since v5 - check if ->copy_file_range is NULL before calling it Changes since v4 - nfsd falls-back to generic_copy_file_range() only *if* it gets -EOPNOTSUPP or -EXDEV. Changes since v3 - dropped the COPY_FILE_SPLICE flag - kept the f_op's checks early in generic_copy_file_checks, implementing Amir's suggestions - modified nfsd to use generic_copy_file_range() Changes since v2 - do all the required checks earlier, in generic_copy_file_checks(), adding new checks for ->remap_file_range - new COPY_FILE_SPLICE flag - don't remove filesystem's fallback to generic_copy_file_range() - updated commit changelog (and subject) Changes since v1 (after Amir review) - restored do_copy_file_range() helper - return -EOPNOTSUPP if fs doesn't implement CFR - updated commit description fs/nfsd/vfs.c | 8 +++- fs/read_write.c | 49 - 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 04937e51de56..23dab0fa9087 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -568,6 +568,7 @@ __be32 nfsd4_clone_file_range(struct nfsd_file *nf_src, u64 src_pos, ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst, u64 dst_pos, u64 count) { + ssize_t ret; /* * Limit copy to 4MB to prevent indefinitely blocking an nfsd @@ -578,7 +579,12 @@ ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst, * limit like this and pipeline multiple COPY requests. */ count = min_t(u64, count, 1 << 22); - return vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); + ret = vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); + + if (ret == -EOPNOTSUPP || ret == -EXDEV) + ret = generic_copy_file_range(src, src_pos, dst, dst_pos, + count, 0); + return ret; } __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh *fhp, diff --git a/fs/read_write.c b/fs/read_write.c index 75f764b43418..5a26297fd410 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -1388,28 +1388,6 @@ 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) -{ - /* -* Although we now allow filesystems to handle cross sb copy, passing -* a file of the wrong filesystem type to filesystem driver can result -* in an attempt to dereference the wrong type of ->private_data, so -* avoid doing that until we really have a good reason. NFS defines -* several different file_system_type structures, but they all end up -* using the same ->copy_file_range() function pointer. -*/ - if (file_out
Re: [PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
On Mon, Feb 22, 2021 at 12:23 PM Luis Henriques wrote: > > A regression has been reported by Nicolas Boichat, found while using the > copy_file_range syscall to copy a tracefs file. Before commit > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the > kernel would return -EXDEV to userspace when trying to copy a file across > different filesystems. After this commit, the syscall doesn't fail anymore > and instead returns zero (zero bytes copied), as this file's content is > generated on-the-fly and thus reports a size of zero. > > This patch restores some cross-filesystem copy restrictions that existed > prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across > devices"). Filesystems are still allowed to fall-back to the VFS > generic_copy_file_range() implementation, but that has now to be done > explicitly. > > nfsd is also modified to fall-back into generic_copy_file_range() in case > vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") > Link: > https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/ > Link: > https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx+BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/ > Link: > https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/ > Reported-by: Nicolas Boichat > Signed-off-by: Luis Henriques > --- Reviewed-by: Amir Goldstein Thanks, Amir.
[PATCH v8] vfs: fix copy_file_range regression in cross-fs copies
A regression has been reported by Nicolas Boichat, found while using the copy_file_range syscall to copy a tracefs file. Before commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the kernel would return -EXDEV to userspace when trying to copy a file across different filesystems. After this commit, the syscall doesn't fail anymore and instead returns zero (zero bytes copied), as this file's content is generated on-the-fly and thus reports a size of zero. This patch restores some cross-filesystem copy restrictions that existed prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices"). Filesystems are still allowed to fall-back to the VFS generic_copy_file_range() implementation, but that has now to be done explicitly. nfsd is also modified to fall-back into generic_copy_file_range() in case vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV. Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") Link: https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drink...@chromium.org/ Link: https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx+BnvW=zmpsg47cyhfjwnw7zs...@mail.gmail.com/ Link: https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/ Reported-by: Nicolas Boichat Signed-off-by: Luis Henriques --- Changes since v7 - set 'ret' to '-EOPNOTSUPP' before the clone 'if' statement so that the error returned is always related to the 'copy' operation Changes since v6 - restored i_sb checks for the clone operation Changes since v5 - check if ->copy_file_range is NULL before calling it Changes since v4 - nfsd falls-back to generic_copy_file_range() only *if* it gets -EOPNOTSUPP or -EXDEV. Changes since v3 - dropped the COPY_FILE_SPLICE flag - kept the f_op's checks early in generic_copy_file_checks, implementing Amir's suggestions - modified nfsd to use generic_copy_file_range() Changes since v2 - do all the required checks earlier, in generic_copy_file_checks(), adding new checks for ->remap_file_range - new COPY_FILE_SPLICE flag - don't remove filesystem's fallback to generic_copy_file_range() - updated commit changelog (and subject) Changes since v1 (after Amir review) - restored do_copy_file_range() helper - return -EOPNOTSUPP if fs doesn't implement CFR - updated commit description fs/nfsd/vfs.c | 8 +++- fs/read_write.c | 49 - 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 04937e51de56..23dab0fa9087 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -568,6 +568,7 @@ __be32 nfsd4_clone_file_range(struct nfsd_file *nf_src, u64 src_pos, ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst, u64 dst_pos, u64 count) { + ssize_t ret; /* * Limit copy to 4MB to prevent indefinitely blocking an nfsd @@ -578,7 +579,12 @@ ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst, * limit like this and pipeline multiple COPY requests. */ count = min_t(u64, count, 1 << 22); - return vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); + ret = vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0); + + if (ret == -EOPNOTSUPP || ret == -EXDEV) + ret = generic_copy_file_range(src, src_pos, dst, dst_pos, + count, 0); + return ret; } __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh *fhp, diff --git a/fs/read_write.c b/fs/read_write.c index 75f764b43418..5a26297fd410 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -1388,28 +1388,6 @@ 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) -{ - /* -* Although we now allow filesystems to handle cross sb copy, passing -* a file of the wrong filesystem type to filesystem driver can result -* in an attempt to dereference the wrong type of ->private_data, so -* avoid doing that until we really have a good reason. NFS defines -* several different file_system_type structures, but they all end up -* using the same ->copy_file_range() function pointer. -*/ - if (file_out->f_op->copy_file_range && - file_out->f_op->copy_file_range == file_in->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, p