Skip to content

Commit d23ef85

Browse files
vlad902gregkh
authored andcommitted
vfio/pci: Fix integer overflows, bitmask check
commit 05692d7005a364add85c6e25a6c4447ce08f913a upstream. The VFIO_DEVICE_SET_IRQS ioctl did not sufficiently sanitize user-supplied integers, potentially allowing memory corruption. This patch adds appropriate integer overflow checks, checks the range bounds for VFIO_IRQ_SET_DATA_NONE, and also verifies that only 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(). Furthermore, a kzalloc is changed to a kcalloc because the use of a kzalloc with an integer multiplication allowed an integer overflow condition to be reached without this patch. kcalloc checks for overflow and should prevent a similar occurrence. Signed-off-by: Vlad Tsyrklevich <vlad@tsyrklevich.net> Signed-off-by: Alex Williamson <alex.williamson@redhat.com> Cc: Ben Hutchings <ben.hutchings@codethink.co.uk> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 65d30f7 commit d23ef85

2 files changed

Lines changed: 22 additions & 13 deletions

File tree

drivers/vfio/pci/vfio_pci.c

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -562,32 +562,41 @@ static long vfio_pci_ioctl(void *device_data,
562562

563563
} else if (cmd == VFIO_DEVICE_SET_IRQS) {
564564
struct vfio_irq_set hdr;
565+
size_t size;
565566
u8 *data = NULL;
566-
int ret = 0;
567+
int max, ret = 0;
567568

568569
minsz = offsetofend(struct vfio_irq_set, count);
569570

570571
if (copy_from_user(&hdr, (void __user *)arg, minsz))
571572
return -EFAULT;
572573

573574
if (hdr.argsz < minsz || hdr.index >= VFIO_PCI_NUM_IRQS ||
575+
hdr.count >= (U32_MAX - hdr.start) ||
574576
hdr.flags & ~(VFIO_IRQ_SET_DATA_TYPE_MASK |
575577
VFIO_IRQ_SET_ACTION_TYPE_MASK))
576578
return -EINVAL;
577579

578-
if (!(hdr.flags & VFIO_IRQ_SET_DATA_NONE)) {
579-
size_t size;
580-
int max = vfio_pci_get_irq_count(vdev, hdr.index);
580+
max = vfio_pci_get_irq_count(vdev, hdr.index);
581+
if (hdr.start >= max || hdr.start + hdr.count > max)
582+
return -EINVAL;
581583

582-
if (hdr.flags & VFIO_IRQ_SET_DATA_BOOL)
583-
size = sizeof(uint8_t);
584-
else if (hdr.flags & VFIO_IRQ_SET_DATA_EVENTFD)
585-
size = sizeof(int32_t);
586-
else
587-
return -EINVAL;
584+
switch (hdr.flags & VFIO_IRQ_SET_DATA_TYPE_MASK) {
585+
case VFIO_IRQ_SET_DATA_NONE:
586+
size = 0;
587+
break;
588+
case VFIO_IRQ_SET_DATA_BOOL:
589+
size = sizeof(uint8_t);
590+
break;
591+
case VFIO_IRQ_SET_DATA_EVENTFD:
592+
size = sizeof(int32_t);
593+
break;
594+
default:
595+
return -EINVAL;
596+
}
588597

589-
if (hdr.argsz - minsz < hdr.count * size ||
590-
hdr.start >= max || hdr.start + hdr.count > max)
598+
if (size) {
599+
if (hdr.argsz - minsz < hdr.count * size)
591600
return -EINVAL;
592601

593602
data = memdup_user((void __user *)(arg + minsz),

drivers/vfio/pci/vfio_pci_intrs.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ static int vfio_msi_enable(struct vfio_pci_device *vdev, int nvec, bool msix)
255255
if (!is_irq_none(vdev))
256256
return -EINVAL;
257257

258-
vdev->ctx = kzalloc(nvec * sizeof(struct vfio_pci_irq_ctx), GFP_KERNEL);
258+
vdev->ctx = kcalloc(nvec, sizeof(struct vfio_pci_irq_ctx), GFP_KERNEL);
259259
if (!vdev->ctx)
260260
return -ENOMEM;
261261

0 commit comments

Comments
 (0)