git: 4008758105a6 - main - vmm: Validate credentials when opening a vmmdev

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Sun, 01 Sep 2024 14:09:42 UTC
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=4008758105a6da9eaa0b96b81dfb3042a33259be

commit 4008758105a6da9eaa0b96b81dfb3042a33259be
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-09-01 14:00:32 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-09-01 14:03:16 +0000

    vmm: Validate credentials when opening a vmmdev
    
    Rather than performing privilege checks after a specific VM's device
    file is opened, do it once at the time the device file is opened.  This
    means that one can continue to access a VM via its device fd after
    attaching to a jail which does not have vmm enabled, but this seems like
    a reasonable semantic to have anyway.
    
    Reviewed by:    jhb
    Differential Revision:  https://reviews.freebsd.org/D46486
---
 sys/dev/vmm/vmm_dev.c | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/sys/dev/vmm/vmm_dev.c b/sys/dev/vmm/vmm_dev.c
index f43429de4d4c..b4ae2997006f 100644
--- a/sys/dev/vmm/vmm_dev.c
+++ b/sys/dev/vmm/vmm_dev.c
@@ -186,10 +186,6 @@ vmmdev_rw(struct cdev *cdev, struct uio *uio, int flags)
 	void *hpa, *cookie;
 	struct vmmdev_softc *sc;
 
-	error = vmm_priv_check(curthread->td_ucred);
-	if (error)
-		return (error);
-
 	sc = vmmdev_lookup2(cdev);
 	if (sc == NULL)
 		return (ENXIO);
@@ -327,6 +323,32 @@ vm_set_register_set(struct vcpu *vcpu, unsigned int count, int *regnum,
 	return (error);
 }
 
+static int
+vmmdev_open(struct cdev *dev, int flags, int fmt, struct thread *td)
+{
+	struct vmmdev_softc *sc;
+	int error;
+
+	sc = vmmdev_lookup2(dev);
+	KASSERT(sc != NULL, ("%s: device not found", __func__));
+
+	/*
+	 * A user can only access VMs that they themselves have created.
+	 */
+	if (td->td_ucred != sc->ucred)
+		return (EPERM);
+
+	/*
+	 * A jail without vmm access shouldn't be able to access vmm device
+	 * files at all, but check here just to be thorough.
+	 */
+	error = vmm_priv_check(td->td_ucred);
+	if (error != 0)
+		return (error);
+
+	return (0);
+}
+
 static const struct vmmdev_ioctl vmmdev_ioctls[] = {
 	VMMDEV_IOCTL(VM_GET_REGISTER, VMMDEV_IOCTL_LOCK_ONE_VCPU),
 	VMMDEV_IOCTL(VM_SET_REGISTER, VMMDEV_IOCTL_LOCK_ONE_VCPU),
@@ -375,10 +397,6 @@ vmmdev_ioctl(struct cdev *cdev, u_long cmd, caddr_t data, int fflag,
 	const struct vmmdev_ioctl *ioctl;
 	int error, vcpuid;
 
-	error = vmm_priv_check(td->td_ucred);
-	if (error)
-		return (error);
-
 	sc = vmmdev_lookup2(cdev);
 	if (sc == NULL)
 		return (ENXIO);
@@ -681,10 +699,6 @@ vmmdev_mmap_single(struct cdev *cdev, vm_ooffset_t *offset, vm_size_t mapsize,
 	int error, found, segid;
 	bool sysmem;
 
-	error = vmm_priv_check(curthread->td_ucred);
-	if (error)
-		return (error);
-
 	first = *offset;
 	last = first + mapsize;
 	if ((nprot & PROT_EXEC) || first < 0 || first >= last)
@@ -833,6 +847,7 @@ SYSCTL_PROC(_hw_vmm, OID_AUTO, destroy,
 static struct cdevsw vmmdevsw = {
 	.d_name		= "vmmdev",
 	.d_version	= D_VERSION,
+	.d_open		= vmmdev_open,
 	.d_ioctl	= vmmdev_ioctl,
 	.d_mmap_single	= vmmdev_mmap_single,
 	.d_read		= vmmdev_rw,