[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@xxxxxxxxxx>
---
 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