[PATCH 1/2] vfio: Publish the container after all fallible initialization

[PATCH 1/2] vfio: Publish the container after all fallible initialization

[Kyle Zeng]

vfio_group_init() publishes /dev/vfio/vfio before vfio_init() has
finished. The optional no-IOMMU backend is also registered after the
misc device. A concurrent open can therefore obtain a container while
the module is still COMING, even if a later initialization step fails.

The failed-module-init path frees the module without waiting for these
open files. misc_deregister() prevents new opens, but does not revoke
existing files, leaving their vfio_fops and callbacks in freed module
memory. An unprivileged first-autoload race can trigger this by exhausting
the global task limit before the IRQFD workqueue creates its rescuer.
A subsequent VFIO_GET_API_VERSION ioctl reports:

  BUG: KASAN: vmalloc-out-of-bounds in __se_sys_ioctl+0xaf/0x120
  Read of size 8 at addr ffffffffc089df70 by task vfio_race/1484

Move container initialization out of vfio_group_init() and make it the
last fallible step in vfio_init(). Register the no-IOMMU backend before
the misc device, and unwind both resources in reverse order. Once an
open can reach vfio_fops, module initialization can no longer fail.

Fixes: cba3345cc494 ("vfio: VFIO core")
Signed-off-by: Kyle Zeng <kylebot@openai.com>
---
 drivers/vfio/container.c | 22 +++++++++++++---------
 drivers/vfio/group.c     |  9 +--------
 drivers/vfio/vfio_main.c |  8 ++++++++
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/vfio/container.c b/drivers/vfio/container.c
index 003281dbf8bc..bd245e23a55d 100644
--- a/drivers/vfio/container.c
+++ b/drivers/vfio/container.c
@@ -577,29 +577,33 @@ int __init vfio_container_init(void)
 	mutex_init(&vfio.iommu_drivers_lock);
 	INIT_LIST_HEAD(&vfio.iommu_drivers_list);
 
+	if (IS_ENABLED(CONFIG_VFIO_NOIOMMU)) {
+		ret = vfio_register_iommu_driver(&vfio_noiommu_ops);
+		if (ret)
+			goto err_mutex;
+	}
+
 	ret = misc_register(&vfio_dev);
 	if (ret) {
 		pr_err("vfio: misc device register failed\n");
-		return ret;
+		goto err_noiommu;
 	}
 
-	if (IS_ENABLED(CONFIG_VFIO_NOIOMMU)) {
-		ret = vfio_register_iommu_driver(&vfio_noiommu_ops);
-		if (ret)
-			goto err_misc;
-	}
 	return 0;
 
-err_misc:
-	misc_deregister(&vfio_dev);
+err_noiommu:
+	if (IS_ENABLED(CONFIG_VFIO_NOIOMMU))
+		vfio_unregister_iommu_driver(&vfio_noiommu_ops);
+err_mutex:
+	mutex_destroy(&vfio.iommu_drivers_lock);
 	return ret;
 }
 
 void vfio_container_cleanup(void)
 {
+	misc_deregister(&vfio_dev);
 	if (IS_ENABLED(CONFIG_VFIO_NOIOMMU))
 		vfio_unregister_iommu_driver(&vfio_noiommu_ops);
-	misc_deregister(&vfio_dev);
 	mutex_destroy(&vfio.iommu_drivers_lock);
 }
 
diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
index b2299e5bc6df..dc8f8a38723f 100644
--- a/drivers/vfio/group.c
+++ b/drivers/vfio/group.c
@@ -898,14 +898,10 @@ int __init vfio_group_init(void)
 	mutex_init(&vfio.group_lock);
 	INIT_LIST_HEAD(&vfio.group_list);
 
-	ret = vfio_container_init();
-	if (ret)
-		return ret;
-
 	/* /dev/vfio/$GROUP */
 	ret = class_register(&vfio_class);
 	if (ret)
-		goto err_group_class;
+		return ret;
 
 	ret = alloc_chrdev_region(&vfio.group_devt, 0, MINORMASK + 1, "vfio");
 	if (ret)
@@ -914,8 +910,6 @@ int __init vfio_group_init(void)
 
 err_alloc_chrdev:
 	class_unregister(&vfio_class);
-err_group_class:
-	vfio_container_cleanup();
 	return ret;
 }
 
@@ -925,5 +919,4 @@ void vfio_group_cleanup(void)
 	ida_destroy(&vfio.group_ida);
 	unregister_chrdev_region(vfio.group_devt, MINORMASK + 1);
 	class_unregister(&vfio_class);
-	vfio_container_cleanup();
 }
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 423ead48aafe..dfc48a9e0fe6 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -1852,10 +1852,17 @@ static int __init vfio_init(void)
 	if (ret)
 		goto err_alloc_dev_chrdev;
 
+	/* Publish /dev/vfio/vfio only after all other fallible setup. */
+	ret = vfio_container_init();
+	if (ret)
+		goto err_container;
+
 	vfio_debugfs_create_root();
 	pr_info(DRIVER_DESC " version: " DRIVER_VERSION "\n");
 	return 0;
 
+err_container:
+	vfio_cdev_cleanup();
 err_alloc_dev_chrdev:
 	class_unregister(&vfio_device_class);
 err_dev_class:
@@ -1868,6 +1875,7 @@ static int __init vfio_init(void)
 static void __exit vfio_cleanup(void)
 {
 	vfio_debugfs_remove_root();
+	vfio_container_cleanup();
 	ida_destroy(&vfio.device_ida);
 	vfio_cdev_cleanup();
 	class_unregister(&vfio_device_class);

base-commit: 70dd4ee72eb2d5793c62e71c14e3aba78e5a4e56
-- 
2.53.0

[PATCH 2/2] iommufd: Reject opens until module initialization succeeds

[Kyle Zeng]

iommufd_init() publishes /dev/iommu before registering the optional
/dev/vfio/vfio compatibility node and running the optional self-test
initialization. Either later operation can fail. Since file operations
can acquire a reference to a COMING module, an already-open file can
survive the failed initialization and refer to freed iommufd module
memory.

Gate the shared open callback until all fallible initialization has
completed. Publish the ready flag with release semantics and read it
with acquire semantics so successful opens see the initialized state.

An early rejected open also needs to drop its module-backed operations
table. misc_open() holds misc_mtx across the callback, so deregistration
waits for it, but the VFS's fops_put() on the failed open happens later.
Restore the inode's core character-device operations before returning
-ENODEV, avoiding that remaining reference to the failed module.

Fixes: 01f70cbb26ea ("iommufd: Allow iommufd to supply /dev/vfio/vfio")
Fixes: 23a1b46f15d5 ("iommufd/selftest: Make the mock iommu driver into a real driver")
Signed-off-by: Kyle Zeng <kylebot@openai.com>
---
 drivers/iommu/iommufd/main.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c
index 9a921b153162..781a8d63dda9 100644
--- a/drivers/iommu/iommufd/main.c
+++ b/drivers/iommu/iommufd/main.c
@@ -16,6 +16,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
+#include <linux/smp.h>
 #include <uapi/linux/iommufd.h>
 
 #include "io_pagetable.h"
@@ -30,6 +31,7 @@ struct iommufd_object_ops {
 };
 static const struct iommufd_object_ops iommufd_object_ops[];
 static struct miscdevice vfio_misc_dev;
+static bool iommufd_ready;
 
 struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,
 					     size_t size,
@@ -316,6 +318,17 @@ static int iommufd_fops_open(struct inode *inode, struct file *filp)
 {
 	struct iommufd_ctx *ictx;
 
+	/* Pairs with the ready publication at the end of iommufd_init(). */
+	if (!smp_load_acquire(&iommufd_ready)) {
+		/*
+		 * misc_deregister() waits for this callback, but not the
+		 * VFS's fops_put() after a failed open. Restore the core
+		 * character-device operations before dropping misc_mtx.
+		 */
+		replace_fops(filp, fops_get(inode->i_fop));
+		return -ENODEV;
+	}
+
 	ictx = kzalloc_obj(*ictx, GFP_KERNEL_ACCOUNT);
 	if (!ictx)
 		return -ENOMEM;
@@ -799,6 +812,8 @@ static int __init iommufd_init(void)
 	ret = iommufd_test_init();
 	if (ret)
 		goto err_vfio_misc;
+	/* Pairs with iommufd_fops_open(); no fallible setup may follow. */
+	smp_store_release(&iommufd_ready, true);
 	return 0;
 
 err_vfio_misc:
-- 
2.53.0

Re: [PATCH 1/2] vfio: Publish the container after all fallible initialization

[Alex Williamson]

On Fri, 30 Oct 2026 21:25:26 -0700
Kyle Zeng <kylebot@openai.com> wrote:

> vfio_group_init() publishes /dev/vfio/vfio before vfio_init() has
> finished. The optional no-IOMMU backend is also registered after the
> misc device. A concurrent open can therefore obtain a container while
> the module is still COMING, even if a later initialization step fails.
> 
> The failed-module-init path frees the module without waiting for these
> open files. misc_deregister() prevents new opens, but does not revoke
> existing files, leaving their vfio_fops and callbacks in freed module
> memory. An unprivileged first-autoload race can trigger this by exhausting
> the global task limit before the IRQFD workqueue creates its rescuer.
> A subsequent VFIO_GET_API_VERSION ioctl reports:
> 
>   BUG: KASAN: vmalloc-out-of-bounds in __se_sys_ioctl+0xaf/0x120
>   Read of size 8 at addr ffffffffc089df70 by task vfio_race/1484
> 
> Move container initialization out of vfio_group_init() and make it the
> last fallible step in vfio_init(). Register the no-IOMMU backend before
> the misc device, and unwind both resources in reverse order. Once an
> open can reach vfio_fops, module initialization can no longer fail.
> 
> Fixes: cba3345cc494 ("vfio: VFIO core")
> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> ---
>  drivers/vfio/container.c | 22 +++++++++++++---------
>  drivers/vfio/group.c     |  9 +--------
>  drivers/vfio/vfio_main.c |  8 ++++++++
>  3 files changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/vfio/container.c b/drivers/vfio/container.c
> index 003281dbf8bc..bd245e23a55d 100644
> --- a/drivers/vfio/container.c
> +++ b/drivers/vfio/container.c
> @@ -577,29 +577,33 @@ int __init vfio_container_init(void)
>  	mutex_init(&vfio.iommu_drivers_lock);
>  	INIT_LIST_HEAD(&vfio.iommu_drivers_list);
>  
> +	if (IS_ENABLED(CONFIG_VFIO_NOIOMMU)) {
> +		ret = vfio_register_iommu_driver(&vfio_noiommu_ops);
> +		if (ret)
> +			goto err_mutex;
> +	}
> +
>  	ret = misc_register(&vfio_dev);
>  	if (ret) {
>  		pr_err("vfio: misc device register failed\n");
> -		return ret;
> +		goto err_noiommu;
>  	}
>  
> -	if (IS_ENABLED(CONFIG_VFIO_NOIOMMU)) {
> -		ret = vfio_register_iommu_driver(&vfio_noiommu_ops);
> -		if (ret)
> -			goto err_misc;
> -	}

This looks wrong to me, an error loading the vfio-noiommu IOMMU backend
shouldn't cause the subsystem to fail.

I think this is fallout from a13b1e472b93 ("vfio: check
vfio_register_iommu_driver() return value") that had no other
justification for inducing this failure than the fact the registration
function can return an error.

The error return is necessary for the module init use cases, not here.

Unwrapping that as an unchecked error avoids the re-ordering here and
in vfio_container_cleanup() below, and this unnecessary unwind
collateral.

This should be a separate patch, and use the correct lists per the
MAINTAINERS file (mising kvm@vger for vfio).

>  	return 0;
>  
> -err_misc:
> -	misc_deregister(&vfio_dev);
> +err_noiommu:
> +	if (IS_ENABLED(CONFIG_VFIO_NOIOMMU))
> +		vfio_unregister_iommu_driver(&vfio_noiommu_ops);
> +err_mutex:
> +	mutex_destroy(&vfio.iommu_drivers_lock);
>  	return ret;
>  }
>  
>  void vfio_container_cleanup(void)
>  {
> +	misc_deregister(&vfio_dev);
>  	if (IS_ENABLED(CONFIG_VFIO_NOIOMMU))
>  		vfio_unregister_iommu_driver(&vfio_noiommu_ops);
> -	misc_deregister(&vfio_dev);
>  	mutex_destroy(&vfio.iommu_drivers_lock);
>  }
>  
> diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
> index b2299e5bc6df..dc8f8a38723f 100644
> --- a/drivers/vfio/group.c
> +++ b/drivers/vfio/group.c
> @@ -898,14 +898,10 @@ int __init vfio_group_init(void)
>  	mutex_init(&vfio.group_lock);
>  	INIT_LIST_HEAD(&vfio.group_list);
>  
> -	ret = vfio_container_init();
> -	if (ret)
> -		return ret;
> -
>  	/* /dev/vfio/$GROUP */
>  	ret = class_register(&vfio_class);
>  	if (ret)
> -		goto err_group_class;
> +		return ret;
>  
>  	ret = alloc_chrdev_region(&vfio.group_devt, 0, MINORMASK + 1, "vfio");
>  	if (ret)
> @@ -914,8 +910,6 @@ int __init vfio_group_init(void)
>  
>  err_alloc_chrdev:
>  	class_unregister(&vfio_class);
> -err_group_class:
> -	vfio_container_cleanup();
>  	return ret;
>  }
>  
> @@ -925,5 +919,4 @@ void vfio_group_cleanup(void)
>  	ida_destroy(&vfio.group_ida);
>  	unregister_chrdev_region(vfio.group_devt, MINORMASK + 1);
>  	class_unregister(&vfio_class);
> -	vfio_container_cleanup();
>  }
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index 423ead48aafe..dfc48a9e0fe6 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -1852,10 +1852,17 @@ static int __init vfio_init(void)
>  	if (ret)
>  		goto err_alloc_dev_chrdev;
>  
> +	/* Publish /dev/vfio/vfio only after all other fallible setup. */
> +	ret = vfio_container_init();
> +	if (ret)
> +		goto err_container;
> +
>  	vfio_debugfs_create_root();
>  	pr_info(DRIVER_DESC " version: " DRIVER_VERSION "\n");
>  	return 0;
>  
> +err_container:
> +	vfio_cdev_cleanup();
>  err_alloc_dev_chrdev:
>  	class_unregister(&vfio_device_class);
>  err_dev_class:
> @@ -1868,6 +1875,7 @@ static int __init vfio_init(void)
>  static void __exit vfio_cleanup(void)
>  {
>  	vfio_debugfs_remove_root();
> +	vfio_container_cleanup();
>  	ida_destroy(&vfio.device_ida);
>  	vfio_cdev_cleanup();
>  	class_unregister(&vfio_device_class);
> 
> base-commit: 70dd4ee72eb2d5793c62e71c14e3aba78e5a4e56

Jason additionally asked[1] whether the try_module_get() via misc_open()
-> fops_get() should be the real barrier, preventing open until the
module has fully initialized.  That seems a valid question here as well.
Thanks,

Alex

[1]https://lore.kernel.org/all/20261030120845.GA3889730@ziepe.ca/

Re: [PATCH 1/2] vfio: Publish the container after all fallible initialization

[Jason Gunthorpe]

On Fri, Oct 30, 2026 at 09:25:26PM -0700, Kyle Zeng wrote:
> vfio_group_init() publishes /dev/vfio/vfio before vfio_init() has
> finished. The optional no-IOMMU backend is also registered after the
> misc device. A concurrent open can therefore obtain a container while
> the module is still COMING, even if a later initialization step fails.

You didn't cc the vfio maintainers or the its list.

Nor did you refer to my prior remarks here:

https://lore.kernel.org/all/20261030120845.GA3889730@ziepe.ca/

> The failed-module-init path frees the module without waiting for these
> open files.

This is a cdev core bug. It should not allow the file to be opened
until it has secured a module refcount. I don't know much about how
the module refcounting mechanisms work, but it is unreasonable to fix
it like you are doing here by hacking every misc user in the kernel.

Jason