Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 14 Nov 2023 13:10:14 -0800
From:      Rick Macklem <rick.macklem@gmail.com>
To:        Alexander Motin <mav@freebsd.org>
Cc:        Mateusz Guzik <mjguzik@gmail.com>, Ronald Klop <ronald-lists@klop.ws>,  Konstantin Belousov <kostikbel@gmail.com>, current@freebsd.org
Subject:   Re: crash zfs_clone_range()
Message-ID:  <CAM5tNy5D3QJksxJv5Wzp63dQLn=K7_53-_TZ79ugb%2BheAntHXg@mail.gmail.com>
In-Reply-To: <ca2b13ee-7144-96ce-78a2-118c7982de23@FreeBSD.org>
References:  <349700057.3452.1699611152405@localhost> <c9c8ab33-efce-5ed0-1f3f-311fa3cf1338@FreeBSD.org> <ZVEdyHFJyTg0cqCo@kib.kiev.ua> <1900239445.5968.1699966796547@localhost> <CAGudoHGdhaea9mkF3RZSCgXuEGNesb9AtkLXrYQNncgreYsv=g@mail.gmail.com> <ea3b2421-a07c-e7c2-68eb-908185dbb98f@FreeBSD.org> <ca2b13ee-7144-96ce-78a2-118c7982de23@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Nov 14, 2023 at 10:46=E2=80=AFAM 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 !=3D outvp->v_mount->mnt_vfc) {
> >> +               goto bad_write_fallback;
> >> +       }
> >> +
> >>          if (invp =3D=3D outvp) {
> >>                  if (vn_lock(outvp, LK_EXCLUSIVE) !=3D 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 =3D len;
> >          if (inmp =3D=3D outmp || strcmp(inmp->mnt_vfc->vfc_name,
> >              outmp->mnt_vfc->vfc_name) =3D=3D 0)
> >                  error =3D VOP_COPY_FILE_RANGE(invp, inoffp, outvp, out=
offp,
> >                      lenp, flags, incred, outcred, fsize_td);
> >          else
> >                  error =3D vn_generic_copy_file_range(invp, inoffp, out=
vp,
> >                      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?
Although it would be nice to do the check before the VOP call, I don't
see an easy way to do that.

It looks like the simple solution is to add a check in each of the
VOP_COPY_FILE_RANGE() calls, such as mjg@ has proposed
for ZFS. At this point there is only the three and I can easily do the
NFS one.

rick

>
> --
> Alexander Motin
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAM5tNy5D3QJksxJv5Wzp63dQLn=K7_53-_TZ79ugb%2BheAntHXg>