On Sun, 25 Oct 2026 at 05:43, Michael Tokarev <[email protected]> wrote: > > 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.
Yes; I cc'd stable on this mostly because it's an array overflow, but as you say it's niche, and this device isn't covered by our security-policy as it's TCG only. Picking up that earlier commit is fine if that's how you'd prefer to avoid the merge conflict. thanks -- PMM
