vfio/pci: Fix racy vfio_device_get_from_dev() call

Testing the driver for a PCI device is racy, it can be all but
complete in the release path and still report the driver as ours.
Therefore we can't trust drvdata to be valid.  This race can sometimes
be seen when one port of a multifunction device is being unbound from
the vfio-pci driver while another function is being released by the
user and attempting a bus reset.  The device in the remove path is
found as a dependent device for the bus reset of the release path
device, the driver is still set to vfio-pci, but the drvdata has
already been cleared, resulting in a null pointer dereference.

To resolve this, fix vfio_device_get_from_dev() to not take the
dev_get_drvdata() shortcut and instead traverse through the
iommu_group, vfio_group, vfio_device path to get a reference we
can trust.  Once we have that reference, we know the device isn't
in transition and we can test to make sure the driver is still what
we expect, so that we don't interfere with devices we don't own.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index e9851add6f4ef251defba65a3bab9af359b60b3a..964ad572aaeede1fc7d2522a9fdb69b7d66f3faa 100644 (file)
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -1056,19 +1056,21 @@ struct vfio_devices {
 static int vfio_pci_get_devs(struct pci_dev *pdev, void *data)
 {
        struct vfio_devices *devs = data;
-       struct pci_driver *pci_drv = ACCESS_ONCE(pdev->driver);
-
-       if (pci_drv != &vfio_pci_driver)
-               return -EBUSY;
+       struct vfio_device *device;
 
        if (devs->cur_index == devs->max_index)
                return -ENOSPC;
 
-       devs->devices[devs->cur_index] = vfio_device_get_from_dev(&pdev->dev);
-       if (!devs->devices[devs->cur_index])
+       device = vfio_device_get_from_dev(&pdev->dev);
+       if (!device)
                return -EINVAL;
 
-       devs->cur_index++;
+       if (pci_dev_driver(pdev) != &vfio_pci_driver) {
+               vfio_device_put(device);
+               return -EBUSY;
+       }
+
+       devs->devices[devs->cur_index++] = device;
        return 0;
 }
 

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index e1278fe04b1e7ba16eddbece247c21620b6e8fb7..2fb29dfeffbd7fa70a8a881f025d73ffd50b0b25 100644 (file)
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -661,18 +661,29 @@ int vfio_add_group_dev(struct device *dev,
 EXPORT_SYMBOL_GPL(vfio_add_group_dev);
 
 /**
- * Get a reference to the vfio_device for a device that is known to
- * be bound to a vfio driver.  The driver implicitly holds a
- * vfio_device reference between vfio_add_group_dev and
- * vfio_del_group_dev.  We can therefore use drvdata to increment
- * that reference from the struct device.  This additional
- * reference must be released by calling vfio_device_put.
+ * Get a reference to the vfio_device for a device.  Even if the
+ * caller thinks they own the device, they could be racing with a
+ * release call path, so we can't trust drvdata for the shortcut.
+ * Go the long way around, from the iommu_group to the vfio_group
+ * to the vfio_device.
  */
 struct vfio_device *vfio_device_get_from_dev(struct device *dev)
 {
-       struct vfio_device *device = dev_get_drvdata(dev);
+       struct iommu_group *iommu_group;
+       struct vfio_group *group;
+       struct vfio_device *device;
+
+       iommu_group = iommu_group_get(dev);
+       if (!iommu_group)
+               return NULL;
 
-       vfio_device_get(device);
+       group = vfio_group_get_from_iommu(iommu_group);
+       iommu_group_put(iommu_group);
+       if (!group)
+               return NULL;
+
+       device = vfio_group_get_device(group, dev);
+       vfio_group_put(group);
 
        return device;
 }