Re: crash zfs_clone_range()

On Tue, Nov 14, 2023 at 07:51:39PM +0100, Mateusz Guzik wrote:
> On 11/14/23, Alexander Motin <mav@freebsd.org> wrote:
> > On 14.11.2023 12:44, Alexander Motin wrote:
> >> On 14.11.2023 12:39, Mateusz Guzik wrote:
> >>> One of the vnodes is probably not zfs, I suspect this will do it
> >>> (untested):
> >>>
> >>> diff --git a/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops_os.c
> >>> b/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops_os.c
> >>> index 107cd69c756c..e799a7091b8e 100644
> >>> --- a/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops_os.c
> >>> +++ b/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops_os.c
> >>> @@ -6270,6 +6270,11 @@ zfs_freebsd_copy_file_range(struct
> >>> vop_copy_file_range_args *ap)
> >>>                          goto bad_write_fallback;
> >>>                  }
> >>>          }
> >>> +
> >>> +       if (invp->v_mount->mnt_vfc != outvp->v_mount->mnt_vfc) {
> >>> +               goto bad_write_fallback;
> >>> +       }
> >>> +
> >>>          if (invp == outvp) {
> >>>                  if (vn_lock(outvp, LK_EXCLUSIVE) != 0) {
> >>>                          goto bad_write_fallback;
> >>>
> >>
> >> vn_copy_file_range() verifies for that:
> >>
> >>          /*
> >>           * If the two vnodes are for the same file system type, call
> >>           * VOP_COPY_FILE_RANGE(), otherwise call
> >> vn_generic_copy_file_range()
> >>           * which can handle copies across multiple file system types.
> >>           */
> >>          *lenp = len;
> >>          if (inmp == outmp || strcmp(inmp->mnt_vfc->vfc_name,
> >>              outmp->mnt_vfc->vfc_name) == 0)
> >>                  error = VOP_COPY_FILE_RANGE(invp, inoffp, outvp,
> >> outoffp,
> >>                      lenp, flags, incred, outcred, fsize_td);
> >>          else
> >>                  error = vn_generic_copy_file_range(invp, inoffp, outvp,
> >>                      outoffp, lenp, flags, incred, outcred, fsize_td);
> >
> > Thinking again, what happen if there are two nullfs mounts on top of two
> > different file systems, one of which is indeed not ZFS?  Do we need to
> > add those checks to all ZFS, NFS and FUSE, implementing
> > VOP_COPY_FILE_RANGE, or it is responsibility of nullfs or VFS?
> >
> 
> I already advocated for not trying to guess for filesystems what they
> can or cannot handle internally.
> 
> That is to say vn_copy_file_range should call VOP_COPY_FILE_RANGE,
> that can try to figure out what to do and if it got nothing punt to a
> fallback. This already happens for some of the cases.
> 
It is nullfs that is to blame there.
See https://reviews.freebsd.org/D42603