[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] accel/tcg: fix race in cpu_exec_step_atomic (bug 1863025)
From: |
Richard Henderson |
Subject: |
Re: [PATCH] accel/tcg: fix race in cpu_exec_step_atomic (bug 1863025) |
Date: |
Fri, 14 Feb 2020 15:31:08 -0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 |
On 2/14/20 6:49 AM, Alex Bennée wrote:
> The bug describes a race whereby cpu_exec_step_atomic can acquire a TB
> which is invalidated by a tb_flush before we execute it. This doesn't
> affect the other cpu_exec modes as a tb_flush by it's nature can only
> occur on a quiescent system. The race was described as:
>
> B2. tcg_cpu_exec => cpu_exec => tb_find => tb_gen_code
> B3. tcg_tb_alloc obtains a new TB
>
> C3. TB obtained with tb_lookup__cpu_state or tb_gen_code
> (same TB as B2)
>
> A3. start_exclusive critical section entered
> A4. do_tb_flush is called, TB memory freed/re-allocated
> A5. end_exclusive exits critical section
>
> B2. tcg_cpu_exec => cpu_exec => tb_find => tb_gen_code
> B3. tcg_tb_alloc reallocates TB from B2
>
> C4. start_exclusive critical section entered
> C5. cpu_tb_exec executes the TB code that was free in A4
>
> The simplest fix is to widen the exclusive period to include the TB
> lookup. As a result we can drop the complication of checking we are in
> the exclusive region before we end it.
I'm not 100% keen on having the tb_gen_code within the exclusive region. It
implies a much larger delay on (at least) the first execution of the atomic
operation.
But I suppose until recently we had a global lock around code generation, and
this is only slightly worse. Plus, it has the advantage of being dead simple,
and without the races vs tb_ctx.tb_flush_count that exist in Yifan's patch.
Applied to tcg-next.
r~