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@xxxxxxxxxx>
---
drivers/vfio/pci/vfio_pci.c | 4 ++++
drivers/vfio/pci/vfio_pci_intrs.c | 17 -----------------
2 files changed, 4 insertions(+), 17 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 576e34e..468a9fb 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -872,9 +872,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 641bc87..2103576 100644
--- 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;
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
