From f6fbd3fb081a8521e82a8b8d501ce4952f556d02 Mon Sep 17 00:00:00 2001 From: Axel Gembe Date: Fri, 13 Oct 2023 18:20:08 +0700 Subject: [PATCH] Fix use-after-free of zfsvfs after unmount During unmount `zfsvfs` is destroyed and the pointer is zeroed in the VCB. There is however still a copy of the pointer in the DCB. Windows can still call into `zfs_AcquireForLazyWrite`, through `CcMgr` after unmount and this would use the already freed `zfsvfs` pointer. To fix this we set the pointer to zero in the DCB and add a zero check in `zfs_AcquireForLazyWrite`, `zfs_AcquireForReadAhead` and `fastio_acquire_for_mod_write`. Signed-off-by: Axel Gembe --- module/os/windows/zfs/zfs_vfsops.c | 10 ++++++++++ module/os/windows/zfs/zfs_vnops_windows.c | 16 ++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/module/os/windows/zfs/zfs_vfsops.c b/module/os/windows/zfs/zfs_vfsops.c index 613cf3a3c2f1..9d2e7bc2bed1 100644 --- a/module/os/windows/zfs/zfs_vfsops.c +++ b/module/os/windows/zfs/zfs_vfsops.c @@ -1565,6 +1565,7 @@ zfsvfs_teardown(zfsvfs_t *zfsvfs, boolean_t unmounting) int zfs_vfs_unmount(struct mount *mp, int mntflags, vfs_context_t context) { + mount_t *mp_dcb = NULL; zfsvfs_t *zfsvfs = vfs_fsprivate(mp); objset_t *os; char osname[MAXNAMELEN]; @@ -1670,6 +1671,15 @@ zfs_vfs_unmount(struct mount *mp, int mntflags, vfs_context_t context) dmu_objset_disown(os, B_TRUE, zfsvfs); } + mp_dcb = mp->parent_device; + if (likely(mp_dcb != NULL)) { + if (mp_dcb->type == MOUNT_TYPE_DCB) { + // dcb also has a reference to zfsvfs, we need to + // remove that before it is freed. + vfs_setfsprivate(mp_dcb, NULL); + } + } + zfs_freevfs(zfsvfs->z_vfs); return (0); diff --git a/module/os/windows/zfs/zfs_vnops_windows.c b/module/os/windows/zfs/zfs_vnops_windows.c index 349dfa1a844c..293ee7d06e03 100644 --- a/module/os/windows/zfs/zfs_vnops_windows.c +++ b/module/os/windows/zfs/zfs_vnops_windows.c @@ -132,6 +132,11 @@ zfs_AcquireForLazyWrite(void *Context, BOOLEAN Wait) struct vnode *vp = fo->FsContext; dprintf("%s:fo %p\n", __func__, fo); + if (unlikely(zfsvfs == NULL)) { + dprintf("%s: fo %p already freed zfsvfs\n", __func__, fo); + return (FALSE); + } + /* Confirm we are mounted, and stop unmounting */ if (vfs_busy(zfsvfs->z_vfs, 0) != 0) return (FALSE); @@ -210,6 +215,11 @@ zfs_AcquireForReadAhead(void *Context, BOOLEAN Wait) dprintf("%s:\n", __func__); + if (unlikely(zfsvfs == NULL)) { + dprintf("%s: fo %p already freed zfsvfs\n", __func__, fo); + return (FALSE); + } + if (vfs_busy(zfsvfs->z_vfs, 0) != 0) return (FALSE); @@ -7950,6 +7960,12 @@ fastio_acquire_for_mod_write(PFILE_OBJECT FileObject, NTSTATUS Status = STATUS_INVALID_PARAMETER; dprintf("%s: \n", __func__); + if (unlikely(zfsvfs == NULL)) { + dprintf("%s: fo %p already freed zfsvfs\n", __func__, + FileObject); + return (STATUS_INVALID_PARAMETER); + } + if (vfs_busy(zfsvfs->z_vfs, 0) != 0) return (STATUS_INVALID_PARAMETER);