Date: Tue, 14 Nov 2023 22:15:34 +0100 From: Mateusz Guzik <mjguzik@gmail.com> To: Rick Macklem <rick.macklem@gmail.com> Cc: Alexander Motin <mav@freebsd.org>, Ronald Klop <ronald-lists@klop.ws>, Konstantin Belousov <kostikbel@gmail.com>, current@freebsd.org Subject: Re: crash zfs_clone_range() Message-ID: <CAGudoHHCnNw9HsHL0WGsu7CTX-7mEyOcPXoiBtg6LzvfbKAncA@mail.gmail.com> In-Reply-To: <CAM5tNy5D3QJksxJv5Wzp63dQLn=K7_53-_TZ79ugb%2BheAntHXg@mail.gmail.com> 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> <CAM5tNy5D3QJksxJv5Wzp63dQLn=K7_53-_TZ79ugb%2BheAntHXg@mail.gmail.com>
index | next in thread | previous in thread | raw e-mail
On 11/14/23, Rick Macklem <rick.macklem@gmail.com> wrote:
> On Tue, Nov 14, 2023 at 10:46 AM 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?
> 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.
>
All filesystems except for zfs are already covered because they check
for mismatched mount.
--
Mateusz Guzik <mjguzik gmail.com>
help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHHCnNw9HsHL0WGsu7CTX-7mEyOcPXoiBtg6LzvfbKAncA>