Re: [PATCH] hw/intc/arm_gic: Allow to use QTest without crashing

[Alexander Bulekov]

On 210128 1714, Philippe Mathieu-Daudé wrote:
> Alexander reported an issue in gic_get_current_cpu() using the
> fuzzer. Yet another "deref current_cpu with QTest" bug, reproducible
> doing:
> 
>   $ echo readb 0xf03ff000 | qemu-system-arm -M npcm750-evb,accel=qtest -qtest 
> stdio
>   [I 1611849440.651452] OPENED
>   [R +0.242498] readb 0xf03ff000
>   hw/intc/arm_gic.c:63:29: runtime error: member access within null pointer 
> of type 'CPUState' (aka 'struct CPUState')
>   SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior 
> hw/intc/arm_gic.c:63:29 in
>   AddressSanitizer:DEADLYSIGNAL
>   =================================================================
>   ==3719691==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000082a0 
> (pc 0x5618790ac882 bp 0x7ffca946f4f0 sp 0x7ffca946f4a0 T0)
>   ==3719691==The signal is caused by a READ memory access.
>       #0 0x5618790ac882 in gic_get_current_cpu hw/intc/arm_gic.c:63:29
>       #1 0x5618790a8901 in gic_dist_readb hw/intc/arm_gic.c:955:11
>       #2 0x5618790a7489 in gic_dist_read hw/intc/arm_gic.c:1158:17
>       #3 0x56187adc573b in memory_region_read_with_attrs_accessor 
> softmmu/memory.c:464:9
>       #4 0x56187ad7903a in access_with_adjusted_size softmmu/memory.c:552:18
>       #5 0x56187ad766d6 in memory_region_dispatch_read1 
> softmmu/memory.c:1426:16
>       #6 0x56187ad758a8 in memory_region_dispatch_read softmmu/memory.c:1449:9
>       #7 0x56187b09e84c in flatview_read_continue softmmu/physmem.c:2822:23
>       #8 0x56187b0a0115 in flatview_read softmmu/physmem.c:2862:12
>       #9 0x56187b09fc9e in address_space_read_full softmmu/physmem.c:2875:18
>       #10 0x56187aa88633 in address_space_read include/exec/memory.h:2489:18
>       #11 0x56187aa88633 in qtest_process_command softmmu/qtest.c:558:13
>       #12 0x56187aa81881 in qtest_process_inbuf softmmu/qtest.c:797:9
>       #13 0x56187aa80e02 in qtest_read softmmu/qtest.c:809:5
> 
> current_cpu is NULL because QTest accelerator does not use CPU.
> 
> Fix by skipping the check and returning the first CPU index when
> QTest accelerator is used, similarly to commit c781a2cc423
> ("hw/i386/vmport: Allow QTest use without crashing").
> 
> Reported-by: Alexander Bulekov <alxndr@bu.edu>

Reviewed-by: Alexander Bulekov <alxndr@bu.edu>

For reference, some older threads about similar issues in the GDB stub
and monitor:
https://bugs.launchpad.net/qemu/+bug/1602247
https://patchew.org/QEMU/20200701182100.26930-1-philmd@redhat.com/
https://bugs.launchpad.net/qemu/+bug/1878645


> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/intc/arm_gic.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index af41e2fb448..c33b1c8c4bc 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -28,6 +28,7 @@
>  #include "qemu/module.h"
>  #include "trace.h"
>  #include "sysemu/kvm.h"
> +#include "sysemu/qtest.h"
>  
>  /* #define DEBUG_GIC */
>  
> @@ -57,7 +58,7 @@ static const uint8_t gic_id_gicv2[] = {
>  
>  static inline int gic_get_current_cpu(GICState *s)
>  {
> -    if (s->num_cpu > 1) {
> +    if (!qtest_enabled() && s->num_cpu > 1) {
>          return current_cpu->cpu_index;
>      }
>      return 0;
> -- 
> 2.26.2
> 
>