diff mbox

[for-2.0?,1/2] page_check_range: don't bail out early after unprotecting page

Message ID 1396543508-12280-2-git-send-email-peter.maydell@linaro.org
State New
Headers show

Commit Message

Peter Maydell April 3, 2014, 4:45 p.m. UTC
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]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 translate-all.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Richard Henderson April 3, 2014, 6:26 p.m. UTC | #1
On 04/03/2014 09:45 AM, Peter Maydell wrote:
> 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]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  translate-all.c | 1 -
>  1 file changed, 1 deletion(-)

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


r~
diff mbox

Patch

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;