On Thu, Aug 7, 2025 at 2:49 PM Chunsheng Luo <luochunsheng@xxxxxxxx> wrote:
>
> On Thu, Aug 07 2025, Luis Henriques wrote:
>
> >> The copy_file_range COPY_FILE_SPLICE capability allows filesystems to
> >> handle cross-superblock copy. However, in the current fuse implementation,
> >> __fuse_copy_file_range accesses src_file->private_data under the assumption
> >> that it points to a fuse_file structure. When the source file belongs to a
> >> non-FUSE filesystem, it will leads to kernel panics.
> >
> > I wonder if you have actually seen this kernel panic happening. It seems
> > like the code you're moving into fuse_copy_file_range() shouldn't be
> > needed as the same check is already done in generic_copy_file_checks()
> > (which is called from vfs_copy_file_range()).
> >
> > Either way, I think your change to fuse_copy_file_range() could be
> > simplified with something like:
> >
> > ssize_t ret = -EXDEV;
> >
> > if (file_inode(src_file)->i_sb == file_inode(dst_file)->i_sb)
> > ret = __fuse_copy_file_range(src_file, src_off, dst_file, dst_off,
> > len, flags);
> >
> > if (ret == -EOPNOTSUPP || ret == -EXDEV)
> > ret = splice_copy_file_range(src_file, src_off, dst_file,
> > dst_off, len);
> >
> > But again, my understanding is that this should never happen in practice
> > and that the superblock check could even be removed from
> > __fuse_copy_file_range().
> >
> > Cheers,
> > --
> > Luís
> >
>
> Yes, now copy_file_range won't crash.
>
> generic_copy_file_checks:
> /*
> * We allow some filesystems to handle cross sb copy, but 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 and cifs define several different file_system_type structures
> * and several different sets of file_operations, but they all end up
> * using the same ->copy_file_range() function pointer.
> */
> if (flags & COPY_FILE_SPLICE) {
> /* cross sb splice is allowed */
> } else if (file_out->f_op->copy_file_range) {
> if (file_in->f_op->copy_file_range !=
> file_out->f_op->copy_file_range)
> return -EXDEV;
> } else if (file_inode(file_in)->i_sb != file_inode(file_out)->i_sb) {
> return -EXDEV;
> }
>
> generic_copy_file_checks mentions that now allows some filesystems to handle cross-sb copy.
>
> code:
> } else if (file_out->f_op->copy_file_range) {
> if (file_in->f_op->copy_file_range !=
> file_out->f_op->copy_file_range)
> return -EXDEV;
>
> If the same filesystem is satisfied but the sb is not same, it will go to fuse_copy_file_range,
> so fuse_copy_file_range needs to handle this situation.
>
> Sorry, there is an mistake with my patch log description. __fuse_copy_file_range does not exist that
> the source file is a NON-Fuse filesystem, so It can safely use ->private_data.
>
> Therefore, this patch does not need.
Indeed, this patch makes no sense and does not change any logic at all.
Thanks,
Amir.
