Commit 3be3a074 authored by Alex Williamson's avatar Alex Williamson

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: default avatarAlex Williamson <alex.williamson@redhat.com>
parent d1099901
...@@ -883,9 +883,13 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev, ...@@ -883,9 +883,13 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev,
return PCI_ERS_RESULT_DISCONNECT; return PCI_ERS_RESULT_DISCONNECT;
} }
mutex_lock(&vdev->igate);
if (vdev->err_trigger) if (vdev->err_trigger)
eventfd_signal(vdev->err_trigger, 1); eventfd_signal(vdev->err_trigger, 1);
mutex_unlock(&vdev->igate);
vfio_device_put(device); vfio_device_put(device);
return PCI_ERS_RESULT_CAN_RECOVER; return PCI_ERS_RESULT_CAN_RECOVER;
......
...@@ -749,54 +749,37 @@ static int vfio_pci_set_err_trigger(struct vfio_pci_device *vdev, ...@@ -749,54 +749,37 @@ static int vfio_pci_set_err_trigger(struct vfio_pci_device *vdev,
unsigned count, uint32_t flags, void *data) unsigned count, uint32_t flags, void *data)
{ {
int32_t fd = *(int32_t *)data; int32_t fd = *(int32_t *)data;
struct pci_dev *pdev = vdev->pdev;
if ((index != VFIO_PCI_ERR_IRQ_INDEX) || if ((index != VFIO_PCI_ERR_IRQ_INDEX) ||
!(flags & VFIO_IRQ_SET_DATA_TYPE_MASK)) !(flags & VFIO_IRQ_SET_DATA_TYPE_MASK))
return -EINVAL; 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 */ /* DATA_NONE/DATA_BOOL enables loopback testing */
if (flags & VFIO_IRQ_SET_DATA_NONE) { if (flags & VFIO_IRQ_SET_DATA_NONE) {
device_lock(&pdev->dev);
if (vdev->err_trigger) if (vdev->err_trigger)
eventfd_signal(vdev->err_trigger, 1); eventfd_signal(vdev->err_trigger, 1);
device_unlock(&pdev->dev);
return 0; return 0;
} else if (flags & VFIO_IRQ_SET_DATA_BOOL) { } else if (flags & VFIO_IRQ_SET_DATA_BOOL) {
uint8_t trigger = *(uint8_t *)data; uint8_t trigger = *(uint8_t *)data;
device_lock(&pdev->dev);
if (trigger && vdev->err_trigger) if (trigger && vdev->err_trigger)
eventfd_signal(vdev->err_trigger, 1); eventfd_signal(vdev->err_trigger, 1);
device_unlock(&pdev->dev);
return 0; return 0;
} }
/* Handle SET_DATA_EVENTFD */ /* Handle SET_DATA_EVENTFD */
if (fd == -1) { if (fd == -1) {
device_lock(&pdev->dev);
if (vdev->err_trigger) if (vdev->err_trigger)
eventfd_ctx_put(vdev->err_trigger); eventfd_ctx_put(vdev->err_trigger);
vdev->err_trigger = NULL; vdev->err_trigger = NULL;
device_unlock(&pdev->dev);
return 0; return 0;
} else if (fd >= 0) { } else if (fd >= 0) {
struct eventfd_ctx *efdctx; struct eventfd_ctx *efdctx;
efdctx = eventfd_ctx_fdget(fd); efdctx = eventfd_ctx_fdget(fd);
if (IS_ERR(efdctx)) if (IS_ERR(efdctx))
return PTR_ERR(efdctx); return PTR_ERR(efdctx);
device_lock(&pdev->dev);
if (vdev->err_trigger) if (vdev->err_trigger)
eventfd_ctx_put(vdev->err_trigger); eventfd_ctx_put(vdev->err_trigger);
vdev->err_trigger = efdctx; vdev->err_trigger = efdctx;
device_unlock(&pdev->dev);
return 0; return 0;
} else } else
return -EINVAL; return -EINVAL;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment