From 09bbdb89bc25660044c946137ec7ccb0d1fcee32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 28 Jan 2021 17:14:17 +0100 Subject: hw/intc/arm_gic: Allow to use QTest without crashing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Darren Kenny Reviewed-by: Alexander Bulekov Message-id: 20210128161417.3726358-1-philmd@redhat.com Signed-off-by: Peter Maydell --- hw/intc/arm_gic.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index af41e2f..c33b1c8 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; -- cgit v1.1 From edfe2eb4360cde4ed5d95bda7777edcb3510f76a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sun, 31 Jan 2021 11:34:01 +0100 Subject: hw/intc/arm_gic: Fix interrupt ID in GICD_SGIR register MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the ARM Generic Interrupt Controller Architecture specification (document "ARM IHI 0048B.b (ID072613)"), the SGIINTID field is 4 bit, not 10: - 4.3 Distributor register descriptions - 4.3.15 Software Generated Interrupt Register, GICD_SG - Table 4-21 GICD_SGIR bit assignments The Interrupt ID of the SGI to forward to the specified CPU interfaces. The value of this field is the Interrupt ID, in the range 0-15, for example a value of 0b0011 specifies Interrupt ID 3. 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 This fixes a security issue when running with KVM on Arm with kernel-irqchip=off. (The default is kernel-irqchip=on, which is unaffected, and which is also the correct choice for performance.) Cc: qemu-stable@nongnu.org Fixes: CVE-2021-20221 Fixes: 9ee6e8bb853 ("ARMv7 support.") Buglink: https://bugs.launchpad.net/qemu/+bug/1913916 Buglink: https://bugs.launchpad.net/qemu/+bug/1913917 Reported-by: Alexander Bulekov Signed-off-by: Philippe Mathieu-Daudé Message-id: 20210131103401.217160-1-f4bug@amsat.org Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/intc/arm_gic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index c33b1c8..a994b1f 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -1477,7 +1477,7 @@ static void gic_dist_writel(void *opaque, hwaddr offset, int target_cpu; cpu = gic_get_current_cpu(s); - irq = value & 0x3ff; + irq = value & 0xf; switch ((value >> 24) & 3) { case 0: mask = (value >> 16) & ALL_CPU_MASK; -- cgit v1.1