linux-kernel - Re: [PATCH] fuse: Move same-superblock check to fuse_copy_file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAOQ4uxho4_3-Ue=QSngdsgQQNSA9VbAZw6wP=+U3xoc2jx3dcg@mail.gmail.com>
Date: Fri, 8 Aug 2025 11:55:11 +0200
From: Amir Goldstein <amir73il@...il.com>
To: Chunsheng Luo <luochunsheng@...c.edu>
Cc: luis@...lia.com, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, miklos@...redi.hu, bernd@...ernd.com
Subject: Re: [PATCH] fuse: Move same-superblock check to fuse_copy_file_range

On Thu, Aug 7, 2025 at 2:49 PM Chunsheng Luo <luochunsheng@...c.edu> 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.