git: 4a46ece6c6a9 - main - vmm: Fix error handling in vmm_handler()

The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=4a46ece6c6a90f18effbfae7ddef79b41ef43eec

commit 4a46ece6c6a90f18effbfae7ddef79b41ef43eec
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2025-01-09 14:49:34 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2025-01-09 14:49:34 +0000

    vmm: Fix error handling in vmm_handler()
    
    In commit a97f683fe3c4 I didn't add code to remove the vmmctl device
    when vmm.ko is unloaded, so it would persist and prevent vmm.ko from
    being re-loaded.
    
    Extend vmmdev_cleanup() to destroy the vmmctl cdev.  Also call
    vmmdev_cleanup() if vmm_init() fails.
    
    Reviewed by:    corvink, andrew
    Fixes:          a97f683fe3c4 ("vmm: Add a device file interface for creating and destroying VMs")
    Differential Revision:  https://reviews.freebsd.org/D48269
---
 sys/amd64/vmm/vmm.c   |  2 ++
 sys/arm64/vmm/vmm.c   | 11 ++++++++---
 sys/dev/vmm/vmm_dev.c | 34 +++++++++++++++++++---------------
 sys/riscv/vmm/vmm.c   | 11 ++++++++---
 4 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/sys/amd64/vmm/vmm.c b/sys/amd64/vmm/vmm.c
index d05d979a531a..aa13d506ac6a 100644
--- a/sys/amd64/vmm/vmm.c
+++ b/sys/amd64/vmm/vmm.c
@@ -467,6 +467,8 @@ vmm_handler(module_t mod, int what, void *arg)
 			error = vmm_init();
 			if (error == 0)
 				vmm_initialized = 1;
+			else
+				(void)vmmdev_cleanup();
 		} else {
 			error = ENXIO;
 		}
diff --git a/sys/arm64/vmm/vmm.c b/sys/arm64/vmm/vmm.c
index 808df5e599ac..77c565e37264 100644
--- a/sys/arm64/vmm/vmm.c
+++ b/sys/arm64/vmm/vmm.c
@@ -361,21 +361,26 @@ vmm_handler(module_t mod, int what, void *arg)
 
 	switch (what) {
 	case MOD_LOAD:
-		/* TODO: if (vmm_is_hw_supported()) { */
 		error = vmmdev_init();
 		if (error != 0)
 			break;
 		error = vmm_init();
 		if (error == 0)
 			vmm_initialized = true;
+		else
+			(void)vmmdev_cleanup();
 		break;
 	case MOD_UNLOAD:
-		/* TODO: if (vmm_is_hw_supported()) { */
 		error = vmmdev_cleanup();
 		if (error == 0 && vmm_initialized) {
 			error = vmmops_modcleanup();
-			if (error)
+			if (error) {
+				/*
+				 * Something bad happened - prevent new
+				 * VMs from being created
+				 */
 				vmm_initialized = false;
+			}
 		}
 		break;
 	default:
diff --git a/sys/dev/vmm/vmm_dev.c b/sys/dev/vmm/vmm_dev.c
index 4ab99f92f72a..27c960c8ef2e 100644
--- a/sys/dev/vmm/vmm_dev.c
+++ b/sys/dev/vmm/vmm_dev.c
@@ -979,6 +979,7 @@ vmmctl_ioctl(struct cdev *cdev, u_long cmd, caddr_t data, int fflag,
 	return (error);
 }
 
+static struct cdev *vmmctl_cdev;
 static struct cdevsw vmmctlsw = {
 	.d_name		= "vmmctl",
 	.d_version	= D_VERSION,
@@ -989,31 +990,34 @@ static struct cdevsw vmmctlsw = {
 int
 vmmdev_init(void)
 {
-	struct cdev *cdev;
 	int error;
 
-	error = make_dev_p(MAKEDEV_CHECKNAME, &cdev, &vmmctlsw, NULL,
+	sx_xlock(&vmmdev_mtx);
+	error = make_dev_p(MAKEDEV_CHECKNAME, &vmmctl_cdev, &vmmctlsw, NULL,
 	    UID_ROOT, GID_WHEEL, 0600, "vmmctl");
-	if (error)
-		return (error);
-
-	pr_allow_flag = prison_add_allow(NULL, "vmm", NULL,
-	    "Allow use of vmm in a jail.");
+	if (error == 0)
+		pr_allow_flag = prison_add_allow(NULL, "vmm", NULL,
+		    "Allow use of vmm in a jail.");
+	sx_xunlock(&vmmdev_mtx);
 
-	return (0);
+	return (error);
 }
 
 int
 vmmdev_cleanup(void)
 {
-	int error;
-
-	if (SLIST_EMPTY(&head))
-		error = 0;
-	else
-		error = EBUSY;
+	sx_xlock(&vmmdev_mtx);
+	if (!SLIST_EMPTY(&head)) {
+		sx_xunlock(&vmmdev_mtx);
+		return (EBUSY);
+	}
+	if (vmmctl_cdev != NULL) {
+		destroy_dev(vmmctl_cdev);
+		vmmctl_cdev = NULL;
+	}
+	sx_xunlock(&vmmdev_mtx);
 
-	return (error);
+	return (0);
 }
 
 static int
diff --git a/sys/riscv/vmm/vmm.c b/sys/riscv/vmm/vmm.c
index f7cbfc1dfea5..96871fc88453 100644
--- a/sys/riscv/vmm/vmm.c
+++ b/sys/riscv/vmm/vmm.c
@@ -259,21 +259,26 @@ vmm_handler(module_t mod, int what, void *arg)
 
 	switch (what) {
 	case MOD_LOAD:
-		/* TODO: check if has_hyp here? */
 		error = vmmdev_init();
 		if (error != 0)
 			break;
 		error = vmm_init();
 		if (error == 0)
 			vmm_initialized = true;
+		else
+			(void)vmmdev_cleanup();
 		break;
 	case MOD_UNLOAD:
-		/* TODO: check if has_hyp here? */
 		error = vmmdev_cleanup();
 		if (error == 0 && vmm_initialized) {
 			error = vmmops_modcleanup();
-			if (error)
+			if (error) {
+				/*
+				 * Something bad happened - prevent new
+				 * VMs from being created
+				 */
 				vmm_initialized = false;
+			}
 		}
 		break;
 	default: