Re: [PATCH RFC] linux-user: Allow gdbstub to ignore page protection

[Richard Henderson <richard.henderson@linaro.org>]

On 12/16/23 10:24, Ilya Leoshkevich wrote:
> @@ -377,22 +379,42 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
>           flags = page_get_flags(page);
>           if (!(flags & PAGE_VALID))
>               return -1;
> +        prot = ((flags & PAGE_READ) ? PROT_READ : 0) |
> +               ((flags & PAGE_WRITE) ? PROT_WRITE : 0) |
> +               ((flags & PAGE_EXEC) ? PROT_EXEC : 0);

See target_to_host_prot where guest PROT_EXEC is mapped to host PROT_READ.
This should be

	flags & (PAGE_READ | PAGE_EXEC) ? PROT_READ

> +            if (!(prot & PROT_WRITE)) {
> +                if (target_mprotect(page, TARGET_PAGE_SIZE,
> +                                    prot | PROT_WRITE)) {
> +                    return -1;
> +                }
> +            }
>              /* XXX: this code should not depend on lock_user */
> -            if (!(p = lock_user(VERIFY_WRITE, addr, l, 0)))
> +            if (!(p = lock_user(VERIFY_NONE, addr, l, 0)))
>                  return -1;
>              memcpy(p, buf, l);
>              unlock_user(p, addr, l);
> +            if (!(prot & PROT_WRITE)) {
> +                if (target_mprotect(page, TARGET_PAGE_SIZE, prot)) {
> +                    return -1;
> +                }
> +            }

Is the mmap lock held here?  You're opening up a window in which a page may be modified 
but we don't catch the modification to translation blocks.

> +            if (!(prot & PROT_READ)) {
> +                if (target_mprotect(page, TARGET_PAGE_SIZE, prot | PROT_READ)) {
> +                    return -1;
> +                }
> +            }

I really really doubt you need to do this.  The page is not accessible, so why expose it 
at all?  If you want to do this, I wonder if we should instead be interacting with ptrace?


r~