Re: [Qemu-devel] [PATCH v2] translate-all: Bugfix for user-mode self-modifying code in 2 page long TB

[Стас Шмаров]

May be this can be done faster.
After invalidation of current TB, we know next executed TB will be 1
instruction long and will write to same address, and will cause segfault.
In sigfault handler write protection to this page will be disabled.
So, we can retranslate TB, and remove protection before execution of new
TB. It will be one signal less.

2016-07-06 13:21 GMT+03:00 Sergey Fedorov <address@hidden>:

> On 06/07/16 10:54, Stanislav Shmarov wrote:
> > In user-mode emulation Translation Block can consist of 2 guest pages.
> > In that case QEMU also mprotects 2 host pages that are dedicated for
> > guest memory, containing instructions. QEMU detects self-modifying code
> > with SEGFAULT signal processing.
> >
> > In case if instruction in 1st page is modifying memory of 2nd
> > page (or vice versa) QEMU will mark 2nd page with PAGE_WRITE,
> > invalidate TB, generate new TB contatining 1 guest instruction and
> > exit to CPU loop. QEMU won't call mprotect, and new TB will cause
> > same SEGFAULT. Page will have both PAGE_WRITE_ORG and PAGE_WRITE
> > flags, so QEMU will handle the signal as guest binary problem,
> > and exit with guest SEGFAULT.
> >
> > Solution is retranslate TB before marking pages as PAGE_WRITE,
> > and remove protection with mprotect on second SEGFAULT.
> >
> > Signed-off-by: Stanislav Shmarov <address@hidden>
>
> Reviewed-by: Sergey Fedorov <address@hidden>
>
> > ---
> >   v2: Moved setting PAGE_WRITE flag to separte loop, to cover cases,
> >       pointed by Sergey Fedorov.
> >
> >  translate-all.c | 17 +++++++++++------
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/translate-all.c b/translate-all.c
> > index eaa95e4..fb3743f 100644
> > --- a/translate-all.c
> > +++ b/translate-all.c
> > @@ -2020,13 +2020,8 @@ int page_unprotect(target_ulong address,
> uintptr_t pc)
> >          host_start = address & qemu_host_page_mask;
> >          host_end = host_start + qemu_host_page_size;
> >
> > -        prot = 0;
> >          for (addr = host_start ; addr < host_end ; addr +=
> TARGET_PAGE_SIZE) {
> > -            p = page_find(addr >> TARGET_PAGE_BITS);
> > -            p->flags |= PAGE_WRITE;
> > -            prot |= p->flags;
> > -
> > -            /* and since the content will be modified, we must
> invalidate
> > +            /* Since the content will be modified, we must invalidate
> >                 the corresponding translated code. */
> >              if (tb_invalidate_phys_page(addr, pc)) {
> >                  mmap_unlock();
> > @@ -2036,6 +2031,16 @@ int page_unprotect(target_ulong address,
> uintptr_t pc)
> >              tb_invalidate_check(addr);
> >  #endif
> >          }
> > +
> > +        /* If we got here, current TB have been retranslated (in case of
> > +         * self-modifying code), now it's safe to remove page
> protection.
> > +         */
> > +        prot = 0;
> > +        for (addr = host_start ; addr < host_end ; addr +=
> TARGET_PAGE_SIZE) {
> > +            p = page_find(addr >> TARGET_PAGE_BITS);
> > +            p->flags |= PAGE_WRITE;
> > +            prot |= p->flags;
> > +        }
> >          mprotect((void *)g2h(host_start), qemu_host_page_size,
> >                   prot & PAGE_BITS);
> >
>
>