| From c8952a707556e04374d7b2fdb3a079d63ddf6f2f Mon Sep 17 00:00:00 2001 |
| From: Alex Williamson <alex.williamson@redhat.com> |
| Date: Mon, 8 Aug 2016 16:16:23 -0600 |
| Subject: vfio/pci: Fix NULL pointer oops in error interrupt setup handling |
| |
| From: Alex Williamson <alex.williamson@redhat.com> |
| |
| commit c8952a707556e04374d7b2fdb3a079d63ddf6f2f 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: Alex Williamson <alex.williamson@redhat.com> |
| Reported-by: Chris Thompson <the_cartographer@hotmail.com> |
| Reviewed-by: Eric Auger <eric.auger@redhat.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/vfio/pci/vfio_pci_intrs.c | 85 +++++++++++++++++++++----------------- |
| 1 file changed, 49 insertions(+), 36 deletions(-) |
| |
| --- a/drivers/vfio/pci/vfio_pci_intrs.c |
| +++ b/drivers/vfio/pci/vfio_pci_intrs.c |
| @@ -564,67 +564,80 @@ static int vfio_pci_set_msi_trigger(stru |
| } |
| |
| static int vfio_pci_set_ctx_trigger_single(struct eventfd_ctx **ctx, |
| - uint32_t flags, void *data) |
| + unsigned int count, uint32_t flags, |
| + void *data) |
| { |
| - int32_t fd = *(int32_t *)data; |
| - |
| - if (!(flags & VFIO_IRQ_SET_DATA_TYPE_MASK)) |
| - return -EINVAL; |
| - |
| /* DATA_NONE/DATA_BOOL enables loopback testing */ |
| if (flags & VFIO_IRQ_SET_DATA_NONE) { |
| - if (*ctx) |
| - eventfd_signal(*ctx, 1); |
| - return 0; |
| + if (*ctx) { |
| + if (count) { |
| + eventfd_signal(*ctx, 1); |
| + } else { |
| + eventfd_ctx_put(*ctx); |
| + *ctx = NULL; |
| + } |
| + return 0; |
| + } |
| } 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 && *ctx) |
| eventfd_signal(*ctx, 1); |
| - return 0; |
| - } |
| |
| - /* Handle SET_DATA_EVENTFD */ |
| - if (fd == -1) { |
| - if (*ctx) |
| - eventfd_ctx_put(*ctx); |
| - *ctx = NULL; |
| return 0; |
| - } else if (fd >= 0) { |
| - struct eventfd_ctx *efdctx; |
| - efdctx = eventfd_ctx_fdget(fd); |
| - if (IS_ERR(efdctx)) |
| - return PTR_ERR(efdctx); |
| - if (*ctx) |
| - eventfd_ctx_put(*ctx); |
| - *ctx = efdctx; |
| + } else if (flags & VFIO_IRQ_SET_DATA_EVENTFD) { |
| + int32_t fd; |
| + |
| + if (!count) |
| + return -EINVAL; |
| + |
| + fd = *(int32_t *)data; |
| + if (fd == -1) { |
| + if (*ctx) |
| + eventfd_ctx_put(*ctx); |
| + *ctx = NULL; |
| + } else if (fd >= 0) { |
| + struct eventfd_ctx *efdctx; |
| + |
| + efdctx = eventfd_ctx_fdget(fd); |
| + if (IS_ERR(efdctx)) |
| + return PTR_ERR(efdctx); |
| + |
| + if (*ctx) |
| + eventfd_ctx_put(*ctx); |
| + |
| + *ctx = efdctx; |
| + } |
| return 0; |
| - } else |
| - return -EINVAL; |
| + } |
| + |
| + return -EINVAL; |
| } |
| |
| static int vfio_pci_set_err_trigger(struct vfio_pci_device *vdev, |
| unsigned index, unsigned start, |
| unsigned count, uint32_t flags, void *data) |
| { |
| - if (index != VFIO_PCI_ERR_IRQ_INDEX) |
| + if (index != VFIO_PCI_ERR_IRQ_INDEX || start != 0 || count > 1) |
| return -EINVAL; |
| |
| - /* |
| - * We should sanitize start & count, but that wasn't caught |
| - * originally, so this IRQ index must forever ignore them :-( |
| - */ |
| - |
| - return vfio_pci_set_ctx_trigger_single(&vdev->err_trigger, flags, data); |
| + return vfio_pci_set_ctx_trigger_single(&vdev->err_trigger, |
| + count, flags, data); |
| } |
| |
| static int vfio_pci_set_req_trigger(struct vfio_pci_device *vdev, |
| unsigned index, unsigned start, |
| unsigned count, uint32_t flags, void *data) |
| { |
| - if (index != VFIO_PCI_REQ_IRQ_INDEX || start != 0 || count != 1) |
| + if (index != VFIO_PCI_REQ_IRQ_INDEX || start != 0 || count > 1) |
| return -EINVAL; |
| |
| - return vfio_pci_set_ctx_trigger_single(&vdev->req_trigger, flags, data); |
| + return vfio_pci_set_ctx_trigger_single(&vdev->req_trigger, |
| + count, flags, data); |
| } |
| |
| int vfio_pci_set_irqs_ioctl(struct vfio_pci_device *vdev, uint32_t flags, |