From 488b6cc82bd8c47bc043f556c8a32d08769bd850 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 1 Mar 2018 09:10:52 -0500 Subject: [PATCH] exec.c: Fix breakpoint invalidation race A bug (1647683) was reported showing a crash when removing breakpoints. The reproducer was bisected to 3359baad when tb_flush was finally made thread safe. While in MTTCG the locking in breakpoint_invalidate would have prevented any problems, but currently tb_lock() is a NOP for system emulation. The race is between a tb_flush from the gdbstub and the tb_invalidate_phys_addr() in breakpoint_invalidate(). Ideally we'd have actual locking here; for the moment the simple fix is to do a full tb_flush() for a bp invalidate, since that is thread-safe even if no lock is taken. Backports commit a9353fe897ca2687e5b3385ed39e3db3927a90e0 from qemu --- qemu/exec.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/qemu/exec.c b/qemu/exec.c index 7fa202bb..904fc806 100644 --- a/qemu/exec.c +++ b/qemu/exec.c @@ -522,29 +522,15 @@ void cpu_exec_init(CPUState *cpu, void *opaque) #endif } -#if defined(CONFIG_USER_ONLY) static void breakpoint_invalidate(CPUState *cpu, target_ulong pc) { - // Unicorn: commented out - mmap_lock(); - //tb_lock(); - tb_invalidate_phys_page_range(pc, pc + 1, 0); - //tb_unlock(); - mmap_unlock(); + /* Flush the whole TB as this will not have race conditions + * even if we don't have proper locking yet. + * Ideally we would just invalidate the TBs for the + * specified PC. + */ + tb_flush(cpu); } -#else -static void breakpoint_invalidate(CPUState *cpu, target_ulong pc) -{ - MemTxAttrs attrs; - hwaddr phys = cpu_get_phys_page_attrs_debug(cpu, pc, &attrs); - int asidx = cpu_asidx_from_attrs(cpu, attrs); - if (phys != -1) { - /* Locks grabbed by tb_invalidate_phys_addr */ - tb_invalidate_phys_addr(cpu->cpu_ases[asidx].as, - phys | (pc & ~TARGET_PAGE_MASK)); - } -} -#endif #if defined(CONFIG_USER_ONLY) void cpu_watchpoint_remove_all(CPUState *cpu, int mask)