On 1/28/21 6:18 PM, Alexander Bulekov wrote:
> 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 <alx...@bu.edu>
>
> Reviewed-by: Alexander Bulekov <alx...@bu.edu>
>
> For reference, some older threads about similar issues in the GDB stub
> and monitor:
> https://bugs.launchpad.net/qemu/+bug/1602247
This one is different. I thought this issue was fixed by
the series around commit 7cf48f6752e ("gdbstub: add multiprocess
support to (f|s)ThreadInfo and ThreadExtraInfo").
When using physical addresses with gdbstub, we should be able to
select a particular address space.
Maybe this fixes pmemsave accessing MMIO here:
https://bugs.launchpad.net/qemu/+bug/1751674
> https://bugs.launchpad.net/qemu/+bug/1878645
> https://patchew.org/QEMU/20200701182100.26930-1-phi...@redhat.com/
I'm still procrastinating this thread :>
>
>> Signed-off-by: Philippe Mathieu-Daudé <phi...@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
>>
>>
>
