Re: [PATCH 1/3] linux-user: Allow gdbstub to ignore page protection

[Ilya Leoshkevich]

On Wed, 2024-01-10 at 04:42 +1100, Richard Henderson wrote:
> On 1/9/24 10:34, Ilya Leoshkevich wrote:
> > gdbserver ignores page protection by virtue of using
> > /proc/$pid/mem.
> > Teach qemu gdbstub to do this too. This will not work if /proc is
> > not
> > mounted; accept this limitation.
> > 
> > One alternative is to temporarily grant the missing PROT_* bit, but
> > this is inherently racy. Another alternative is self-debugging with
> > ptrace(POKE), which will break if QEMU itself is being debugged - a
> > much more severe limitation.
> > 
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >   cpu-target.c | 55 ++++++++++++++++++++++++++++++++++++++---------
> > -----
> >   1 file changed, 40 insertions(+), 15 deletions(-)
> > 
> > diff --git a/cpu-target.c b/cpu-target.c
> > index 5eecd7ea2d7..69e97f78980 100644
> > --- a/cpu-target.c
> > +++ b/cpu-target.c
> > @@ -406,6 +406,15 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr
> > addr,
> >       vaddr l, page;
> >       void * p;
> >       uint8_t *buf = ptr;
> > +    int ret = -1;
> > +    int mem_fd;
> > +
> > +    /*
> > +     * Try ptrace first. If /proc is not mounted or if there is a
> > different
> > +     * problem, fall back to the manual page access. Note that,
> > unlike ptrace,
> > +     * it will not be able to ignore the protection bits.
> > +     */
> > +    mem_fd = open("/proc/self/mem", is_write ? O_WRONLY :
> > O_RDONLY);
> 
> Surely this is the unlikely fallback, and you don't need to open
> unless the page is 
> otherwise inaccessible.

Ok, I can move this under (flags & PAGE_*) checks.

> I see no handling for writes to pages that contain TranslationBlocks.

Sorry, I completely missed that. I'm currently experimenting with the
following:

        /*
         * If there is a TranslationBlock and we weren't bypassing
host
         * page protection, the memcpy() above would SEGV, ultimately
         * leading to page_unprotect(). So invalidate the translations
         * manually. Both invalidation and pwrite() must be under
         * mmap_lock() in order to prevent the creation of another
         * TranslationBlock in between.
         */
        mmap_lock();
        tb_invalidate_phys_page(page);
        written = pwrite(fd, buf, l, (off_t)g2h_untagged(addr));
        mmap_unlock();

Does that look okay?

[...]