Stian Halseth <[email protected]> writes:
> In a serial context, tb_gen_code() flushes a full code buffer and
> retries in place. cpu_exec_loop() then chains its last_tb, now freed,
> into the new TB. When that TB is invalidated, tb_jmp_unlink() follows
> the stale entry and qemu crashes inside its SIGSEGV handler.
I wonder how much:
/* flush must be done */
if (cpu_in_serial_context(cpu)) {
trace_tb_gen_code_buffer_overflow("tcg_tb_alloc");
tb_flush__exclusive_or_serial();
goto buffer_overflow;
}
actually saves? Because we are skipping both cleanup in cpu_loop_exit
(the mem helpers) and cpu_exec_longjmp_cleanup which we get to when we
jump out.
>
> Drop last_tb when tb_gen_code() has flushed.
>
> User-only builds hit this easily since cross-page goto_tb chaining,
> e.g. qemu-sparc64 -tb-size 1 on a program writing next to its PLT.
>
> Fixes: ebf7a5d294a3 ("accel/tcg: Improve buffer overflow in tb_gen_code")
> Fixes: 25e5c6a320db ("accel/tcg: Allow cross-page goto_tb chaining in
> user-only builds")
> Signed-off-by: Stian Halseth <[email protected]>
> ---
> 25e5c6a320db is not in a release yet (v11.1.x does not have it), so the
> crash is new in the 11.2 cycle.
>
> Reproducers available on request: on x86_64 the stale chain happens
> too but has stayed harmless; on sparc64 it crashes every time.
>
> accel/tcg/cpu-exec.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 5b4bca5084..600e8c5fa8 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -968,11 +968,17 @@ cpu_exec_loop(CPUState *cpu, SyncClocks *sc)
> if (tb == NULL) {
> CPUJumpCache *jc;
> uint32_t h;
> + unsigned flush_count = qatomic_read(&tb_ctx.tb_flush_count);
>
> mmap_lock();
> tb = tb_gen_code(cpu, s);
> mmap_unlock();
>
> + /* tb_gen_code may have flushed, freeing last_tb. */
> + if (flush_count != qatomic_read(&tb_ctx.tb_flush_count)) {
> + last_tb = NULL;
> + }
> +
> /*
> * We add the TB in the virtual pc hash table
> * for the fast lookup
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
