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

[Alexander Bulekov]

On 210202 1221, Peter Maydell wrote:
> 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.
> 

It happens in the standalone reproducer build with
ASAN/--enable-sanitizers, so it seems like an oversight/bug in the
Sanitizers, rather than the fuzzer. It is strange that UBSan seemed to
detect an OOB index, but proceeded as normal. I should start paying
closer attention to those non-fatal UBSan errors.

> > >> | > 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.
>

Ah I had a gut feeling there was a GIC in kvm. CVE or no CVE, --n_bugs
-Alex

> thanks
> -- PMM