From nobody Thu Jan 09 14:53:11 2025 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4YTST40nWhz5kwYP; Thu, 09 Jan 2025 14:53:12 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4YTST408w0z47Wd; Thu, 9 Jan 2025 14:53:12 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1736434392; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=z64Yqk6GcKPKOpTY6cF0h3AOeSp9jrt1nb41mz9L0Ew=; b=aaaYhqx0OYvIiHa5tXZOU+UtNytdgrYSdxcaUNj9s8Jky695pM7auX3hQUJUXJH4flqfMy 9CVTSTrRXPB3iVeP8p0x6Xs8TpZDZRa+SzZTnxUPhe5eYOFwUSHxuYIY9WtaTqoIykwDor 9DV33xs2YYTsj4vZVR8bCnXm7dMr0UDFeqMMnIgCh5z5lcYY1jxUgJuvK0B/b1K/zF3B+3 7+d+Dh0LyXhy5VOucMl9HIp7jR3U3YMeo9vrXpVqYqt0csMyF5GPJ+RiFiQkrnkJXHlZP+ Qi03mf0F/LIjshldJpDLv+LAVJ+9XodEy5JekzIMW6gsob2Y5XTIGOVzwyrEdg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1736434392; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=z64Yqk6GcKPKOpTY6cF0h3AOeSp9jrt1nb41mz9L0Ew=; b=byWROXhmPiO1+Oam4Fwf69GHObTJlwE7mwlAqj0baHzGcKlRNj0cFCzX7V7jukWKYtJ1Zg iI5Pp8voNFzbf7d9ZvxBcTPECuemXGOvHsvD0dniFnknPUwVSXE9zFUbuGOl7qqA13VFmM pdmH9jJ6hh6SrgPZNR+vX7nXlqjkXOQf98wXC5O9prQn8s3xXSdha05f1hzekbiM4eg4AG G++4rZ7bAYEMled9yCNP8PPqw5ALvB069qM2LGFStVhA21/q/eWxrs4+tPtkfUH/Xcfeov XX9VB8G9jwepSN7k/oX5zx9HzzcukPG0grc8VSdPOlve94lG8+pa0mUy6iwr8w== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1736434392; a=rsa-sha256; cv=none; b=JtogkRekZUO4hCLnJrpuhPvA7qVdocr9/VrzAtZCXFL5qfOt+9V+s6AInSy/ES9fOTO2xP G79vBA8cr5/1/YBgVBslpSfUr82s0XMw2QYYpoedEOvhbrvBHSRLsnGenDv80TZi4lLvfW 2KzT+B5Xm+5P6rPJsQ0+rAK0wN774gEyfkU75P8NCJKYP7pq8GhbB2JrDfx/KOGJeY+B9P M74qjAIcIl4D9usCArhg1X3AkDbfrJgM6MulIivnlA2lxNdUpgSSkfvkUj1no0P9PF+CmS LcHH13+Q4gpsI6OdqhoJOMimyyeULl0IdsktYiMrhd9IZanFsmqpAzGnIvVS7w== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4YTST36thhzVZy; Thu, 09 Jan 2025 14:53:11 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 509ErBhk046105; Thu, 9 Jan 2025 14:53:11 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 509ErB12046101; Thu, 9 Jan 2025 14:53:11 GMT (envelope-from git) Date: Thu, 9 Jan 2025 14:53:11 GMT Message-Id: <202501091453.509ErB12046101@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mark Johnston Subject: git: 4a46ece6c6a9 - main - vmm: Fix error handling in vmm_handler() List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 4a46ece6c6a90f18effbfae7ddef79b41ef43eec Auto-Submitted: auto-generated The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=4a46ece6c6a90f18effbfae7ddef79b41ef43eec commit 4a46ece6c6a90f18effbfae7ddef79b41ef43eec Author: Mark Johnston AuthorDate: 2025-01-09 14:49:34 +0000 Commit: Mark Johnston 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: