[v6] linux-user: Fix siginfo_t contents when jumping to non-readable pages

[PATCH v6 09/21] accel/tcg: Unlock mmap_lock after longjmp

Richard Henderson posted 21 patches 3 years, 5 months ago

Diff against v1 v2 v3 v4 v5 v7
Download mbox

Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Riku Voipio <riku.voipio@iki.fi>, Laurent Vivier <laurent@vivier.eu>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@redhat.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Peter Maydell <peter.maydell@linaro.org>, Michael Rolnik <mrolnik@gmail.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Taylor Simpson <tsimpson@quicinc.com>, Eduardo Habkost <eduardo@habkost.net>, Song Gao <gaosong@loongson.cn>, Xiaojuan Yang <yangxiaojuan@loongson.cn>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Chris Wulff <crwulff@gmail.com>, Marek Vasut <marex@denx.de>, Stafford Horne <shorne@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, Cornelia Huck <cohuck@redhat.com>, Thomas Huth <thuth@redhat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Max Filippov <jcmvbkbc@gmail.com>, "Alex Bennée" <alex.bennee@linaro.org>

There is a newer version of this series

Expand all Fold all

[PATCH v6 09/21] accel/tcg: Unlock mmap_lock after longjmp

Posted by Richard Henderson 3 years, 5 months ago

The mmap_lock is held around tb_gen_code.  While the comment
is correct that the lock is dropped when tb_gen_code runs out
of memory, the lock is *not* dropped when an exception is
raised reading code for translation.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/cpu-exec.c  | 12 ++++++------
 accel/tcg/user-exec.c |  3 ---
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index a565a3f8ec..d18081ca6f 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -462,13 +462,11 @@ void cpu_exec_step_atomic(CPUState *cpu)
         cpu_tb_exec(cpu, tb, &tb_exit);
         cpu_exec_exit(cpu);
     } else {
-        /*
-         * The mmap_lock is dropped by tb_gen_code if it runs out of
-         * memory.
-         */
 #ifndef CONFIG_SOFTMMU
         clear_helper_retaddr();
-        tcg_debug_assert(!have_mmap_lock());
+        if (have_mmap_lock()) {
+            mmap_unlock();
+        }
 #endif
         if (qemu_mutex_iothread_locked()) {
             qemu_mutex_unlock_iothread();
@@ -936,7 +934,9 @@ int cpu_exec(CPUState *cpu)
 
 #ifndef CONFIG_SOFTMMU
         clear_helper_retaddr();
-        tcg_debug_assert(!have_mmap_lock());
+        if (have_mmap_lock()) {
+            mmap_unlock();
+        }
 #endif
         if (qemu_mutex_iothread_locked()) {
             qemu_mutex_unlock_iothread();
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index a20234fb02..58edd33896 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -80,10 +80,7 @@ MMUAccessType adjust_signal_pc(uintptr_t *pc, bool is_write)
          * (and if the translator doesn't handle page boundaries correctly
          * there's little we can do about that here).  Therefore, do not
          * trigger the unwinder.
-         *
-         * Like tb_gen_code, release the memory lock before cpu_loop_exit.
          */
-        mmap_unlock();
         *pc = 0;
         return MMU_INST_FETCH;
     }
-- 
2.34.1

Re: [PATCH v6 09/21] accel/tcg: Unlock mmap_lock after longjmp

Posted by Alistair Francis 3 years, 5 months ago

On Fri, Aug 19, 2022 at 1:29 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The mmap_lock is held around tb_gen_code.  While the comment
> is correct that the lock is dropped when tb_gen_code runs out
> of memory, the lock is *not* dropped when an exception is
> raised reading code for translation.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  accel/tcg/cpu-exec.c  | 12 ++++++------
>  accel/tcg/user-exec.c |  3 ---
>  2 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index a565a3f8ec..d18081ca6f 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -462,13 +462,11 @@ void cpu_exec_step_atomic(CPUState *cpu)
>          cpu_tb_exec(cpu, tb, &tb_exit);
>          cpu_exec_exit(cpu);
>      } else {
> -        /*
> -         * The mmap_lock is dropped by tb_gen_code if it runs out of
> -         * memory.
> -         */
>  #ifndef CONFIG_SOFTMMU
>          clear_helper_retaddr();
> -        tcg_debug_assert(!have_mmap_lock());
> +        if (have_mmap_lock()) {
> +            mmap_unlock();
> +        }
>  #endif
>          if (qemu_mutex_iothread_locked()) {
>              qemu_mutex_unlock_iothread();
> @@ -936,7 +934,9 @@ int cpu_exec(CPUState *cpu)
>
>  #ifndef CONFIG_SOFTMMU
>          clear_helper_retaddr();
> -        tcg_debug_assert(!have_mmap_lock());
> +        if (have_mmap_lock()) {
> +            mmap_unlock();
> +        }
>  #endif
>          if (qemu_mutex_iothread_locked()) {
>              qemu_mutex_unlock_iothread();
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index a20234fb02..58edd33896 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -80,10 +80,7 @@ MMUAccessType adjust_signal_pc(uintptr_t *pc, bool is_write)
>           * (and if the translator doesn't handle page boundaries correctly
>           * there's little we can do about that here).  Therefore, do not
>           * trigger the unwinder.
> -         *
> -         * Like tb_gen_code, release the memory lock before cpu_loop_exit.
>           */
> -        mmap_unlock();
>          *pc = 0;
>          return MMU_INST_FETCH;
>      }
> --
> 2.34.1
>
>