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 <[email protected]>
openzfsonwindows · Oct 14, 2023 · 350db38 · 350db38
1 parent fee66c7
commit 350db38
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 0 deletions.
diff --git 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
@@ -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);