diff mbox series

[v2,1/4] accel/tcg: Invalidate translations when clearing PAGE_READ

Message ID 20220805160914.1106091-2-iii@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series linux-user: Fix siginfo_t contents when jumping to non-readable pages | expand

Commit Message

Ilya Leoshkevich Aug. 5, 2022, 4:09 p.m. UTC
After mprotect(addr, PROT_NONE), addr can still be executed if there
are cached translations. Drop them.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 accel/tcg/translate-all.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Comments

Peter Maydell Aug. 5, 2022, 5:42 p.m. UTC | #1
On Fri, 5 Aug 2022 at 18:33, Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> After mprotect(addr, PROT_NONE), addr can still be executed if there
> are cached translations. Drop them.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  accel/tcg/translate-all.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index ef62a199c7..9318ada6b9 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -2295,12 +2295,19 @@ void page_set_flags(target_ulong start, target_ulong end, int flags)
>           len != 0;
>           len -= TARGET_PAGE_SIZE, addr += TARGET_PAGE_SIZE) {
>          PageDesc *p = page_find_alloc(addr >> TARGET_PAGE_BITS, 1);
> +        bool write_set, read_cleared;
>
> -        /* If the write protection bit is set, then we invalidate
> -           the code inside.  */
> -        if (!(p->flags & PAGE_WRITE) &&
> -            (flags & PAGE_WRITE) &&
> -            p->first_tb) {
> +        /*
> +         * If the write protection bit is set, then we invalidate the code
> +         * inside.
> +         */
> +        write_set = !(p->flags & PAGE_WRITE) && (flags & PAGE_WRITE);
> +        /*
> +         * If PAGE_READ is cleared, we also need to invalidate the code in
> +         * order to force a fault when trying to run it.
> +         */
> +        read_cleared = (p->flags & PAGE_READ) && !(flags & PAGE_READ);

Isn't it architecture-dependent whether you need PAGE_READ
to execute code ? How about PAGE_EXEC ?

thanks
-- PMM
Richard Henderson Aug. 5, 2022, 5:55 p.m. UTC | #2
On 8/5/22 09:09, Ilya Leoshkevich wrote:
> After mprotect(addr, PROT_NONE), addr can still be executed if there
> are cached translations. Drop them.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   accel/tcg/translate-all.c | 17 ++++++++++++-----
>   1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index ef62a199c7..9318ada6b9 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -2295,12 +2295,19 @@ void page_set_flags(target_ulong start, target_ulong end, int flags)
>            len != 0;
>            len -= TARGET_PAGE_SIZE, addr += TARGET_PAGE_SIZE) {
>           PageDesc *p = page_find_alloc(addr >> TARGET_PAGE_BITS, 1);
> +        bool write_set, read_cleared;
>   
> -        /* If the write protection bit is set, then we invalidate
> -           the code inside.  */
> -        if (!(p->flags & PAGE_WRITE) &&
> -            (flags & PAGE_WRITE) &&
> -            p->first_tb) {
> +        /*
> +         * If the write protection bit is set, then we invalidate the code
> +         * inside.
> +         */
> +        write_set = !(p->flags & PAGE_WRITE) && (flags & PAGE_WRITE);
> +        /*
> +         * If PAGE_READ is cleared, we also need to invalidate the code in
> +         * order to force a fault when trying to run it.
> +         */
> +        read_cleared = (p->flags & PAGE_READ) && !(flags & PAGE_READ);

PAGE_READ has nothing to do with it -- PAGE_EXEC does though.


r~
diff mbox series

Patch

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index ef62a199c7..9318ada6b9 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -2295,12 +2295,19 @@  void page_set_flags(target_ulong start, target_ulong end, int flags)
          len != 0;
          len -= TARGET_PAGE_SIZE, addr += TARGET_PAGE_SIZE) {
         PageDesc *p = page_find_alloc(addr >> TARGET_PAGE_BITS, 1);
+        bool write_set, read_cleared;
 
-        /* If the write protection bit is set, then we invalidate
-           the code inside.  */
-        if (!(p->flags & PAGE_WRITE) &&
-            (flags & PAGE_WRITE) &&
-            p->first_tb) {
+        /*
+         * If the write protection bit is set, then we invalidate the code
+         * inside.
+         */
+        write_set = !(p->flags & PAGE_WRITE) && (flags & PAGE_WRITE);
+        /*
+         * If PAGE_READ is cleared, we also need to invalidate the code in
+         * order to force a fault when trying to run it.
+         */
+        read_cleared = (p->flags & PAGE_READ) && !(flags & PAGE_READ);
+        if ((write_set || read_cleared) && p->first_tb) {
             tb_invalidate_phys_page(addr, 0);
         }
         if (reset_target_data) {