[PATCH 0/7] vfio: Fix release ordering races and use driver_override

[PATCH 0/7] vfio: Fix release ordering races and use driver_override

[Alex Williamson]

VM hotplug testing reveals a number of races in the vfio device,
group, container shutdown path, some attributed to libvirt's ask/take
unplug behavior and some long standing with groups potentially
composed of multiple devices, where each device can be independently
bound to drivers.  Libvirt's ask/take behavior is a result of the
asynchronous nature of PCI hotplug, libvirt registers a hot-unplug
request (ask), which is acknowledged almost immediately and then
proceeds to try to unbind the device from the vfio bus driver (take).
This sets us off on racing paths where we allow the device to be
released from the group much like would happen in groups with multiple
devices, while the group and container are torn down separately.
These races are addressed in the first 3 patches of this series.

The long standing issue with removing devices from in-use groups is
that we feel that the system is compromised if we allow user and host
devices within the same non-isolated group.  This triggers a BUG_ON
when we detect this condition after the rogue driver binding.  Since
that code was put in place we've added driver_override support for
all of the physical buses supported by vfio, giving us a way to block
binding to such compromising drivers.  We finally enable that in the
latter 4 patches of this series, minding that we need to allow
re-binding to non-compromising drivers, and also noting that a small
synchronization stall is effective in eliminating the need for this
blocking in the more common singleton device group case.

Reviews, comments, and acks appreciated.  Thanks,

Alex
---

Alex Williamson (7):
      vfio: Fix group release deadlock
      kvm-vfio: Decouple only when we match a group
      vfio: New external user group/file match
      iommu: Add driver-not-bound notification
      vfio: Create interface for vfio bus drivers to register
      vfio: Register pci, platform, amba, and mdev bus drivers
      vfio: Use driver_override to avert binding to compromising drivers


 drivers/iommu/iommu.c                 |    2 
 drivers/vfio/mdev/vfio_mdev.c         |   13 ++
 drivers/vfio/pci/vfio_pci.c           |    7 +
 drivers/vfio/platform/vfio_amba.c     |   24 +++
 drivers/vfio/platform/vfio_platform.c |   24 +++
 drivers/vfio/vfio.c                   |  252 ++++++++++++++++++++++++++++++++-
 include/linux/iommu.h                 |    1 
 include/linux/vfio.h                  |    5 +
 virt/kvm/vfio.c                       |   33 +++-
 9 files changed, 338 insertions(+), 23 deletions(-)

[PATCH 1/7] vfio: Fix group release deadlock

[Alex Williamson]

If vfio_iommu_group_notifier() acquires a group reference and that
reference becomes the last reference to the group, then vfio_group_put
introduces a deadlock code path where we're trying to unregister from
the iommu notifier chain from within a callout of that chain.  Use a
work_struct to release this reference asynchronously.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/vfio/vfio.c |   37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 561084ab387f..f2e24d5699f2 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -423,6 +423,34 @@ static void vfio_group_put(struct vfio_group *group)
 	kref_put_mutex(&group->kref, vfio_group_release, &vfio.group_lock);
 }
 
+struct vfio_group_put_work {
+	struct work_struct work;
+	struct vfio_group *group;
+};
+
+static void vfio_group_put_bg(struct work_struct *work)
+{
+	struct vfio_group_put_work *do_work;
+
+	do_work = container_of(work, struct vfio_group_put_work, work);
+
+	vfio_group_put(do_work->group);
+	kfree(do_work);
+}
+
+static void vfio_group_schedule_put(struct vfio_group *group)
+{
+	struct vfio_group_put_work *do_work;
+
+	do_work = kmalloc(sizeof(*do_work), GFP_KERNEL);
+	if (WARN_ON(!do_work))
+		return;
+
+	INIT_WORK(&do_work->work, vfio_group_put_bg);
+	do_work->group = group;
+	schedule_work(&do_work->work);
+}
+
 /* Assume group_lock or group reference is held */
 static void vfio_group_get(struct vfio_group *group)
 {
@@ -762,7 +790,14 @@ static int vfio_iommu_group_notifier(struct notifier_block *nb,
 		break;
 	}
 
-	vfio_group_put(group);
+	/*
+	 * If we're the last reference to the group, the group will be
+	 * released, which includes unregistering the iommu group notifier.
+	 * We hold a read-lock on that notifier list, unregistering needs
+	 * a write-lock... deadlock.  Release our reference asyncronously
+	 * to avoid that situation.
+	 */
+	vfio_group_schedule_put(group);
 	return NOTIFY_OK;
 }
 

[PATCH 2/7] kvm-vfio: Decouple only when we match a group

[Alex Williamson]

Unset-KVM and decrement-assignment only when we find the group in our
list.  Otherwise we can get out of sync if the user triggers this for
groups that aren't currently on our list.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
---
 virt/kvm/vfio.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
index d32f239eb471..db9036ef8c73 100644
--- a/virt/kvm/vfio.c
+++ b/virt/kvm/vfio.c
@@ -201,18 +201,16 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
 				continue;
 
 			list_del(&kvg->node);
+			kvm_arch_end_assignment(dev->kvm);
+			kvm_vfio_group_set_kvm(kvg->vfio_group, NULL);
 			kvm_vfio_group_put_external_user(kvg->vfio_group);
 			kfree(kvg);
 			ret = 0;
 			break;
 		}
 
-		kvm_arch_end_assignment(dev->kvm);
-
 		mutex_unlock(&kv->lock);
 
-		kvm_vfio_group_set_kvm(vfio_group, NULL);
-
 		kvm_vfio_group_put_external_user(vfio_group);
 
 		kvm_vfio_update_coherency(dev);

[PATCH 3/7] vfio: New external user group/file match

[Alex Williamson]

At the point where the kvm-vfio pseudo device wants to release its
vfio group reference, we can't always acquire a new reference to make
that happen.  The group can be in a state where we wouldn't allow a
new reference to be added.  This new helper function allows a caller
to match a file to a group to facilitate this.  Given a file and
group, report if they match.  Thus the caller needs to already have a
group reference to match to the file.  This allows the deletion of a
group without acquiring a new reference.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
---
 drivers/vfio/vfio.c  |    9 +++++++++
 include/linux/vfio.h |    2 ++
 virt/kvm/vfio.c      |   27 +++++++++++++++++++--------
 3 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index f2e24d5699f2..e6117de60f87 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1776,6 +1776,15 @@ void vfio_group_put_external_user(struct vfio_group *group)
 }
 EXPORT_SYMBOL_GPL(vfio_group_put_external_user);
 
+bool vfio_external_group_match_file(struct vfio_group *test_group,
+				    struct file *filep)
+{
+	struct vfio_group *group = filep->private_data;
+
+	return (filep->f_op == &vfio_group_fops) && (group == test_group);
+}
+EXPORT_SYMBOL_GPL(vfio_external_group_match_file);
+
 int vfio_external_user_iommu_id(struct vfio_group *group)
 {
 	return iommu_group_id(group->iommu_group);
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index edf9b2cad277..9b34d0af5d27 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -97,6 +97,8 @@ extern void vfio_unregister_iommu_driver(
  */
 extern struct vfio_group *vfio_group_get_external_user(struct file *filep);
 extern void vfio_group_put_external_user(struct vfio_group *group);
+extern bool vfio_external_group_match_file(struct vfio_group *group,
+					   struct file *filep);
 extern int vfio_external_user_iommu_id(struct vfio_group *group);
 extern long vfio_external_check_extension(struct vfio_group *group,
 					  unsigned long arg);
diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
index db9036ef8c73..7c14729b1f2c 100644
--- a/virt/kvm/vfio.c
+++ b/virt/kvm/vfio.c
@@ -47,6 +47,22 @@ static struct vfio_group *kvm_vfio_group_get_external_user(struct file *filep)
 	return vfio_group;
 }
 
+static bool kvm_vfio_external_group_match_file(struct vfio_group *group,
+					       struct file *filep)
+{
+	bool ret, (*fn)(struct vfio_group *, struct file *);
+
+	fn = symbol_get(vfio_external_group_match_file);
+	if (!fn)
+		return false;
+
+	ret = fn(group, filep);
+
+	symbol_put(vfio_external_group_match_file);
+
+	return ret;
+}
+
 static void kvm_vfio_group_put_external_user(struct vfio_group *vfio_group)
 {
 	void (*fn)(struct vfio_group *);
@@ -186,18 +202,13 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
 		if (!f.file)
 			return -EBADF;
 
-		vfio_group = kvm_vfio_group_get_external_user(f.file);
-		fdput(f);
-
-		if (IS_ERR(vfio_group))
-			return PTR_ERR(vfio_group);
-
 		ret = -ENOENT;
 
 		mutex_lock(&kv->lock);
 
 		list_for_each_entry(kvg, &kv->group_list, node) {
-			if (kvg->vfio_group != vfio_group)
+			if (!kvm_vfio_external_group_match_file(kvg->vfio_group,
+								f.file))
 				continue;
 
 			list_del(&kvg->node);
@@ -211,7 +222,7 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
 
 		mutex_unlock(&kv->lock);
 
-		kvm_vfio_group_put_external_user(vfio_group);
+		fdput(f);
 
 		kvm_vfio_update_coherency(dev);
 

[PATCH 4/7] iommu: Add driver-not-bound notification

[Alex Williamson]

The driver core supports a BUS_NOTIFY_DRIVER_NOT_BOUND notification
sent if a driver fails to bind to a device.  Extend IOMMU group
notifications to include a version of this.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Cc: Joerg Roedel <joro@8bytes.org>
---
 drivers/iommu/iommu.c |    2 ++
 include/linux/iommu.h |    1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3b67144dead2..cf6051db4208 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1113,6 +1113,8 @@ static int iommu_bus_notifier(struct notifier_block *nb,
 	case BUS_NOTIFY_UNBOUND_DRIVER:
 		group_action = IOMMU_GROUP_NOTIFY_UNBOUND_DRIVER;
 		break;
+	case BUS_NOTIFY_DRIVER_NOT_BOUND:
+		group_action = IOMMU_GROUP_NOTIFY_DRIVER_NOT_BOUND;
 	}
 
 	if (group_action)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 2e4de0deee53..54a0eb96da25 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -268,6 +268,7 @@ static inline void iommu_device_set_fwnode(struct iommu_device *iommu,
 #define IOMMU_GROUP_NOTIFY_BOUND_DRIVER		4 /* Post Driver bind */
 #define IOMMU_GROUP_NOTIFY_UNBIND_DRIVER	5 /* Pre Driver unbind */
 #define IOMMU_GROUP_NOTIFY_UNBOUND_DRIVER	6 /* Post Driver unbind */
+#define IOMMU_GROUP_NOTIFY_DRIVER_NOT_BOUND	7 /* Driver bind failed */
 
 extern int bus_set_iommu(struct bus_type *bus, const struct iommu_ops *ops);
 extern bool iommu_present(struct bus_type *bus);

[PATCH 5/7] vfio: Create interface for vfio bus drivers to register

[Alex Williamson]

Generally we don't know about vfio bus drivers until a device is
added to the vfio-core with vfio_add_group_dev(), this optional
registration with vfio_register_bus_driver() allows vfio-core to
track known drivers.  Our current use for this information is to
know whether a driver is vfio compatible during a bind operation.
For devices on buses with driver_override support, we can use this
linkage to block non-vfio drivers from binding to devices where
the iommu group state would trigger a BUG to avoid host/user
integrity issues.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/vfio/vfio.c  |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/vfio.h |    3 +++
 2 files changed, 58 insertions(+)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index e6117de60f87..a25ee4930200 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -43,6 +43,8 @@
 	struct class			*class;
 	struct list_head		iommu_drivers_list;
 	struct mutex			iommu_drivers_lock;
+	struct list_head		bus_drivers_list;
+	struct mutex			bus_drivers_lock;
 	struct list_head		group_list;
 	struct idr			group_idr;
 	struct mutex			group_lock;
@@ -51,6 +53,11 @@
 	wait_queue_head_t		release_q;
 } vfio;
 
+struct vfio_bus_driver {
+	struct device_driver		*drv;
+	struct list_head		vfio_next;
+};
+
 struct vfio_iommu_driver {
 	const struct vfio_iommu_driver_ops	*ops;
 	struct list_head			vfio_next;
@@ -2243,6 +2250,52 @@ int vfio_unregister_notifier(struct device *dev, enum vfio_notify_type type,
 }
 EXPORT_SYMBOL(vfio_unregister_notifier);
 
+int vfio_register_bus_driver(struct device_driver *drv)
+{
+	struct vfio_bus_driver *driver, *tmp;
+
+	driver = kzalloc(sizeof(*driver), GFP_KERNEL);
+	if (!driver)
+		return -ENOMEM;
+
+	driver->drv = drv;
+
+	mutex_lock(&vfio.bus_drivers_lock);
+
+	/* Check for duplicates */
+	list_for_each_entry(tmp, &vfio.bus_drivers_list, vfio_next) {
+		if (tmp->drv == drv) {
+			mutex_unlock(&vfio.bus_drivers_lock);
+			kfree(driver);
+			return -EINVAL;
+		}
+	}
+
+	list_add(&driver->vfio_next, &vfio.bus_drivers_list);
+
+	mutex_unlock(&vfio.bus_drivers_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vfio_register_bus_driver);
+
+void vfio_unregister_bus_driver(struct device_driver *drv)
+{
+	struct vfio_bus_driver *driver;
+
+	mutex_lock(&vfio.bus_drivers_lock);
+	list_for_each_entry(driver, &vfio.bus_drivers_list, vfio_next) {
+		if (driver->drv == drv) {
+			list_del(&driver->vfio_next);
+			mutex_unlock(&vfio.bus_drivers_lock);
+			kfree(driver);
+			return;
+		}
+	}
+	mutex_unlock(&vfio.bus_drivers_lock);
+}
+EXPORT_SYMBOL_GPL(vfio_unregister_bus_driver);
+
 /**
  * Module/class support
  */
@@ -2266,8 +2319,10 @@ static int __init vfio_init(void)
 	idr_init(&vfio.group_idr);
 	mutex_init(&vfio.group_lock);
 	mutex_init(&vfio.iommu_drivers_lock);
+	mutex_init(&vfio.bus_drivers_lock);
 	INIT_LIST_HEAD(&vfio.group_list);
 	INIT_LIST_HEAD(&vfio.iommu_drivers_list);
+	INIT_LIST_HEAD(&vfio.bus_drivers_list);
 	init_waitqueue_head(&vfio.release_q);
 
 	ret = misc_register(&vfio_dev);
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 9b34d0af5d27..dab0f8105e4a 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -92,6 +92,9 @@ struct vfio_iommu_driver_ops {
 extern void vfio_unregister_iommu_driver(
 				const struct vfio_iommu_driver_ops *ops);
 
+extern int vfio_register_bus_driver(struct device_driver *drv);
+extern void vfio_unregister_bus_driver(struct device_driver *drv);
+
 /*
  * External user API
  */

[PATCH 6/7] vfio: Register pci, platform, amba, and mdev bus drivers

[Alex Williamson]

Hook into vfio bus driver register/unregister support.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Cc: Baptiste Reynal <b.reynal@virtualopensystems.com>
Cc: Kirti Wankhede <kwankhede@nvidia.com>
Cc: Eric Auger <eric.auger@redhat.com>
---
 drivers/vfio/mdev/vfio_mdev.c         |   13 ++++++++++++-
 drivers/vfio/pci/vfio_pci.c           |    7 +++++++
 drivers/vfio/platform/vfio_amba.c     |   24 +++++++++++++++++++++++-
 drivers/vfio/platform/vfio_platform.c |   24 +++++++++++++++++++++++-
 4 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
index fa848a701b8b..b73d6f3e8ad5 100644
--- a/drivers/vfio/mdev/vfio_mdev.c
+++ b/drivers/vfio/mdev/vfio_mdev.c
@@ -131,11 +131,22 @@ struct mdev_driver vfio_mdev_driver = {
 
 static int __init vfio_mdev_init(void)
 {
-	return mdev_register_driver(&vfio_mdev_driver, THIS_MODULE);
+	int ret;
+
+	ret = mdev_register_driver(&vfio_mdev_driver, THIS_MODULE);
+	if (ret)
+		return ret;
+
+	ret = vfio_register_bus_driver(&vfio_mdev_driver.driver);
+	if (ret)
+		mdev_unregister_driver(&vfio_mdev_driver);
+
+	return ret;
 }
 
 static void __exit vfio_mdev_exit(void)
 {
+	vfio_unregister_bus_driver(&vfio_mdev_driver.driver);
 	mdev_unregister_driver(&vfio_mdev_driver);
 }
 
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 324c52e3a1a4..15f4b190ad88 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -1397,6 +1397,7 @@ static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev)
 
 static void __exit vfio_pci_cleanup(void)
 {
+	vfio_unregister_bus_driver(&vfio_pci_driver.driver);
 	pci_unregister_driver(&vfio_pci_driver);
 	vfio_pci_uninit_perm_bits();
 }
@@ -1456,6 +1457,12 @@ static int __init vfio_pci_init(void)
 	if (ret)
 		goto out_driver;
 
+	ret = vfio_register_bus_driver(&vfio_pci_driver.driver);
+	if (ret) {
+		pci_unregister_driver(&vfio_pci_driver);
+		goto out_driver;
+	}
+
 	vfio_pci_fill_ids();
 
 	return 0;
diff --git a/drivers/vfio/platform/vfio_amba.c b/drivers/vfio/platform/vfio_amba.c
index 31372fbf6c5b..7fd9cb4a6756 100644
--- a/drivers/vfio/platform/vfio_amba.c
+++ b/drivers/vfio/platform/vfio_amba.c
@@ -109,7 +109,29 @@ static int vfio_amba_remove(struct amba_device *adev)
 	},
 };
 
-module_amba_driver(vfio_amba_driver);
+static void __exit vfio_amba_exit(void)
+{
+	vfio_unregister_bus_driver(&vfio_amba_driver.drv);
+	amba_driver_unregister(&vfio_amba_driver);
+}
+
+static int __init vfio_amba_init(void)
+{
+	int ret;
+
+	ret = amba_driver_register(&vfio_amba_driver);
+	if (ret)
+		return ret;
+
+	ret = vfio_register_bus_driver(&vfio_amba_driver.drv);
+	if (ret)
+		amba_driver_unregister(&vfio_amba_driver);
+
+	return ret;
+}
+
+module_init(vfio_amba_init);
+module_exit(vfio_amba_exit);
 
 MODULE_VERSION(DRIVER_VERSION);
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
index 6561751a1063..3974dc65e6dc 100644
--- a/drivers/vfio/platform/vfio_platform.c
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -100,7 +100,29 @@ static int vfio_platform_remove(struct platform_device *pdev)
 	},
 };
 
-module_platform_driver(vfio_platform_driver);
+static void __exit vfio_platform_exit(void)
+{
+	vfio_unregister_bus_driver(&vfio_platform_driver.driver);
+	platform_driver_unregister(&vfio_platform_driver);
+}
+
+static int __init vfio_platform_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&vfio_platform_driver);
+	if (ret)
+		return ret;
+
+	ret = vfio_register_bus_driver(&vfio_platform_driver.driver);
+	if (ret)
+		platform_driver_unregister(&vfio_platform_driver);
+
+	return ret;
+}
+
+module_init(vfio_platform_init);
+module_exit(vfio_platform_exit);
 
 MODULE_VERSION(DRIVER_VERSION);
 MODULE_LICENSE("GPL v2");

[PATCH 7/7] vfio: Use driver_override to avert binding to compromising drivers

[Alex Williamson]

If a device is bound to a non-vfio, non-whitelisted driver while a
group is in use, then the integrity of the group is compromised and
will result in hitting a BUG_ON.  This code tries to avoid this case
by mangling driver_override to force a no-match for the driver.  The
driver-core will either follow-up with a DRIVER_NOT_BOUND (preferred)
or BOUND_DRIVER, at which point we can remove the driver_override
mangling.

A complication here is that even though the window between these
notifications is expected to be extremely small, the vfio group could
be removed, which would prevent us from finding the group again to
remove the driver_override.  We therefore take a group reference when
adding to driver_override and release it when removed.  A second
complication is that driver_override can be modified by the system
admin through sysfs.  To avoid trivial interference, we add a non-
user-visible UUID to the group and use this as part of the mangle
string.

The above blocks binding to a driver that would compromise the host,
but we'd also like to avoid reaching that step if possible.  For this
we add a wait_event_timeout() with a short, 1 second timeout, which is
highly effective in allowing empty groups to finish cleanup.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/vfio/vfio.c |  151 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 144 insertions(+), 7 deletions(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index a25ee4930200..ea786d512faa 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -25,6 +25,7 @@
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/platform_device.h>
 #include <linux/pci.h>
 #include <linux/rwsem.h>
 #include <linux/sched.h>
@@ -32,8 +33,10 @@
 #include <linux/stat.h>
 #include <linux/string.h>
 #include <linux/uaccess.h>
+#include <linux/uuid.h>
 #include <linux/vfio.h>
 #include <linux/wait.h>
+#include <linux/amba/bus.h>
 
 #define DRIVER_VERSION	"0.3"
 #define DRIVER_AUTHOR	"Alex Williamson <alex.williamson@redhat.com>"
@@ -95,6 +98,7 @@ struct vfio_group {
 	bool				noiommu;
 	struct kvm			*kvm;
 	struct blocking_notifier_head	notifier;
+	unsigned char			uuid[16];
 };
 
 struct vfio_device {
@@ -352,6 +356,8 @@ static struct vfio_group *vfio_create_group(struct iommu_group *iommu_group)
 
 	group->nb.notifier_call = vfio_iommu_group_notifier;
 
+	generate_random_uuid(group->uuid);
+
 	/*
 	 * blocking notifiers acquire a rwsem around registering and hold
 	 * it around callback.  Therefore, need to register outside of
@@ -728,6 +734,116 @@ static int vfio_group_nb_verify(struct vfio_group *group, struct device *dev)
 	return vfio_dev_viable(dev, group);
 }
 
+#define VFIO_TAG_PREFIX "#vfio_group:"
+
+static char **vfio_find_driver_override(struct device *dev)
+{
+	if (dev_is_pci(dev)) {
+		struct pci_dev *pdev = to_pci_dev(dev);
+		return &pdev->driver_override;
+	} else if (dev->bus == &platform_bus_type) {
+		struct platform_device *pdev = to_platform_device(dev);
+		return &pdev->driver_override;
+#ifdef CONFIG_ARM_AMBA
+	} else if (dev->bus == &amba_bustype) {
+		struct amba_device *adev = to_amba_device(dev);
+		return &adev->driver_override;
+#endif
+	}
+
+	return NULL;
+}
+
+/*
+ * If we're about to bind to something other than a known whitelisted driver
+ * or known vfio bus driver, try to avert it with driver_override.
+ */
+static void vfio_group_nb_pre_bind(struct vfio_group *group, struct device *dev)
+{
+	struct vfio_bus_driver *driver;
+	struct device_driver *drv = ACCESS_ONCE(dev->driver);
+	char **driver_override;
+
+	if (vfio_dev_whitelisted(dev, drv))
+		return; /* Binding to known "innocuous" device/driver */
+
+	mutex_lock(&vfio.bus_drivers_lock);
+	list_for_each_entry(driver, &vfio.bus_drivers_list, vfio_next) {
+		if (driver->drv == drv) {
+			mutex_unlock(&vfio.bus_drivers_lock);
+			return; /* Binding to known vfio bus driver, ok */
+		}
+	}
+	mutex_unlock(&vfio.bus_drivers_lock);
+
+	/* Can we stall slightly to let users fall off? */
+	if (list_empty(&group->device_list)) {
+		if (wait_event_timeout(vfio.release_q,
+				!atomic_read(&group->container_users), HZ))
+			return;
+	}
+
+	driver_override = vfio_find_driver_override(dev);
+	if (driver_override) {
+		char tag[50], *new = NULL, *old = *driver_override;
+
+		snprintf(tag, sizeof(tag), "%s%pU",
+			 VFIO_TAG_PREFIX, group->uuid);
+
+		if (old && strstr(old, tag))
+			return; /* block already in place */
+
+		new = kasprintf(GFP_KERNEL, "%s%s", old ? old : "", tag);
+		if (new) {
+			*driver_override = new;
+			kfree(old);
+			vfio_group_get(group);
+			dev_warn(dev, "vfio: Blocking unsafe driver bind\n");
+			return;
+		}
+	}
+
+	dev_warn(dev, "vfio: Unsafe driver binding to in-use group!\n");
+}
+
+/* If we've mangled driver_override, remove it */
+static void vfio_group_nb_post_bind(struct vfio_group *group,
+				    struct device *dev)
+{
+	char **driver_override = vfio_find_driver_override(dev);
+
+	if (driver_override && *driver_override) {
+		char tag[50], *new, *start, *end, *old = *driver_override;
+
+		snprintf(tag, sizeof(tag), "%s%pU",
+			 VFIO_TAG_PREFIX, group->uuid);
+
+		start = strstr(old, tag);
+		if (start) {
+			end = start + strlen(tag);
+
+			if (old + strlen(old) > end)
+				memmove(start, end,
+					strlen(old) - (end - old) + 1);
+			else
+				*start = 0;
+
+			if (strlen(old)) {
+				new = kasprintf(GFP_KERNEL, "%s", old);
+				if (new) {
+					*driver_override = new;
+					kfree(old);
+				} /* else, in-place terminated, ok */
+			} else {
+				*driver_override = NULL;
+				kfree(old);
+			}
+
+			vfio_group_put(group);
+		}
+	}
+}
+
 static int vfio_iommu_group_notifier(struct notifier_block *nb,
 				     unsigned long action, void *data)
 {
@@ -757,14 +873,23 @@ static int vfio_iommu_group_notifier(struct notifier_block *nb,
 		 */
 		break;
 	case IOMMU_GROUP_NOTIFY_BIND_DRIVER:
-		pr_debug("%s: Device %s, group %d binding to driver\n",
+		pr_debug("%s: Device %s, group %d binding to driver %s\n",
 			 __func__, dev_name(dev),
-			 iommu_group_id(group->iommu_group));
+			 iommu_group_id(group->iommu_group), dev->driver->name);
+		if (vfio_group_nb_verify(group, dev))
+			vfio_group_nb_pre_bind(group, dev);
+		break;
+	case IOMMU_GROUP_NOTIFY_DRIVER_NOT_BOUND:
+		pr_debug("%s: Device %s, group %d binding fail to driver %s\n",
+			 __func__, dev_name(dev),
+			 iommu_group_id(group->iommu_group), dev->driver->name);
+		vfio_group_nb_post_bind(group, dev);
 		break;
 	case IOMMU_GROUP_NOTIFY_BOUND_DRIVER:
 		pr_debug("%s: Device %s, group %d bound to driver %s\n",
 			 __func__, dev_name(dev),
 			 iommu_group_id(group->iommu_group), dev->driver->name);
+		vfio_group_nb_post_bind(group, dev);
 		BUG_ON(vfio_group_nb_verify(group, dev));
 		break;
 	case IOMMU_GROUP_NOTIFY_UNBIND_DRIVER:
@@ -1351,6 +1476,7 @@ static int vfio_group_unset_container(struct vfio_group *group)
 	if (users != 1)
 		return -EBUSY;
 
+	wake_up(&vfio.release_q);
 	__vfio_group_unset_container(group);
 
 	return 0;
@@ -1364,7 +1490,11 @@ static int vfio_group_unset_container(struct vfio_group *group)
  */
 static void vfio_group_try_dissolve_container(struct vfio_group *group)
 {
-	if (0 == atomic_dec_if_positive(&group->container_users))
+	int users = atomic_dec_if_positive(&group->container_users);
+
+	wake_up(&vfio.release_q);
+
+	if (!users)
 		__vfio_group_unset_container(group);
 }
 
@@ -1433,19 +1563,26 @@ static bool vfio_group_viable(struct vfio_group *group)
 
 static int vfio_group_add_container_user(struct vfio_group *group)
 {
+	int ret;
+
 	if (!atomic_inc_not_zero(&group->container_users))
 		return -EINVAL;
 
 	if (group->noiommu) {
-		atomic_dec(&group->container_users);
-		return -EPERM;
+		ret = -EPERM;
+		goto out;
 	}
 	if (!group->container->iommu_driver || !vfio_group_viable(group)) {
-		atomic_dec(&group->container_users);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
 	return 0;
+
+out:
+	atomic_dec(&group->container_users);
+	wake_up(&vfio.release_q);
+	return ret;
 }
 
 static const struct file_operations vfio_device_fops;

Re: [PATCH 4/7] iommu: Add driver-not-bound notification

[Joerg Roedel]

On Fri, Jun 09, 2017 at 03:59:59PM -0600, Alex Williamson wrote:
> The driver core supports a BUS_NOTIFY_DRIVER_NOT_BOUND notification
> sent if a driver fails to bind to a device.  Extend IOMMU group
> notifications to include a version of this.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Cc: Joerg Roedel <joro@8bytes.org>

Since this is part of a larger patch-set I assume it won't go through
iommu-tree, so:

	Acked-by: Joerg Roedel <jroedel@suse.de>

Re: [PATCH 4/7] iommu: Add driver-not-bound notification

[Alex Williamson]

On Sat, 10 Jun 2017 00:21:33 +0200
Joerg Roedel <joro@8bytes.org> wrote:

> On Fri, Jun 09, 2017 at 03:59:59PM -0600, Alex Williamson wrote:
> > The driver core supports a BUS_NOTIFY_DRIVER_NOT_BOUND notification
> > sent if a driver fails to bind to a device.  Extend IOMMU group
> > notifications to include a version of this.
> > 
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Joerg Roedel <joro@8bytes.org>  
> 
> Since this is part of a larger patch-set I assume it won't go through
> iommu-tree, so:
> 
> 	Acked-by: Joerg Roedel <jroedel@suse.de>
> 

Thanks Joerg!  Yes, I was hoping for your ack so I could pull it
through the vfio tree.  I also note that I added a new case, but missed
the break.  It doesn't change the code, but I think it makes it more
maintainable to include it.  If there are no objections I'll make this
trivial update to the commit:

commit 95c30f6a55638edaf9673f451c0050441cf18ab8
Author: Alex Williamson <alex.williamson@redhat.com>
Date:   Sat Jun 3 13:00:06 2017 -0600

    iommu: Add driver-not-bound notification
    
    The driver core supports a BUS_NOTIFY_DRIVER_NOT_BOUND notification
    sent if a driver fails to bind to a device.  Extend IOMMU group
    notifications to include a version of this.
    
    Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
    Acked-by: Joerg Roedel <jroedel@suse.de>

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3b67144dead2..7043f95ee43d 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1113,6 +1113,9 @@ static int iommu_bus_notifier(struct notifier_block *nb,
 	case BUS_NOTIFY_UNBOUND_DRIVER:
 		group_action = IOMMU_GROUP_NOTIFY_UNBOUND_DRIVER;
 		break;
+	case BUS_NOTIFY_DRIVER_NOT_BOUND:
+		group_action = IOMMU_GROUP_NOTIFY_DRIVER_NOT_BOUND;
+		break;
 	}
 
 	if (group_action)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 2e4de0deee53..54a0eb96da25 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -268,6 +268,7 @@ static inline void iommu_device_set_fwnode(struct iommu_device *iommu,
 #define IOMMU_GROUP_NOTIFY_BOUND_DRIVER		4 /* Post Driver bind */
 #define IOMMU_GROUP_NOTIFY_UNBIND_DRIVER	5 /* Pre Driver unbind */
 #define IOMMU_GROUP_NOTIFY_UNBOUND_DRIVER	6 /* Post Driver unbind */
+#define IOMMU_GROUP_NOTIFY_DRIVER_NOT_BOUND	7 /* Driver bind failed */
 
 extern int bus_set_iommu(struct bus_type *bus, const struct iommu_ops *ops);
 extern bool iommu_present(struct bus_type *bus);

Re: [PATCH 4/7] iommu: Add driver-not-bound notification

[Joerg Roedel]

On Fri, Jun 09, 2017 at 04:43:34PM -0600, Alex Williamson wrote:
> +	case BUS_NOTIFY_DRIVER_NOT_BOUND:
> +		group_action = IOMMU_GROUP_NOTIFY_DRIVER_NOT_BOUND;
> +		break;

Ah yes, I missed that too when looking at the patch. Still Acked-by-Me
:)


	Joerg

Re: [PATCH 3/7] vfio: New external user group/file match

[Paolo Bonzini]

On 09/06/2017 23:59, Alex Williamson wrote:
> At the point where the kvm-vfio pseudo device wants to release its
> vfio group reference, we can't always acquire a new reference to make
> that happen.  The group can be in a state where we wouldn't allow a
> new reference to be added.  This new helper function allows a caller
> to match a file to a group to facilitate this.  Given a file and
> group, report if they match.  Thus the caller needs to already have a
> group reference to match to the file.  This allows the deletion of a
> group without acquiring a new reference.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  drivers/vfio/vfio.c  |    9 +++++++++
>  include/linux/vfio.h |    2 ++
>  virt/kvm/vfio.c      |   27 +++++++++++++++++++--------
>  3 files changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index f2e24d5699f2..e6117de60f87 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1776,6 +1776,15 @@ void vfio_group_put_external_user(struct vfio_group *group)
>  }
>  EXPORT_SYMBOL_GPL(vfio_group_put_external_user);
>  
> +bool vfio_external_group_match_file(struct vfio_group *test_group,
> +				    struct file *filep)
> +{
> +	struct vfio_group *group = filep->private_data;
> +
> +	return (filep->f_op == &vfio_group_fops) && (group == test_group);
> +}
> +EXPORT_SYMBOL_GPL(vfio_external_group_match_file);
> +
>  int vfio_external_user_iommu_id(struct vfio_group *group)
>  {
>  	return iommu_group_id(group->iommu_group);
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index edf9b2cad277..9b34d0af5d27 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -97,6 +97,8 @@ extern void vfio_unregister_iommu_driver(
>   */
>  extern struct vfio_group *vfio_group_get_external_user(struct file *filep);
>  extern void vfio_group_put_external_user(struct vfio_group *group);
> +extern bool vfio_external_group_match_file(struct vfio_group *group,
> +					   struct file *filep);
>  extern int vfio_external_user_iommu_id(struct vfio_group *group);
>  extern long vfio_external_check_extension(struct vfio_group *group,
>  					  unsigned long arg);
> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> index db9036ef8c73..7c14729b1f2c 100644
> --- a/virt/kvm/vfio.c
> +++ b/virt/kvm/vfio.c
> @@ -47,6 +47,22 @@ static struct vfio_group *kvm_vfio_group_get_external_user(struct file *filep)
>  	return vfio_group;
>  }
>  
> +static bool kvm_vfio_external_group_match_file(struct vfio_group *group,
> +					       struct file *filep)
> +{
> +	bool ret, (*fn)(struct vfio_group *, struct file *);
> +
> +	fn = symbol_get(vfio_external_group_match_file);
> +	if (!fn)
> +		return false;
> +
> +	ret = fn(group, filep);
> +
> +	symbol_put(vfio_external_group_match_file);
> +
> +	return ret;
> +}
> +
>  static void kvm_vfio_group_put_external_user(struct vfio_group *vfio_group)
>  {
>  	void (*fn)(struct vfio_group *);
> @@ -186,18 +202,13 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>  		if (!f.file)
>  			return -EBADF;
>  
> -		vfio_group = kvm_vfio_group_get_external_user(f.file);
> -		fdput(f);
> -
> -		if (IS_ERR(vfio_group))
> -			return PTR_ERR(vfio_group);
> -
>  		ret = -ENOENT;
>  
>  		mutex_lock(&kv->lock);
>  
>  		list_for_each_entry(kvg, &kv->group_list, node) {
> -			if (kvg->vfio_group != vfio_group)
> +			if (!kvm_vfio_external_group_match_file(kvg->vfio_group,
> +								f.file))
>  				continue;
>  
>  			list_del(&kvg->node);
> @@ -211,7 +222,7 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>  
>  		mutex_unlock(&kv->lock);
>  
> -		kvm_vfio_group_put_external_user(vfio_group);
> +		fdput(f);
>  
>  		kvm_vfio_update_coherency(dev);
>  
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [PATCH 2/7] kvm-vfio: Decouple only when we match a group

[Paolo Bonzini]

On 09/06/2017 23:59, Alex Williamson wrote:
> Unset-KVM and decrement-assignment only when we find the group in our
> list.  Otherwise we can get out of sync if the user triggers this for
> groups that aren't currently on our list.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  virt/kvm/vfio.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> index d32f239eb471..db9036ef8c73 100644
> --- a/virt/kvm/vfio.c
> +++ b/virt/kvm/vfio.c
> @@ -201,18 +201,16 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>  				continue;
>  
>  			list_del(&kvg->node);
> +			kvm_arch_end_assignment(dev->kvm);
> +			kvm_vfio_group_set_kvm(kvg->vfio_group, NULL);
>  			kvm_vfio_group_put_external_user(kvg->vfio_group);
>  			kfree(kvg);
>  			ret = 0;
>  			break;
>  		}
>  
> -		kvm_arch_end_assignment(dev->kvm);
> -
>  		mutex_unlock(&kv->lock);
>  
> -		kvm_vfio_group_set_kvm(vfio_group, NULL);
> -
>  		kvm_vfio_group_put_external_user(vfio_group);
>  
>  		kvm_vfio_update_coherency(dev);
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Should these two patches be applied to stable kernels too?  In any case,
please take care your self of getting these to Linus!

Thanks,

Paolo

Re: [PATCH 1/7] vfio: Fix group release deadlock

[Auger Eric]

Hi Alex,

On 09/06/2017 23:59, Alex Williamson wrote:
> If vfio_iommu_group_notifier() acquires a group reference and that
> reference becomes the last reference to the group, then vfio_group_put
> introduces a deadlock code path where we're trying to unregister from
> the iommu notifier chain from within a callout of that chain.  Use a
> work_struct to release this reference asynchronously.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  drivers/vfio/vfio.c |   37 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 561084ab387f..f2e24d5699f2 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -423,6 +423,34 @@ static void vfio_group_put(struct vfio_group *group)
>  	kref_put_mutex(&group->kref, vfio_group_release, &vfio.group_lock);
>  }
>  
> +struct vfio_group_put_work {
> +	struct work_struct work;
> +	struct vfio_group *group;
> +};
> +
> +static void vfio_group_put_bg(struct work_struct *work)
> +{
> +	struct vfio_group_put_work *do_work;
> +
> +	do_work = container_of(work, struct vfio_group_put_work, work);
> +
> +	vfio_group_put(do_work->group);
> +	kfree(do_work);
> +}
> +
> +static void vfio_group_schedule_put(struct vfio_group *group)
> +{
> +	struct vfio_group_put_work *do_work;
> +
> +	do_work = kmalloc(sizeof(*do_work), GFP_KERNEL);
> +	if (WARN_ON(!do_work))
> +		return;
> +
> +	INIT_WORK(&do_work->work, vfio_group_put_bg);
> +	do_work->group = group;
> +	schedule_work(&do_work->work);
> +}
> +
>  /* Assume group_lock or group reference is held */
>  static void vfio_group_get(struct vfio_group *group)
>  {
> @@ -762,7 +790,14 @@ static int vfio_iommu_group_notifier(struct notifier_block *nb,
>  		break;
>  	}
>  
> -	vfio_group_put(group);
> +	/*
> +	 * If we're the last reference to the group, the group will be
> +	 * released, which includes unregistering the iommu group notifier.
> +	 * We hold a read-lock on that notifier list, unregistering needs
> +	 * a write-lock... deadlock.  Release our reference asyncronously
nit: s/asyncronously/asynchronously

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric


> +	 * to avoid that situation.
> +	 */
> +	vfio_group_schedule_put(group);
>  	return NOTIFY_OK;
>  }
>  
> 

Re: [PATCH 2/7] kvm-vfio: Decouple only when we match a group

[Auger Eric]

Hi Alex,

On 09/06/2017 23:59, Alex Williamson wrote:
> Unset-KVM and decrement-assignment only when we find the group in our
> list.  Otherwise we can get out of sync if the user triggers this for
> groups that aren't currently on our list.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  virt/kvm/vfio.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
This patch and 3/7 do not apply on 4.12-rc5 as there are small conflicts
with
commit 121f80ba68f1a5779a36d7b3247206e60e0a7418
KVM: PPC: VFIO: Add in-kernel acceleration for VFIO

Besides

Reviewed-by: Eric Auger <eric.auger@redhat.com>


Thanks

Eric
> 
> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> index d32f239eb471..db9036ef8c73 100644
> --- a/virt/kvm/vfio.c
> +++ b/virt/kvm/vfio.c
> @@ -201,18 +201,16 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>  				continue;
>  
>  			list_del(&kvg->node);
> +			kvm_arch_end_assignment(dev->kvm);
> +			kvm_vfio_group_set_kvm(kvg->vfio_group, NULL);
>  			kvm_vfio_group_put_external_user(kvg->vfio_group);
>  			kfree(kvg);
>  			ret = 0;
>  			break;
>  		}
>  
> -		kvm_arch_end_assignment(dev->kvm);
> -
>  		mutex_unlock(&kv->lock);
>  
> -		kvm_vfio_group_set_kvm(vfio_group, NULL);
> -
>  		kvm_vfio_group_put_external_user(vfio_group);
>  
>  		kvm_vfio_update_coherency(dev);
> 

Re: [PATCH 3/7] vfio: New external user group/file match

[Auger Eric]

Hi,
On 09/06/2017 23:59, Alex Williamson wrote:
> At the point where the kvm-vfio pseudo device wants to release its
> vfio group reference, we can't always acquire a new reference to make
> that happen.  The group can be in a state where we wouldn't allow a
> new reference to be added.  This new helper function allows a caller
> to match a file to a group to facilitate this.  Given a file and
> group, report if they match.  Thus the caller needs to already have a
> group reference to match to the file.  This allows the deletion of a
> group without acquiring a new reference.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  drivers/vfio/vfio.c  |    9 +++++++++
>  include/linux/vfio.h |    2 ++
>  virt/kvm/vfio.c      |   27 +++++++++++++++++++--------
>  3 files changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index f2e24d5699f2..e6117de60f87 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1776,6 +1776,15 @@ void vfio_group_put_external_user(struct vfio_group *group)
>  }
>  EXPORT_SYMBOL_GPL(vfio_group_put_external_user);
>  
> +bool vfio_external_group_match_file(struct vfio_group *test_group,
> +				    struct file *filep)
> +{
> +	struct vfio_group *group = filep->private_data;
> +
> +	return (filep->f_op == &vfio_group_fops) && (group == test_group);
> +}
> +EXPORT_SYMBOL_GPL(vfio_external_group_match_file);
> +
>  int vfio_external_user_iommu_id(struct vfio_group *group)
>  {
>  	return iommu_group_id(group->iommu_group);
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index edf9b2cad277..9b34d0af5d27 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -97,6 +97,8 @@ extern void vfio_unregister_iommu_driver(
>   */
>  extern struct vfio_group *vfio_group_get_external_user(struct file *filep);
>  extern void vfio_group_put_external_user(struct vfio_group *group);
> +extern bool vfio_external_group_match_file(struct vfio_group *group,
> +					   struct file *filep);
>  extern int vfio_external_user_iommu_id(struct vfio_group *group);
>  extern long vfio_external_check_extension(struct vfio_group *group,
>  					  unsigned long arg);
> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> index db9036ef8c73..7c14729b1f2c 100644
> --- a/virt/kvm/vfio.c
> +++ b/virt/kvm/vfio.c
> @@ -47,6 +47,22 @@ static struct vfio_group *kvm_vfio_group_get_external_user(struct file *filep)
>  	return vfio_group;
>  }
>  
> +static bool kvm_vfio_external_group_match_file(struct vfio_group *group,
> +					       struct file *filep)
> +{
> +	bool ret, (*fn)(struct vfio_group *, struct file *);
> +
> +	fn = symbol_get(vfio_external_group_match_file);
> +	if (!fn)
> +		return false;
> +
> +	ret = fn(group, filep);
> +
> +	symbol_put(vfio_external_group_match_file);
> +
> +	return ret;
> +}
> +
>  static void kvm_vfio_group_put_external_user(struct vfio_group *vfio_group)
>  {
>  	void (*fn)(struct vfio_group *);
> @@ -186,18 +202,13 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>  		if (!f.file)
>  			return -EBADF;
>  
> -		vfio_group = kvm_vfio_group_get_external_user(f.file);
> -		fdput(f);
> -
> -		if (IS_ERR(vfio_group))
> -			return PTR_ERR(vfio_group);
> -
>  		ret = -ENOENT;
>  
>  		mutex_lock(&kv->lock);
>  
>  		list_for_each_entry(kvg, &kv->group_list, node) {
> -			if (kvg->vfio_group != vfio_group)
> +			if (!kvm_vfio_external_group_match_file(kvg->vfio_group,
> +								f.file))
>  				continue;
>  
>  			list_del(&kvg->node);
> @@ -211,7 +222,7 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>  
>  		mutex_unlock(&kv->lock);
>  
> -		kvm_vfio_group_put_external_user(vfio_group);
> +		fdput(f);
>  
>  		kvm_vfio_update_coherency(dev);
>  
> 

Re: [PATCH 4/7] iommu: Add driver-not-bound notification

[Auger Eric]

Hi,
On 09/06/2017 23:59, Alex Williamson wrote:
> The driver core supports a BUS_NOTIFY_DRIVER_NOT_BOUND notification
> sent if a driver fails to bind to a device.  Extend IOMMU group
> notifications to include a version of this.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> ---
>  drivers/iommu/iommu.c |    2 ++
>  include/linux/iommu.h |    1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 3b67144dead2..cf6051db4208 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1113,6 +1113,8 @@ static int iommu_bus_notifier(struct notifier_block *nb,
>  	case BUS_NOTIFY_UNBOUND_DRIVER:
>  		group_action = IOMMU_GROUP_NOTIFY_UNBOUND_DRIVER;
>  		break;
> +	case BUS_NOTIFY_DRIVER_NOT_BOUND:
> +		group_action = IOMMU_GROUP_NOTIFY_DRIVER_NOT_BOUND;
so with the break ;-)

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
>  	}
>  
>  	if (group_action)
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 2e4de0deee53..54a0eb96da25 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -268,6 +268,7 @@ static inline void iommu_device_set_fwnode(struct iommu_device *iommu,
>  #define IOMMU_GROUP_NOTIFY_BOUND_DRIVER		4 /* Post Driver bind */
>  #define IOMMU_GROUP_NOTIFY_UNBIND_DRIVER	5 /* Pre Driver unbind */
>  #define IOMMU_GROUP_NOTIFY_UNBOUND_DRIVER	6 /* Post Driver unbind */
> +#define IOMMU_GROUP_NOTIFY_DRIVER_NOT_BOUND	7 /* Driver bind failed */
>  
>  extern int bus_set_iommu(struct bus_type *bus, const struct iommu_ops *ops);
>  extern bool iommu_present(struct bus_type *bus);
> 

Re: [PATCH 5/7] vfio: Create interface for vfio bus drivers to register

[Auger Eric]

Hi,

On 10/06/2017 00:00, Alex Williamson wrote:
> Generally we don't know about vfio bus drivers until a device is
> added to the vfio-core with vfio_add_group_dev(), this optional
> registration with vfio_register_bus_driver() allows vfio-core to
> track known drivers.  Our current use for this information is to
> know whether a driver is vfio compatible during a bind operation.
> For devices on buses with driver_override support, we can use this
> linkage to block non-vfio drivers from binding to devices where
> the iommu group state would trigger a BUG to avoid host/user
> integrity issues.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  drivers/vfio/vfio.c  |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/vfio.h |    3 +++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index e6117de60f87..a25ee4930200 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -43,6 +43,8 @@
>  	struct class			*class;
>  	struct list_head		iommu_drivers_list;
>  	struct mutex			iommu_drivers_lock;
> +	struct list_head		bus_drivers_list;
> +	struct mutex			bus_drivers_lock;
>  	struct list_head		group_list;
>  	struct idr			group_idr;
>  	struct mutex			group_lock;
> @@ -51,6 +53,11 @@
>  	wait_queue_head_t		release_q;
>  } vfio;
>  
> +struct vfio_bus_driver {
> +	struct device_driver		*drv;
> +	struct list_head		vfio_next;
> +};
> +
>  struct vfio_iommu_driver {
>  	const struct vfio_iommu_driver_ops	*ops;
>  	struct list_head			vfio_next;
> @@ -2243,6 +2250,52 @@ int vfio_unregister_notifier(struct device *dev, enum vfio_notify_type type,
>  }
>  EXPORT_SYMBOL(vfio_unregister_notifier);
>  
> +int vfio_register_bus_driver(struct device_driver *drv)
> +{
> +	struct vfio_bus_driver *driver, *tmp;
> +
> +	driver = kzalloc(sizeof(*driver), GFP_KERNEL);
> +	if (!driver)
> +		return -ENOMEM;
> +
> +	driver->drv = drv;
> +
> +	mutex_lock(&vfio.bus_drivers_lock);
> +
> +	/* Check for duplicates */
> +	list_for_each_entry(tmp, &vfio.bus_drivers_list, vfio_next) {
> +		if (tmp->drv == drv) {
> +			mutex_unlock(&vfio.bus_drivers_lock);
> +			kfree(driver);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	list_add(&driver->vfio_next, &vfio.bus_drivers_list);
> +
> +	mutex_unlock(&vfio.bus_drivers_lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(vfio_register_bus_driver);
> +
> +void vfio_unregister_bus_driver(struct device_driver *drv)
> +{
> +	struct vfio_bus_driver *driver;
> +
> +	mutex_lock(&vfio.bus_drivers_lock);
> +	list_for_each_entry(driver, &vfio.bus_drivers_list, vfio_next) {
> +		if (driver->drv == drv) {
> +			list_del(&driver->vfio_next);
> +			mutex_unlock(&vfio.bus_drivers_lock);
> +			kfree(driver);
> +			return;
> +		}
> +	}
> +	mutex_unlock(&vfio.bus_drivers_lock);
> +}
> +EXPORT_SYMBOL_GPL(vfio_unregister_bus_driver);
> +
>  /**
>   * Module/class support
>   */
> @@ -2266,8 +2319,10 @@ static int __init vfio_init(void)
>  	idr_init(&vfio.group_idr);
>  	mutex_init(&vfio.group_lock);
>  	mutex_init(&vfio.iommu_drivers_lock);
> +	mutex_init(&vfio.bus_drivers_lock);
>  	INIT_LIST_HEAD(&vfio.group_list);
>  	INIT_LIST_HEAD(&vfio.iommu_drivers_list);
> +	INIT_LIST_HEAD(&vfio.bus_drivers_list);
>  	init_waitqueue_head(&vfio.release_q);
>  
>  	ret = misc_register(&vfio_dev);
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 9b34d0af5d27..dab0f8105e4a 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -92,6 +92,9 @@ struct vfio_iommu_driver_ops {
>  extern void vfio_unregister_iommu_driver(
>  				const struct vfio_iommu_driver_ops *ops);
>  
> +extern int vfio_register_bus_driver(struct device_driver *drv);
> +extern void vfio_unregister_bus_driver(struct device_driver *drv);
> +
>  /*
>   * External user API
>   */
> 

Re: [PATCH 6/7] vfio: Register pci, platform, amba, and mdev bus drivers

[Auger Eric]

Hi Alex,

On 10/06/2017 00:00, Alex Williamson wrote:
> Hook into vfio bus driver register/unregister support.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Cc: Baptiste Reynal <b.reynal@virtualopensystems.com>
> Cc: Kirti Wankhede <kwankhede@nvidia.com>
> Cc: Eric Auger <eric.auger@redhat.com>

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  drivers/vfio/mdev/vfio_mdev.c         |   13 ++++++++++++-
>  drivers/vfio/pci/vfio_pci.c           |    7 +++++++
>  drivers/vfio/platform/vfio_amba.c     |   24 +++++++++++++++++++++++-
>  drivers/vfio/platform/vfio_platform.c |   24 +++++++++++++++++++++++-
>  4 files changed, 65 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
> index fa848a701b8b..b73d6f3e8ad5 100644
> --- a/drivers/vfio/mdev/vfio_mdev.c
> +++ b/drivers/vfio/mdev/vfio_mdev.c
> @@ -131,11 +131,22 @@ struct mdev_driver vfio_mdev_driver = {
>  
>  static int __init vfio_mdev_init(void)
>  {
> -	return mdev_register_driver(&vfio_mdev_driver, THIS_MODULE);
> +	int ret;
> +
> +	ret = mdev_register_driver(&vfio_mdev_driver, THIS_MODULE);
> +	if (ret)
> +		return ret;
> +
> +	ret = vfio_register_bus_driver(&vfio_mdev_driver.driver);
> +	if (ret)
> +		mdev_unregister_driver(&vfio_mdev_driver);
> +
> +	return ret;
>  }
>  
>  static void __exit vfio_mdev_exit(void)
>  {
> +	vfio_unregister_bus_driver(&vfio_mdev_driver.driver);
>  	mdev_unregister_driver(&vfio_mdev_driver);
>  }
>  
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 324c52e3a1a4..15f4b190ad88 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -1397,6 +1397,7 @@ static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev)
>  
>  static void __exit vfio_pci_cleanup(void)
>  {
> +	vfio_unregister_bus_driver(&vfio_pci_driver.driver);
>  	pci_unregister_driver(&vfio_pci_driver);
>  	vfio_pci_uninit_perm_bits();
>  }
> @@ -1456,6 +1457,12 @@ static int __init vfio_pci_init(void)
>  	if (ret)
>  		goto out_driver;
>  
> +	ret = vfio_register_bus_driver(&vfio_pci_driver.driver);
> +	if (ret) {
> +		pci_unregister_driver(&vfio_pci_driver);
> +		goto out_driver;
> +	}
> +
>  	vfio_pci_fill_ids();
>  
>  	return 0;
> diff --git a/drivers/vfio/platform/vfio_amba.c b/drivers/vfio/platform/vfio_amba.c
> index 31372fbf6c5b..7fd9cb4a6756 100644
> --- a/drivers/vfio/platform/vfio_amba.c
> +++ b/drivers/vfio/platform/vfio_amba.c
> @@ -109,7 +109,29 @@ static int vfio_amba_remove(struct amba_device *adev)
>  	},
>  };
>  
> -module_amba_driver(vfio_amba_driver);
> +static void __exit vfio_amba_exit(void)
> +{
> +	vfio_unregister_bus_driver(&vfio_amba_driver.drv);
> +	amba_driver_unregister(&vfio_amba_driver);
> +}
> +
> +static int __init vfio_amba_init(void)
> +{
> +	int ret;
> +
> +	ret = amba_driver_register(&vfio_amba_driver);
> +	if (ret)
> +		return ret;
> +
> +	ret = vfio_register_bus_driver(&vfio_amba_driver.drv);
> +	if (ret)
> +		amba_driver_unregister(&vfio_amba_driver);
> +
> +	return ret;
> +}
> +
> +module_init(vfio_amba_init);
> +module_exit(vfio_amba_exit);
>  
>  MODULE_VERSION(DRIVER_VERSION);
>  MODULE_LICENSE("GPL v2");
> diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
> index 6561751a1063..3974dc65e6dc 100644
> --- a/drivers/vfio/platform/vfio_platform.c
> +++ b/drivers/vfio/platform/vfio_platform.c
> @@ -100,7 +100,29 @@ static int vfio_platform_remove(struct platform_device *pdev)
>  	},
>  };
>  
> -module_platform_driver(vfio_platform_driver);
> +static void __exit vfio_platform_exit(void)
> +{
> +	vfio_unregister_bus_driver(&vfio_platform_driver.driver);
> +	platform_driver_unregister(&vfio_platform_driver);
> +}
> +
> +static int __init vfio_platform_init(void)
> +{
> +	int ret;
> +
> +	ret = platform_driver_register(&vfio_platform_driver);
> +	if (ret)
> +		return ret;
> +
> +	ret = vfio_register_bus_driver(&vfio_platform_driver.driver);
> +	if (ret)
> +		platform_driver_unregister(&vfio_platform_driver);
> +
> +	return ret;
> +}
> +
> +module_init(vfio_platform_init);
> +module_exit(vfio_platform_exit);
>  
>  MODULE_VERSION(DRIVER_VERSION);
>  MODULE_LICENSE("GPL v2");
> 

Re: [PATCH 7/7] vfio: Use driver_override to avert binding to compromising drivers

[Auger Eric]

Hi Alex,

On 10/06/2017 00:00, Alex Williamson wrote:
> If a device is bound to a non-vfio, non-whitelisted driver while a
> group is in use, then the integrity of the group is compromised and
> will result in hitting a BUG_ON.  This code tries to avoid this case
> by mangling driver_override to force a no-match for the driver.  The
> driver-core will either follow-up with a DRIVER_NOT_BOUND (preferred)
> or BOUND_DRIVER, at which point we can remove the driver_override
> mangling.
> 
> A complication here is that even though the window between these
> notifications is expected to be extremely small, the vfio group could
> be removed, which would prevent us from finding the group again to
> remove the driver_override.  We therefore take a group reference when
> adding to driver_override and release it when removed.  A second
> complication is that driver_override can be modified by the system
> admin through sysfs.  To avoid trivial interference, we add a non-
> user-visible UUID to the group and use this as part of the mangle
> string.
> 
> The above blocks binding to a driver that would compromise the host,
> but we'd also like to avoid reaching that step if possible.  For this
> we add a wait_event_timeout() with a short, 1 second timeout, which is
> highly effective in allowing empty groups to finish cleanup.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  drivers/vfio/vfio.c |  151 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 144 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index a25ee4930200..ea786d512faa 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -25,6 +25,7 @@
>  #include <linux/miscdevice.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/platform_device.h>
>  #include <linux/pci.h>
>  #include <linux/rwsem.h>
>  #include <linux/sched.h>
> @@ -32,8 +33,10 @@
>  #include <linux/stat.h>
>  #include <linux/string.h>
>  #include <linux/uaccess.h>
> +#include <linux/uuid.h>
>  #include <linux/vfio.h>
>  #include <linux/wait.h>
> +#include <linux/amba/bus.h>
>  
>  #define DRIVER_VERSION	"0.3"
>  #define DRIVER_AUTHOR	"Alex Williamson <alex.williamson@redhat.com>"
> @@ -95,6 +98,7 @@ struct vfio_group {
>  	bool				noiommu;
>  	struct kvm			*kvm;
>  	struct blocking_notifier_head	notifier;
> +	unsigned char			uuid[16];
>  };
>  
>  struct vfio_device {
> @@ -352,6 +356,8 @@ static struct vfio_group *vfio_create_group(struct iommu_group *iommu_group)
>  
>  	group->nb.notifier_call = vfio_iommu_group_notifier;
>  
> +	generate_random_uuid(group->uuid);
> +
>  	/*
>  	 * blocking notifiers acquire a rwsem around registering and hold
>  	 * it around callback.  Therefore, need to register outside of
> @@ -728,6 +734,116 @@ static int vfio_group_nb_verify(struct vfio_group *group, struct device *dev)
>  	return vfio_dev_viable(dev, group);
>  }
>  
> +#define VFIO_TAG_PREFIX "#vfio_group:"
> +
> +static char **vfio_find_driver_override(struct device *dev)
> +{
> +	if (dev_is_pci(dev)) {
> +		struct pci_dev *pdev = to_pci_dev(dev);
> +		return &pdev->driver_override;
> +	} else if (dev->bus == &platform_bus_type) {
> +		struct platform_device *pdev = to_platform_device(dev);
> +		return &pdev->driver_override;
> +#ifdef CONFIG_ARM_AMBA
> +	} else if (dev->bus == &amba_bustype) {

EXPORT_SYMBOL_GPL(amba_bustype);
needs to be added in drivers/amba/bus.c otherwise this causes a link error
ERROR: "amba_bustype" [drivers/vfio/vfio.ko] undefined!

Otherwise it looks good to me and I 've tested this with dual port igb
on ARM.

Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric


> +		struct amba_device *adev = to_amba_device(dev);
> +		return &adev->driver_override;
> +#endif
> +	}
> +
> +	return NULL;
> +}
> +
> +/*
> + * If we're about to bind to something other than a known whitelisted driver
> + * or known vfio bus driver, try to avert it with driver_override.
> + */
> +static void vfio_group_nb_pre_bind(struct vfio_group *group, struct device *dev)
> +{
> +	struct vfio_bus_driver *driver;
> +	struct device_driver *drv = ACCESS_ONCE(dev->driver);
> +	char **driver_override;
> +
> +	if (vfio_dev_whitelisted(dev, drv))
> +		return; /* Binding to known "innocuous" device/driver */
> +
> +	mutex_lock(&vfio.bus_drivers_lock);
> +	list_for_each_entry(driver, &vfio.bus_drivers_list, vfio_next) {
> +		if (driver->drv == drv) {
> +			mutex_unlock(&vfio.bus_drivers_lock);
> +			return; /* Binding to known vfio bus driver, ok */
> +		}
> +	}
> +	mutex_unlock(&vfio.bus_drivers_lock);
> +
> +	/* Can we stall slightly to let users fall off? */
> +	if (list_empty(&group->device_list)) {
> +		if (wait_event_timeout(vfio.release_q,
> +				!atomic_read(&group->container_users), HZ))
> +			return;
> +	}
> +
> +	driver_override = vfio_find_driver_override(dev);
> +	if (driver_override) {
> +		char tag[50], *new = NULL, *old = *driver_override;
> +
> +		snprintf(tag, sizeof(tag), "%s%pU",
> +			 VFIO_TAG_PREFIX, group->uuid);
> +
> +		if (old && strstr(old, tag))
> +			return; /* block already in place */
> +
> +		new = kasprintf(GFP_KERNEL, "%s%s", old ? old : "", tag);
> +		if (new) {
> +			*driver_override = new;
> +			kfree(old);
> +			vfio_group_get(group);
> +			dev_warn(dev, "vfio: Blocking unsafe driver bind\n");
> +			return;
> +		}
> +	}
> +
> +	dev_warn(dev, "vfio: Unsafe driver binding to in-use group!\n");
> +}
> +
> +/* If we've mangled driver_override, remove it */
> +static void vfio_group_nb_post_bind(struct vfio_group *group,
> +				    struct device *dev)
> +{
> +	char **driver_override = vfio_find_driver_override(dev);
> +
> +	if (driver_override && *driver_override) {
> +		char tag[50], *new, *start, *end, *old = *driver_override;
> +
> +		snprintf(tag, sizeof(tag), "%s%pU",
> +			 VFIO_TAG_PREFIX, group->uuid);
> +
> +		start = strstr(old, tag);
> +		if (start) {
> +			end = start + strlen(tag);
> +
> +			if (old + strlen(old) > end)
> +				memmove(start, end,
> +					strlen(old) - (end - old) + 1);
> +			else
> +				*start = 0;
> +
> +			if (strlen(old)) {
> +				new = kasprintf(GFP_KERNEL, "%s", old);
> +				if (new) {
> +					*driver_override = new;
> +					kfree(old);
> +				} /* else, in-place terminated, ok */
> +			} else {
> +				*driver_override = NULL;
> +				kfree(old);
> +			}
> +
> +			vfio_group_put(group);
> +		}
> +	}
> +}
> +
>  static int vfio_iommu_group_notifier(struct notifier_block *nb,
>  				     unsigned long action, void *data)
>  {
> @@ -757,14 +873,23 @@ static int vfio_iommu_group_notifier(struct notifier_block *nb,
>  		 */
>  		break;
>  	case IOMMU_GROUP_NOTIFY_BIND_DRIVER:
> -		pr_debug("%s: Device %s, group %d binding to driver\n",
> +		pr_debug("%s: Device %s, group %d binding to driver %s\n",
>  			 __func__, dev_name(dev),
> -			 iommu_group_id(group->iommu_group));
> +			 iommu_group_id(group->iommu_group), dev->driver->name);
> +		if (vfio_group_nb_verify(group, dev))
> +			vfio_group_nb_pre_bind(group, dev);
> +		break;
> +	case IOMMU_GROUP_NOTIFY_DRIVER_NOT_BOUND:
> +		pr_debug("%s: Device %s, group %d binding fail to driver %s\n",
> +			 __func__, dev_name(dev),
> +			 iommu_group_id(group->iommu_group), dev->driver->name);
> +		vfio_group_nb_post_bind(group, dev);
>  		break;
>  	case IOMMU_GROUP_NOTIFY_BOUND_DRIVER:
>  		pr_debug("%s: Device %s, group %d bound to driver %s\n",
>  			 __func__, dev_name(dev),
>  			 iommu_group_id(group->iommu_group), dev->driver->name);
> +		vfio_group_nb_post_bind(group, dev);
>  		BUG_ON(vfio_group_nb_verify(group, dev));
>  		break;
>  	case IOMMU_GROUP_NOTIFY_UNBIND_DRIVER:
> @@ -1351,6 +1476,7 @@ static int vfio_group_unset_container(struct vfio_group *group)
>  	if (users != 1)
>  		return -EBUSY;
>  
> +	wake_up(&vfio.release_q);
>  	__vfio_group_unset_container(group);
>  
>  	return 0;
> @@ -1364,7 +1490,11 @@ static int vfio_group_unset_container(struct vfio_group *group)
>   */
>  static void vfio_group_try_dissolve_container(struct vfio_group *group)
>  {
> -	if (0 == atomic_dec_if_positive(&group->container_users))
> +	int users = atomic_dec_if_positive(&group->container_users);
> +
> +	wake_up(&vfio.release_q);
> +
> +	if (!users)
>  		__vfio_group_unset_container(group);
>  }
>  
> @@ -1433,19 +1563,26 @@ static bool vfio_group_viable(struct vfio_group *group)
>  
>  static int vfio_group_add_container_user(struct vfio_group *group)
>  {
> +	int ret;
> +
>  	if (!atomic_inc_not_zero(&group->container_users))
>  		return -EINVAL;
>  
>  	if (group->noiommu) {
> -		atomic_dec(&group->container_users);
> -		return -EPERM;
> +		ret = -EPERM;
> +		goto out;
>  	}
>  	if (!group->container->iommu_driver || !vfio_group_viable(group)) {
> -		atomic_dec(&group->container_users);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out;
>  	}
>  
>  	return 0;
> +
> +out:
> +	atomic_dec(&group->container_users);
> +	wake_up(&vfio.release_q);
> +	return ret;
>  }
>  
>  static const struct file_operations vfio_device_fops;
>