From b646559cc90f1a1f475fc47048bbbd955e20f4a2 Mon Sep 17 00:00:00 2001 From: John Levon Date: Thu, 21 Mar 2024 16:21:39 +0000 Subject: correct IRQ range check (#791) Our previous fuzzing attempts missed this incorrect range check, but SPDK's fuzzing did catch it. Make the check using a saturating add so that we account for overflow. Fixes issue #790. Reported-by: Sebastian Brzezinka Signed-off-by: John Levon --- lib/common.h | 8 ++++++++ lib/irq.c | 14 +++++++++++--- test/py/test_device_set_irqs.py | 9 +++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/common.h b/lib/common.h index 40b9b27..c4de49a 100644 --- a/lib/common.h +++ b/lib/common.h @@ -77,6 +77,14 @@ ERROR_PTR(int err) return NULL; } +/* Saturating uint32_t addition. */ +static inline uint32_t +satadd_u32(uint32_t a, uint32_t b) +{ + uint64_t res = a + b; + return (res < a) ? UINT32_MAX : res; +} + /* Saturating uint64_t addition. */ static inline uint64_t satadd_u64(uint64_t a, uint64_t b) diff --git a/lib/irq.c b/lib/irq.c index 183d071..a6fd575 100644 --- a/lib/irq.c +++ b/lib/irq.c @@ -312,12 +312,20 @@ device_set_irqs_validate(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg) line = __LINE__; goto invalid; } - // Ensure irq_set's start and count are within bounds. - if ((irq_set->start >= vfu_ctx->irq_count[irq_set->index]) || - (irq_set->start + irq_set->count > vfu_ctx->irq_count[irq_set->index])) { + + // Ensure irq_set's start is within bounds. + if (irq_set->start >= vfu_ctx->irq_count[irq_set->index]) { + line = __LINE__; + goto invalid; + } + + // Ensure irq_set's start+count is within bounds. + if (satadd_u32(irq_set->start, irq_set->count) > + vfu_ctx->irq_count[irq_set->index]) { line = __LINE__; goto invalid; } + // Only TRIGGER is valid for ERR/REQ. if (((irq_set->index == VFIO_PCI_ERR_IRQ_INDEX) || (irq_set->index == VFIO_PCI_REQ_IRQ_INDEX)) && diff --git a/test/py/test_device_set_irqs.py b/test/py/test_device_set_irqs.py index a2a2701..a8a1bd0 100644 --- a/test/py/test_device_set_irqs.py +++ b/test/py/test_device_set_irqs.py @@ -133,6 +133,15 @@ def test_device_set_irqs_bad_start_count_range2(): expect=errno.EINVAL) +def test_device_set_irqs_bad_start_count_range3(): + payload = vfio_irq_set(argsz=argsz, flags=VFIO_IRQ_SET_ACTION_TRIGGER | + VFIO_IRQ_SET_DATA_EVENTFD, index=VFU_DEV_MSIX_IRQ, + start=284, count=0xffffffff) + + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, + expect=errno.EINVAL) + + def test_device_set_irqs_bad_action_for_err_irq(): payload = vfio_irq_set(argsz=argsz, flags=VFIO_IRQ_SET_ACTION_MASK | VFIO_IRQ_SET_DATA_NONE, index=VFU_DEV_ERR_IRQ, -- cgit v1.1