[Qemu-devel] [PATCH for-2.0 v2 0/2] fix bugs involving linux-user signal handling

[Qemu-devel] [PATCH for-2.0 v2 0/2] fix bugs involving linux-user signal handling

[Peter Maydell]

This patch series fixes bugs reported by Andrei Warkentin involving
signal handling in linux-user mode. The first is Andrei's first patch
(though I have tweaked the commit message a little). The second
patch is aimed at fixing the locking bug that Andrei noted, in
a somewhat simpler way than his patches use.

The test cases Andrei provided both pass with these patches.

Changes v1->v2:
 * reset have_tb_lock to false when we unlock it on the
   after-longjmp code path

Andrei Warkentin (1):
  page_check_range: don't bail out early after unprotecting page

Peter Maydell (1):
  cpu-exec: Unlock tb_lock if we longjmp out of code generation

 cpu-exec.c      | 8 ++++++++
 translate-all.c | 1 -
 2 files changed, 8 insertions(+), 1 deletion(-)

-- 
1.9.0

[Qemu-devel] [PATCH for-2.0 v2 1/2] page_check_range: don't bail out early after unprotecting page

[Peter Maydell]

From: Andrei Warkentin <andrey.warkentin@gmail.com>

When checking a page range, if we found that a page was
made read-only by QEMU because it contained translated code,
we were incorrectly returning immediately after unprotecting
that page, rather than continuing to check the entire range,
so we might fail to unprotect pages later in the range, or
might incorrectly return a "success" result even if later
pages were not writable.

In particular, this could cause segfaults in a case where
signals are delivered back to back on a target architecture
which uses trampoline code in the stack frame (as AArch64
currently does). The second signal causes a segfault because
the frame cannot be written to (it was protected because
we translated and executed the restorer trampoline, and the
unprotect logic did not unprotect the whole range).

Signed-off-by: Andrei Warkentin <andrey.warkentin@gmail.com
[PMM: expanded commit message a bit]
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 translate-all.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/translate-all.c b/translate-all.c
index f243c10..5759974 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1777,7 +1777,6 @@ int page_check_range(target_ulong start, target_ulong len, int flags)
                     return -1;
                 }
             }
-            return 0;
         }
     }
     return 0;
-- 
1.9.0

[Qemu-devel] [PATCH for-2.0 v2 2/2] cpu-exec: Unlock tb_lock if we longjmp out of code generation

[Peter Maydell]

If the guest attempts to execute from unreadable memory, this will
cause us to longjmp back to the main loop from inside the
target frontend decoder. For linux-user mode, this means we will
still hold the tb_ctx.tb_lock, and will deadlock when we try to
start executing code again. Unlock the lock in the return-from-longjmp
code path to avoid this.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Acked-by: Andrei Warkentin <andrey.warkentin@gmail.com>
---
 cpu-exec.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/cpu-exec.c b/cpu-exec.c
index 0914d3c..2f54054 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -227,6 +227,8 @@ int cpu_exec(CPUArchState *env)
     TranslationBlock *tb;
     uint8_t *tc_ptr;
     uintptr_t next_tb;
+    /* This must be volatile so it is not trashed by longjmp() */
+    volatile bool have_tb_lock = false;
 
     if (cpu->halted) {
         if (!cpu_has_work(cpu)) {
@@ -600,6 +602,7 @@ int cpu_exec(CPUArchState *env)
                     cpu_loop_exit(cpu);
                 }
                 spin_lock(&tcg_ctx.tb_ctx.tb_lock);
+                have_tb_lock = true;
                 tb = tb_find_fast(env);
                 /* Note: we do it here to avoid a gcc bug on Mac OS X when
                    doing it in tb_find_slow */
@@ -621,6 +624,7 @@ int cpu_exec(CPUArchState *env)
                     tb_add_jump((TranslationBlock *)(next_tb & ~TB_EXIT_MASK),
                                 next_tb & TB_EXIT_MASK, tb);
                 }
+                have_tb_lock = false;
                 spin_unlock(&tcg_ctx.tb_ctx.tb_lock);
 
                 /* cpu_interrupt might be called while translating the
@@ -692,6 +696,10 @@ int cpu_exec(CPUArchState *env)
 #ifdef TARGET_I386
             x86_cpu = X86_CPU(cpu);
 #endif
+            if (have_tb_lock) {
+                spin_unlock(&tcg_ctx.tb_ctx.tb_lock);
+                have_tb_lock = false;
+            }
         }
     } /* for(;;) */
 
-- 
1.9.0

Re: [Qemu-devel] [PATCH for-2.0 v2 2/2] cpu-exec: Unlock tb_lock if we longjmp out of code generation

[Richard Henderson]

On 04/04/2014 04:52 AM, Peter Maydell wrote:
> If the guest attempts to execute from unreadable memory, this will
> cause us to longjmp back to the main loop from inside the
> target frontend decoder. For linux-user mode, this means we will
> still hold the tb_ctx.tb_lock, and will deadlock when we try to
> start executing code again. Unlock the lock in the return-from-longjmp
> code path to avoid this.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Acked-by: Andrei Warkentin <andrey.warkentin@gmail.com>

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~