Re: [QEMU-SECURITY] [PATCH] hw/intc/arm_gic: Fix interrupt ID in GICD_SGIR register

[Peter Maydell]

On Tue, 2 Feb 2021 at 09:32, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 2/2/21 7:21 AM, P J P wrote:
> > On Sunday, 31 January, 2021, 08:48:26 pm IST, Philippe Mathieu-Daudé 
> > <f4bug@amsat.org> wrote:
> >> Forwarding to qemu-security@ to see if this issue is worth a CVE.
> >>
> >> | On 1/31/21 11:34 AM, Philippe Mathieu-Daudé wrote:
> >> | > Buglink: https://bugs.launchpad.net/qemu/+bug/1913916
> >> | > Buglink: https://bugs.launchpad.net/qemu/+bug/1913917
> >
> > * Does above patch address both these bugs? For BZ#1913917 'irq' is derived 
> > from 'offset' it seems.
> >
> >         /* Interrupt Configuration.  */
> >         irq = (offset - 0xc00) * 4;
>
> I haven't done a thorough analysis, simply tried to fixed this
> bug ASAP as it is public so many users are exposed.
>
> I had the impression the first call (writel 0x8000f00 0x5affaf)
> break the heap, so the memory is inconsistent when the second
> call (write 0x8000eff 0x1 0x0) is done, but better have developers
> familiar with GIC and security auditing this again.

Philippe is correct here. In both cases the overrun is on the
first writel to 0x8000f00, but the fuzzer has for some reason not
reported that but instead blundered on until it happens to trigger
some other issue that resulted from the memory corruption it induced
with the first write.

> >> | > Correct the irq mask to fix an undefined behavior (which eventually
> >> | > lead to a heap-buffer-overflow, see [Buglink]):
> >> | >
> >> | >    $ echo 'writel 0x8000f00 0xff4affb0' | qemu-system-aarch64 -M 
> >> virt,accel=qtest -qtest stdio
> >> | >    [I 1612088147.116987] OPENED
> >> | >  [R +0.278293] writel 0x8000f00 0xff4affb0
> >> | >  ../hw/intc/arm_gic.c:1498:13: runtime error: index 944 out of bounds 
> >> for type 'uint8_t [16][8]'
> >> | >  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior 
> >> ../hw/intc/arm_gic.c:1498:13
> >> | >
> >> | > Cc: qemu-stable@nongnu.org
> >> | > Fixes: 9ee6e8bb853 ("ARMv7 support.")
> >> |
> >> | > ---
> >> | > Isnt it worth a CVE to help distributions track backports?
> >> | > ---
> >
> > Thank you for reporting this issue. Will process further.

On the CVE:

Since this can affect systems using KVM, this is a security bug for
us. However, it only affects an uncommon configuration:
you are only vulnerable if you are using "kernel-irqchip=off"
(the default is 'on', and turning it off is an odd thing to do).

I've applied this patch to target-arm.next.

thanks
-- PMM