On 10/19/26 19:22, Peter Maydell wrote:
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(-)
(This is now commit ab6579bf73b in the master branch).
Hi Peter!
This change has been Cc'ed qemu-stable. For 10.0.x series,
I'm also picking up a previous commit,
b6d36a1af79 hw/intc/exynos4210_combiner: Avoid hw_error for guest error
instead of adjusting this change. Please let me know if I shouldn't :)
Overall, it feels like this a niche device, and it's unlikely anyone
actually cares for the stable series -- actual users of this device
in qemu most likely have their own local builds of qemu anyway. But
sure thing I might be wrong.
Thanks!
/mjt
