Commit 8b7d7828 authored by Alex Williamson's avatar Alex Williamson Committed by Ben Hutchings

vfio/pci: Fix NULL pointer oops in error interrupt setup handling

commit c8952a70 upstream.

There are multiple cases in vfio_pci_set_ctx_trigger_single() where
we assume we can safely read from our data pointer without actually
checking whether the user has passed any data via the count field.
VFIO_IRQ_SET_DATA_NONE in particular is entirely broken since we
attempt to pull an int32_t file descriptor out before even checking
the data type.  The other data types assume the data pointer contains
one element of their type as well.

In part this is good news because we were previously restricted from
doing much sanitization of parameters because it was missed in the
past and we didn't want to break existing users.  Clearly DATA_NONE
is completely broken, so it must not have any users and we can fix
it up completely.  For DATA_BOOL and DATA_EVENTFD, we'll just
protect ourselves, returning error when count is zero since we
previously would have oopsed.
Signed-off-by: default avatarAlex Williamson <alex.williamson@redhat.com>
Reported-by: default avatarChris Thompson <the_cartographer@hotmail.com>
Reviewed-by: default avatarEric Auger <eric.auger@redhat.com>
[bwh: Backported to 3.16:
 - Drop changes to vfio_pci_set_req_trigger()
 - Apply remaining changes in vfio_pci_set_err_trigger()]
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
parent 9269db2d
...@@ -752,40 +752,57 @@ static int vfio_pci_set_err_trigger(struct vfio_pci_device *vdev, ...@@ -752,40 +752,57 @@ static int vfio_pci_set_err_trigger(struct vfio_pci_device *vdev,
unsigned index, unsigned start, unsigned index, unsigned start,
unsigned count, uint32_t flags, void *data) unsigned count, uint32_t flags, void *data)
{ {
int32_t fd = *(int32_t *)data; if (index != VFIO_PCI_ERR_IRQ_INDEX || start != 0 || count > 1)
if ((index != VFIO_PCI_ERR_IRQ_INDEX) ||
!(flags & VFIO_IRQ_SET_DATA_TYPE_MASK))
return -EINVAL; return -EINVAL;
/* 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) {
if (vdev->err_trigger) if (vdev->err_trigger) {
if (count) {
eventfd_signal(vdev->err_trigger, 1); eventfd_signal(vdev->err_trigger, 1);
} else {
eventfd_ctx_put(vdev->err_trigger);
vdev->err_trigger = NULL;
}
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;
if (!count)
return -EINVAL;
trigger = *(uint8_t *)data;
if (trigger && vdev->err_trigger) if (trigger && vdev->err_trigger)
eventfd_signal(vdev->err_trigger, 1); eventfd_signal(vdev->err_trigger, 1);
return 0; return 0;
} } else if (flags & VFIO_IRQ_SET_DATA_EVENTFD) {
int32_t fd;
if (!count)
return -EINVAL;
/* Handle SET_DATA_EVENTFD */ fd = *(int32_t *)data;
if (fd == -1) { if (fd == -1) {
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;
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);
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;
}
return 0; return 0;
} else }
return -EINVAL; return -EINVAL;
} }
int vfio_pci_set_irqs_ioctl(struct vfio_pci_device *vdev, uint32_t flags, int vfio_pci_set_irqs_ioctl(struct vfio_pci_device *vdev, uint32_t flags,
......
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