vfio-pci: Don't use device_lock around AER interrupt setup

device_lock is much too prone to lockups.  For instance if we have a
pending .remove then device_lock is already held.  If userspace
attempts to modify AER signaling after that point, a deadlock occurs.
eventfd setup/teardown is already protected in vfio with the igate
mutex.  AER is not a high performance interrupt, so we can also use
the same mutex to protect signaling versus setup races.

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 6ab71b9fcf8d692dcb9891d05ab02c2f153d72ef..3ffd27f424182a7f6be3245b5fbb932af25b2dd7 100644 (file)
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -883,9 +883,13 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev,
                return PCI_ERS_RESULT_DISCONNECT;
        }
 
+       mutex_lock(&vdev->igate);
+
        if (vdev->err_trigger)
                eventfd_signal(vdev->err_trigger, 1);
 
+       mutex_unlock(&vdev->igate);
+
        vfio_device_put(device);
 
        return PCI_ERS_RESULT_CAN_RECOVER;

diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
index 641bc87bdb96aa23500c48483c81defbf0088026..210357691dc0a06e65b522e5847321e62c43541c 100644 (file)
--- a/drivers/vfio/pci/vfio_pci_intrs.c
+++ b/drivers/vfio/pci/vfio_pci_intrs.c
@@ -749,54 +749,37 @@ static int vfio_pci_set_err_trigger(struct vfio_pci_device *vdev,
                                    unsigned count, uint32_t flags, void *data)
 {
        int32_t fd = *(int32_t *)data;
-       struct pci_dev *pdev = vdev->pdev;
 
        if ((index != VFIO_PCI_ERR_IRQ_INDEX) ||
            !(flags & VFIO_IRQ_SET_DATA_TYPE_MASK))
                return -EINVAL;
 
-       /*
-        * device_lock synchronizes setting and checking of
-        * err_trigger. The vfio_pci_aer_err_detected() is also
-        * called with device_lock held.
-        */
-
        /* DATA_NONE/DATA_BOOL enables loopback testing */
-
        if (flags & VFIO_IRQ_SET_DATA_NONE) {
-               device_lock(&pdev->dev);
                if (vdev->err_trigger)
                        eventfd_signal(vdev->err_trigger, 1);
-               device_unlock(&pdev->dev);
                return 0;
        } else if (flags & VFIO_IRQ_SET_DATA_BOOL) {
                uint8_t trigger = *(uint8_t *)data;
-               device_lock(&pdev->dev);
                if (trigger && vdev->err_trigger)
                        eventfd_signal(vdev->err_trigger, 1);
-               device_unlock(&pdev->dev);
                return 0;
        }
 
        /* Handle SET_DATA_EVENTFD */
-
        if (fd == -1) {
-               device_lock(&pdev->dev);
                if (vdev->err_trigger)
                        eventfd_ctx_put(vdev->err_trigger);
                vdev->err_trigger = NULL;
-               device_unlock(&pdev->dev);
                return 0;
        } else if (fd >= 0) {
                struct eventfd_ctx *efdctx;
                efdctx = eventfd_ctx_fdget(fd);
                if (IS_ERR(efdctx))
                        return PTR_ERR(efdctx);
-               device_lock(&pdev->dev);
                if (vdev->err_trigger)
                        eventfd_ctx_put(vdev->err_trigger);
                vdev->err_trigger = efdctx;
-               device_unlock(&pdev->dev);
                return 0;
        } else
                return -EINVAL;