From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vlad Tsyrklevich Subject: [PATCH v2] vfio/pci: Fix integer overflows, bitmask check Date: Wed, 12 Oct 2016 09:53:49 +0200 Message-ID: <1476258829-44541-1-git-send-email-vlad@tsyrklevich.net> References: <20161011163957.2cadbc69@t450s.home> Cc: alex.williamson@redhat.com, Vlad Tsyrklevich To: kvm@vger.kernel.org Return-path: Received: from mail-wm0-f65.google.com ([74.125.82.65]:36607 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754073AbcJLH6l (ORCPT ); Wed, 12 Oct 2016 03:58:41 -0400 Received: by mail-wm0-f65.google.com with SMTP id b80so1115617wme.3 for ; Wed, 12 Oct 2016 00:58:08 -0700 (PDT) In-Reply-To: <20161011163957.2cadbc69@t450s.home> Sender: kvm-owner@vger.kernel.org List-ID: The VFIO_DEVICE_SET_IRQS ioctl did not sufficiently sanitize user-supplied integers, potentially allowing arbitrary memory writes. This patch adds appropriate integer checks, and also verifies that only a single element in the VFIO_IRQ_SET_DATA_TYPE_MASK bitmask is set. VFIO_IRQ_SET_ACTION_TYPE_MASK is already correctly checked later in vfio_pci_set_irqs_ioctl(). Signed-off-by: Vlad Tsyrklevich --- drivers/vfio/pci/vfio_pci.c | 26 +++++++++++++++++--------- drivers/vfio/pci/vfio_pci_intrs.c | 2 +- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index d624a52..44de13c 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -829,6 +829,7 @@ static long vfio_pci_ioctl(void *device_data, } else if (cmd == VFIO_DEVICE_SET_IRQS) { struct vfio_irq_set hdr; + size_t size; u8 *data = NULL; int ret = 0; @@ -838,20 +839,27 @@ static long vfio_pci_ioctl(void *device_data, return -EFAULT; if (hdr.argsz < minsz || hdr.index >= VFIO_PCI_NUM_IRQS || + hdr.count >= (U32_MAX - hdr.start) || hdr.flags & ~(VFIO_IRQ_SET_DATA_TYPE_MASK | VFIO_IRQ_SET_ACTION_TYPE_MASK)) return -EINVAL; - if (!(hdr.flags & VFIO_IRQ_SET_DATA_NONE)) { - size_t size; - int max = vfio_pci_get_irq_count(vdev, hdr.index); + switch (hdr.flags & VFIO_IRQ_SET_DATA_TYPE_MASK) { + case VFIO_IRQ_SET_DATA_NONE: + size = 0; + break; + case VFIO_IRQ_SET_DATA_BOOL: + size = sizeof(uint8_t); + break; + case VFIO_IRQ_SET_DATA_EVENTFD: + size = sizeof(int32_t); + break; + default: + return -EINVAL; + } - if (hdr.flags & VFIO_IRQ_SET_DATA_BOOL) - size = sizeof(uint8_t); - else if (hdr.flags & VFIO_IRQ_SET_DATA_EVENTFD) - size = sizeof(int32_t); - else - return -EINVAL; + if (size) { + int max = vfio_pci_get_irq_count(vdev, hdr.index); if (hdr.argsz - minsz < hdr.count * size || hdr.start >= max || hdr.start + hdr.count > max) diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c index c2e6089..1c46045 100644 --- a/drivers/vfio/pci/vfio_pci_intrs.c +++ b/drivers/vfio/pci/vfio_pci_intrs.c @@ -256,7 +256,7 @@ static int vfio_msi_enable(struct vfio_pci_device *vdev, int nvec, bool msix) if (!is_irq_none(vdev)) return -EINVAL; - vdev->ctx = kzalloc(nvec * sizeof(struct vfio_pci_irq_ctx), GFP_KERNEL); + vdev->ctx = kcalloc(nvec, sizeof(struct vfio_pci_irq_ctx), GFP_KERNEL); if (!vdev->ctx) return -ENOMEM; -- 2.7.0