From: Paolo Bonzini <pbonzini@redhat.com>
To: "Alex Bennée" <alex.bennee@linaro.org>, qemu-devel@nongnu.org
Cc: Bug 1863025 <1863025@bugs.launchpad.net>, Yifan <me@yifanlu.com>,
Richard Henderson <rth@twiddle.net>
Subject: Re: [PATCH] accel/tcg: fix race in cpu_exec_step_atomic (bug 1863025)
Date: Fri, 14 Feb 2020 16:22:34 +0100 [thread overview]
Message-ID: <244ec8f5-5f19-ca98-5cbe-c3ec39494222@redhat.com> (raw)
In-Reply-To: <20200214144952.15502-1-alex.bennee@linaro.org>
On 14/02/20 15:49, 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.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Yifan <me@yifanlu.com>
> Cc: Bug 1863025 <1863025@bugs.launchpad.net>
> ---
> accel/tcg/cpu-exec.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 2560c90eec7..d95c4848a47 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -240,6 +240,8 @@ void cpu_exec_step_atomic(CPUState *cpu)
> uint32_t cf_mask = cflags & CF_HASH_MASK;
>
> if (sigsetjmp(cpu->jmp_env, 0) == 0) {
> + start_exclusive();
> +
> tb = tb_lookup__cpu_state(cpu, &pc, &cs_base, &flags, cf_mask);
> if (tb == NULL) {
> mmap_lock();
> @@ -247,8 +249,6 @@ void cpu_exec_step_atomic(CPUState *cpu)
> mmap_unlock();
> }
>
> - start_exclusive();
> -
> /* Since we got here, we know that parallel_cpus must be true. */
> parallel_cpus = false;
> cc->cpu_exec_enter(cpu);
> @@ -271,14 +271,15 @@ void cpu_exec_step_atomic(CPUState *cpu)
> qemu_plugin_disable_mem_helpers(cpu);
> }
>
> - if (cpu_in_exclusive_context(cpu)) {
> - /* We might longjump out of either the codegen or the
> - * execution, so must make sure we only end the exclusive
> - * region if we started it.
> - */
> - parallel_cpus = true;
> - end_exclusive();
> - }
> +
> + /*
> + * As we start the exclusive region before codegen we must still
> + * be in the region if we longjump out of either the codegen or
> + * the execution.
> + */
> + g_assert(cpu_in_exclusive_context(cpu));
> + parallel_cpus = true;
> + end_exclusive();
> }
>
> struct tb_desc {
>
next prev parent reply other threads:[~2020-02-14 15:27 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-12 22:01 [Bug 1863025] [NEW] Use-after-free after flush in TCG accelerator Yifan
2020-02-14 14:23 ` [Bug 1863025] " Alex Bennée
2020-02-14 14:29 ` Alex Bennée
2020-02-14 14:49 ` [PATCH] accel/tcg: fix race in cpu_exec_step_atomic (bug 1863025) Alex Bennée
2020-02-14 14:49 ` [Bug 1863025] Re: Use-after-free after flush in TCG accelerator Alex Bennée
2020-02-14 15:22 ` Paolo Bonzini [this message]
2020-02-14 23:31 ` [PATCH] accel/tcg: fix race in cpu_exec_step_atomic (bug 1863025) Richard Henderson
2020-02-15 0:01 ` Yifan Lu
2020-02-15 0:01 ` [Bug 1863025] " Yifan
2020-02-14 14:51 ` [Bug 1863025] Re: Use-after-free after flush in TCG accelerator Alex Bennée
2020-02-14 18:09 ` Yifan
2020-02-14 18:18 ` Yifan
2020-03-10 9:14 ` Laurent Vivier
2020-04-30 13:43 ` Laurent Vivier
2023-08-31 12:48 ` Samuel Henrique
2023-08-31 13:40 ` Philippe Mathieu-Daudé
2023-08-31 13:57 ` Daniel P. Berrangé
2023-08-31 13:57 ` Daniel Berrange
2023-08-31 14:10 ` Mauro Matteo Cascella
2023-08-31 14:10 ` Mauro Matteo Cascella
2023-08-31 14:12 ` Mauro Matteo Cascella
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=244ec8f5-5f19-ca98-5cbe-c3ec39494222@redhat.com \
--to=pbonzini@redhat.com \
--cc=1863025@bugs.launchpad.net \
--cc=alex.bennee@linaro.org \
--cc=me@yifanlu.com \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).