Re: [PATCH v3 1/4] accel/tcg: Invalidate translations when clearing PAGE_EXEC

[Richard Henderson]

On 8/8/22 10:10, 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..32ea5f0adf 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, exec_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_EXEC is cleared, we also need to invalidate the code in
> +         * order to force a fault when trying to run it.
> +         */
> +        exec_cleared = (p->flags & PAGE_EXEC) && !(flags & PAGE_EXEC);
> +        if ((write_set || exec_cleared) && p->first_tb) {

I believe the bug you're trying to fix is in get_page_addr_code, which for USER_ONLY is 
currently a no-op.  It ought to be checking the page permissions there, as we do for softmmu.

I have a patch for get_page_addr_code in the works, because I was working on pther stuff 
in the area.


r~