From 3bf7dcd47a3da0e86a9347ce5b2b5d5a1dcb5857 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 19 Nov 2024 13:02:05 +0000 Subject: [PATCH] hw/intc/openpic: Avoid taking address of out-of-bounds array index The clang sanitizer complains about the code in the EOI handling of openpic_cpu_write_internal(): UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1 ./build/clang/qemu-system-ppc -M mac99,graphics=off -display none -kernel day15/invaders.elf ../../hw/intc/openpic.c:1034:16: runtime error: index -1 out of bounds for type 'IRQSource[264]' (aka 'struct IRQSource[264]') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/intc/openpic.c:1034:16 in This is because we do src = &opp->src[n_IRQ]; when n_IRQ may be -1. This is in practice harmless because if n_IRQ is -1 then we don't do anything with the src pointer, but it is undefined behaviour. (This has been present since this device was first added to QEMU.) Rearrange the code so we only do the array index when n_IRQ is not -1. Cc: qemu-stable@nongnu.org Fixes: e9df014c0b ("Implement embedded IRQ controller for PowerPC 6xx/740 & 75") Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Reviewed-by: Mark Cave-Ayland Message-id: 20241105180205.3074071-1-peter.maydell@linaro.org --- hw/intc/openpic.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c index cd3d87768e..2ead4b9ba0 100644 --- a/hw/intc/openpic.c +++ b/hw/intc/openpic.c @@ -1031,13 +1031,14 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr, s_IRQ = IRQ_get_next(opp, &dst->servicing); /* Check queued interrupts. */ n_IRQ = IRQ_get_next(opp, &dst->raised); - src = &opp->src[n_IRQ]; - if (n_IRQ != -1 && - (s_IRQ == -1 || - IVPR_PRIORITY(src->ivpr) > dst->servicing.priority)) { - DPRINTF("Raise OpenPIC INT output cpu %d irq %d", - idx, n_IRQ); - qemu_irq_raise(opp->dst[idx].irqs[OPENPIC_OUTPUT_INT]); + if (n_IRQ != -1) { + src = &opp->src[n_IRQ]; + if (s_IRQ == -1 || + IVPR_PRIORITY(src->ivpr) > dst->servicing.priority) { + DPRINTF("Raise OpenPIC INT output cpu %d irq %d", + idx, n_IRQ); + qemu_irq_raise(opp->dst[idx].irqs[OPENPIC_OUTPUT_INT]); + } } break; default: