From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40425) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bL83j-00020P-Ov for qemu-devel@nongnu.org; Thu, 07 Jul 2016 08:05:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bL83d-0004Rp-LX for qemu-devel@nongnu.org; Thu, 07 Jul 2016 08:05:34 -0400 Received: from mail-lf0-x22e.google.com ([2a00:1450:4010:c07::22e]:36226) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bL83d-0004Rc-1a for qemu-devel@nongnu.org; Thu, 07 Jul 2016 08:05:29 -0400 Received: by mail-lf0-x22e.google.com with SMTP id q132so9846845lfe.3 for ; Thu, 07 Jul 2016 05:05:28 -0700 (PDT) References: <1467880392-1043630-1-git-send-email-snarpix@gmail.com> From: Sergey Fedorov Message-ID: <577E4586.3090600@linaro.org> Date: Thu, 7 Jul 2016 15:05:26 +0300 MIME-Version: 1.0 In-Reply-To: <1467880392-1043630-1-git-send-email-snarpix@gmail.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3] translate-all: Bugfix for user-mode self-modifying code in 2 page long TB List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stanislav Shmarov , qemu-devel@nongnu.org Cc: Paolo Bonzini , Peter Crosthwaite , Richard Henderson , Sergey Fedorov On 07/07/16 11:33, 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 to do following: In case if current TB was invalidated > continue to invalidate TBs from remaining guest pages and mark pages > as PAGE_WRITE. After that disable host page protection with mprotect. > If current tb was invalidated longjmp to main loop. That is more > efficient, since we won't get SEGFAULT when executing new TB. > > Signed-off-by: Stanislav Shmarov Reviewed-by: Sergey Fedorov > --- > v3: Now mprotect is called on first SEGFAULT. (Significant changes) > v2: Moved setting PAGE_WRITE flag to separte loop, to cover cases, > pointed by Sergey Fedorov. > translate-all.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/translate-all.c b/translate-all.c > index eaa95e4..0d47c1c 100644 > --- a/translate-all.c > +++ b/translate-all.c > @@ -2000,6 +2000,7 @@ int page_check_range(target_ulong start, target_ulong len, int flags) > int page_unprotect(target_ulong address, uintptr_t pc) > { > unsigned int prot; > + bool current_tb_invalidated; > PageDesc *p; > target_ulong host_start, host_end, addr; > > @@ -2021,6 +2022,7 @@ int page_unprotect(target_ulong address, uintptr_t pc) > host_end = host_start + qemu_host_page_size; > > prot = 0; > + current_tb_invalidated = false; > for (addr = host_start ; addr < host_end ; addr += TARGET_PAGE_SIZE) { > p = page_find(addr >> TARGET_PAGE_BITS); > p->flags |= PAGE_WRITE; > @@ -2028,10 +2030,7 @@ int page_unprotect(target_ulong address, uintptr_t pc) > > /* and since the content will be modified, we must invalidate > the corresponding translated code. */ > - if (tb_invalidate_phys_page(addr, pc)) { > - mmap_unlock(); > - return 2; > - } > + current_tb_invalidated |= tb_invalidate_phys_page(addr, pc); > #ifdef DEBUG_TB_CHECK > tb_invalidate_check(addr); > #endif > @@ -2040,7 +2039,8 @@ int page_unprotect(target_ulong address, uintptr_t pc) > prot & PAGE_BITS); > > mmap_unlock(); > - return 1; > + /* If current TB was invalidated return to main loop */ > + return current_tb_invalidated ? 2 : 1; > } > mmap_unlock(); > return 0;