From 9b76db0857a8a32bc0d3643e12ca098e1a6a25cc Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Thu, 28 Oct 2021 11:58:57 -0400 Subject: [PATCH 1/2] Fix potential use-after-frees in FreeBSD getpages and setattr VOPs The objset object is reallocated during certain dataset operations, such as rollbacks, so the objset pointer must be loaded after acquiring the teardown lock. Signed-off-by: Mark Johnston --- module/os/freebsd/zfs/zfs_vnops_os.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/module/os/freebsd/zfs/zfs_vnops_os.c b/module/os/freebsd/zfs/zfs_vnops_os.c index e4a15e6d9ea3..322d64f01f16 100644 --- a/module/os/freebsd/zfs/zfs_vnops_os.c +++ b/module/os/freebsd/zfs/zfs_vnops_os.c @@ -2208,7 +2208,7 @@ zfs_setattr(znode_t *zp, vattr_t *vap, int flags, cred_t *cr) { vnode_t *vp = ZTOV(zp); zfsvfs_t *zfsvfs = zp->z_zfsvfs; - objset_t *os = zfsvfs->z_os; + objset_t *os; zilog_t *zilog; dmu_tx_t *tx; vattr_t oldva; @@ -2243,6 +2243,7 @@ zfs_setattr(znode_t *zp, vattr_t *vap, int flags, cred_t *cr) ZFS_ENTER(zfsvfs); ZFS_VERIFY_ZP(zp); + os = zfsvfs->z_os; zilog = zfsvfs->z_log; /* @@ -4046,7 +4047,6 @@ zfs_getpages(struct vnode *vp, vm_page_t *ma, int count, int *rbehind, { znode_t *zp = VTOZ(vp); zfsvfs_t *zfsvfs = zp->z_zfsvfs; - objset_t *os = zp->z_zfsvfs->z_os; zfs_locked_range_t *lr; vm_object_t object; off_t start, end, obj_size; @@ -4116,8 +4116,8 @@ zfs_getpages(struct vnode *vp, vm_page_t *ma, int count, int *rbehind, * ZFS will panic if we request DMU to read beyond the end of the last * allocated block. */ - error = dmu_read_pages(os, zp->z_id, ma, count, &pgsin_b, &pgsin_a, - MIN(end, obj_size) - (end - PAGE_SIZE)); + error = dmu_read_pages(zfsvfs->z_os, zp->z_id, ma, count, &pgsin_b, + &pgsin_a, MIN(end, obj_size) - (end - PAGE_SIZE)); if (lr != NULL) zfs_rangelock_exit(lr); From 37185e7d0b8ba58a55a0962cf06611cf3c48a323 Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Thu, 28 Oct 2021 13:25:26 -0400 Subject: [PATCH 2/2] Exit the teardown section later in rename on FreeBSD We have to hold the teardown lock while dereferencing zfsvfs->z_os and, I believe, when committing to the ZIL. Note that jumping to the "out" label, "error" is always non-zero. Signed-off-by: Mark Johnston --- module/os/freebsd/zfs/zfs_vnops_os.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/module/os/freebsd/zfs/zfs_vnops_os.c b/module/os/freebsd/zfs/zfs_vnops_os.c index 322d64f01f16..6f63fb9db5bf 100644 --- a/module/os/freebsd/zfs/zfs_vnops_os.c +++ b/module/os/freebsd/zfs/zfs_vnops_os.c @@ -3431,7 +3431,6 @@ zfs_rename_(vnode_t *sdvp, vnode_t **svpp, struct componentname *scnp, dmu_tx_commit(tx); unlockout: /* all 4 vnodes are locked, ZFS_ENTER called */ - ZFS_EXIT(zfsvfs); if (want_seqc_end) { vn_seqc_write_end(*svpp); vn_seqc_write_end(sdvp); @@ -3444,10 +3443,12 @@ zfs_rename_(vnode_t *sdvp, vnode_t **svpp, struct componentname *scnp, VOP_UNLOCK1(*svpp); VOP_UNLOCK1(sdvp); -out: /* original two vnodes are locked */ - MPASS(!want_seqc_end); if (error == 0 && zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS) zil_commit(zilog, 0); + ZFS_EXIT(zfsvfs); + +out: /* original two vnodes are locked */ + MPASS(!want_seqc_end); if (*tvpp != NULL) VOP_UNLOCK1(*tvpp);