The exynos4210 combiner has registers arranged in sets of 4: offset 0x0: IIESR0 (handles interrupt groups 0..3) offset 0x4: IIECR0 offset 0x8: IISTR0 offset 0xc: IIMSR0 offset 0x10: IIESR1 (handles interrupt groups 4..7) offset 0x14: IIECR1
and so on for a total of 16 sets of registers, which puts the final IIMSR15 at offset 0xfc. After that come two registers which aren't grouped, ICIPSR0 at 0x100 and ICIPSR1 at 0x104. The entire register memory region is thus 0x108 bytes. The logic which separates out the "offset is in one of the sets of registers" from "offset is one of the ICIPSRn registers" is incorrect, and so a write to offset 0x10448100 (which should be ICIPSR0) falls through into the code for handling accesses to a set of registers, and indexes off the end of the s->group[] array. For an input offset of, say, 0x3C, req_quad_base_n is 0x3, because this is in the 3rd set of registers. grp_quad_base_n is then 0xC, because this set of registers handles interrupt groups 12..15, and we will index s->group[] with grp_quad_base_n through to grp_quad_base_n + 3. IIC_NGRP is the size of the group[] array, and so we need to check grp_quad_base_n against it, not req_quad_base_n. Fix the bounds check, and add an assertion that the code path where we are beyond the register-set range isn't going to overrun its array, which we know to be true because of the size used for the MemoryRegion. Cc: [email protected] Fixes: 8e03cf1eeb1f9e0 ("ARM: exynos4210: IRQ subsystem support.") Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4709 Signed-off-by: Peter Maydell <[email protected]> --- hw/intc/exynos4210_combiner.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/intc/exynos4210_combiner.c b/hw/intc/exynos4210_combiner.c index dbbdee4d316..4f079fedaff 100644 --- a/hw/intc/exynos4210_combiner.c +++ b/hw/intc/exynos4210_combiner.c @@ -91,8 +91,9 @@ exynos4210_combiner_read(void *opaque, hwaddr offset, unsigned size) grp_quad_base_n = req_quad_base_n << 2; reg_n = (offset - (req_quad_base_n << 4)) >> 2; - if (req_quad_base_n >= IIC_NGRP) { - /* Read of ICIPSR register */ + if (grp_quad_base_n >= IIC_NGRP) { + /* Read of ICIPSR register. MR size ensures this must be in bounds */ + assert(reg_n < ARRAY_SIZE(s->icipsr)); return s->icipsr[reg_n]; } @@ -184,7 +185,7 @@ static void exynos4210_combiner_write(void *opaque, hwaddr offset, grp_quad_base_n = req_quad_base_n << 2; reg_n = (offset - (req_quad_base_n << 4)) >> 2; - if (req_quad_base_n >= IIC_NGRP) { + if (grp_quad_base_n >= IIC_NGRP) { qemu_log_mask(LOG_GUEST_ERROR, "exynos4210.combiner: unallowed write access at offset 0x" HWADDR_FMT_plx "\n", offset); -- 2.43.0
