From nobody Fri May 3 22:00:40 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 151187986547782.49424248120613; Tue, 28 Nov 2017 06:37:45 -0800 (PST) Received: from localhost ([::1]:38293 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJh14-0006ih-PT for importer@patchew.org; Tue, 28 Nov 2017 09:37:42 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44009) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJgzJ-0004RE-80 for qemu-devel@nongnu.org; Tue, 28 Nov 2017 09:35:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eJgzI-0003rM-57 for qemu-devel@nongnu.org; Tue, 28 Nov 2017 09:35:53 -0500 Received: from orth.archaic.org.uk ([2001:8b0:1d0::2]:38580) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eJgzH-0003lI-TK for qemu-devel@nongnu.org; Tue, 28 Nov 2017 09:35:52 -0500 Received: from pm215 by orth.archaic.org.uk with local (Exim 4.89) (envelope-from ) id 1eJgyt-00020d-Dg; Tue, 28 Nov 2017 14:35:27 +0000 From: Peter Maydell To: qemu-devel@nongnu.org Date: Tue, 28 Nov 2017 14:35:24 +0000 Message-Id: <1511879725-9576-2-git-send-email-peter.maydell@linaro.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1511879725-9576-1-git-send-email-peter.maydell@linaro.org> References: <1511879725-9576-1-git-send-email-peter.maydell@linaro.org> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:8b0:1d0::2 Subject: [Qemu-devel] [PATCH 1/2] linux-user: Propagate siginfo_t through to handle_cpu_signal() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paolo Bonzini , Riku Voipio , Richard Henderson , Laurent Vivier , patches@linaro.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Currently all the architecture/OS specific cpu_signal_handler() functions call handle_cpu_signal() without passing it the siginfo_t. We're going to want that so we can look at the si_code to determine whether this is a SEGV_ACCERR access violation or some other kind of fault, so change the functions to pass through the pointer to the siginfo_t rather than just the si_addr value. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson --- accel/tcg/user-exec.c | 39 ++++++++++++++------------------------- 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c index f42285e..e8f26ff 100644 --- a/accel/tcg/user-exec.c +++ b/accel/tcg/user-exec.c @@ -57,12 +57,13 @@ static void cpu_exit_tb_from_sighandler(CPUState *cpu, = sigset_t *old_set) the effective address of the memory exception. 'is_write' is 1 if a write caused the exception and otherwise 0'. 'old_set' is the signal set which should be restored */ -static inline int handle_cpu_signal(uintptr_t pc, unsigned long address, +static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, int is_write, sigset_t *old_set) { CPUState *cpu =3D current_cpu; CPUClass *cc; int ret; + unsigned long address =3D (unsigned long)info->si_addr; =20 /* We must handle PC addresses from two different sources: * a call return address and a signal frame address. @@ -215,9 +216,8 @@ int cpu_signal_handler(int host_signum, void *pinfo, #endif pc =3D EIP_sig(uc); trapno =3D TRAP_sig(uc); - return handle_cpu_signal(pc, (unsigned long)info->si_addr, - trapno =3D=3D 0xe ? - (ERROR_sig(uc) >> 1) & 1 : 0, + return handle_cpu_signal(pc, info, + trapno =3D=3D 0xe ? (ERROR_sig(uc) >> 1) & 1 = : 0, &MASK_sig(uc)); } =20 @@ -261,9 +261,8 @@ int cpu_signal_handler(int host_signum, void *pinfo, #endif =20 pc =3D PC_sig(uc); - return handle_cpu_signal(pc, (unsigned long)info->si_addr, - TRAP_sig(uc) =3D=3D 0xe ? - (ERROR_sig(uc) >> 1) & 1 : 0, + return handle_cpu_signal(pc, info, + TRAP_sig(uc) =3D=3D 0xe ? (ERROR_sig(uc) >> 1= ) & 1 : 0, &MASK_sig(uc)); } =20 @@ -341,8 +340,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, is_write =3D 1; } #endif - return handle_cpu_signal(pc, (unsigned long)info->si_addr, - is_write, &uc->uc_sigmask); + return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask); } =20 #elif defined(__alpha__) @@ -372,8 +370,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, is_write =3D 1; } =20 - return handle_cpu_signal(pc, (unsigned long)info->si_addr, - is_write, &uc->uc_sigmask); + return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask); } #elif defined(__sparc__) =20 @@ -432,8 +429,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, break; } } - return handle_cpu_signal(pc, (unsigned long)info->si_addr, - is_write, sigmask); + return handle_cpu_signal(pc, info, is_write, sigmask); } =20 #elif defined(__arm__) @@ -466,9 +462,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, * later processor; on v5 we will always report this as a read). */ is_write =3D extract32(uc->uc_mcontext.error_code, 11, 1); - return handle_cpu_signal(pc, (unsigned long)info->si_addr, - is_write, - &uc->uc_sigmask); + return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask); } =20 #elif defined(__aarch64__) @@ -495,8 +489,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, vo= id *puc) /* Ignore bits 23 & 24, controlling indexing. */ || (insn & 0x3a400000) =3D=3D 0x28000000); /* C3.3.7,14-16= */ =20 - return handle_cpu_signal(pc, (uintptr_t)info->si_addr, - is_write, &uc->uc_sigmask); + return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask); } =20 #elif defined(__ia64) @@ -529,9 +522,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, vo= id *puc) default: break; } - return handle_cpu_signal(ip, (unsigned long)info->si_addr, - is_write, - (sigset_t *)&uc->uc_sigmask); + return handle_cpu_signal(ip, info, is_write, (sigset_t *)&uc->uc_sigma= sk); } =20 #elif defined(__s390__) @@ -583,8 +574,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, } break; } - return handle_cpu_signal(pc, (unsigned long)info->si_addr, - is_write, &uc->uc_sigmask); + return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask); } =20 #elif defined(__mips__) @@ -599,8 +589,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, =20 /* XXX: compute is_write */ is_write =3D 0; - return handle_cpu_signal(pc, (unsigned long)info->si_addr, - is_write, &uc->uc_sigmask); + return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask); } =20 #else --=20 2.7.4 From nobody Fri May 3 22:00:40 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1511879986441247.1320718444839; Tue, 28 Nov 2017 06:39:46 -0800 (PST) Received: from localhost ([::1]:38303 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJh31-0007sn-Py for importer@patchew.org; Tue, 28 Nov 2017 09:39:43 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44045) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJgzP-0004ay-7M for qemu-devel@nongnu.org; Tue, 28 Nov 2017 09:36:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eJgzJ-0003rd-4Y for qemu-devel@nongnu.org; Tue, 28 Nov 2017 09:35:59 -0500 Received: from orth.archaic.org.uk ([2001:8b0:1d0::2]:38580) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eJgzI-0003lI-SZ for qemu-devel@nongnu.org; Tue, 28 Nov 2017 09:35:53 -0500 Received: from pm215 by orth.archaic.org.uk with local (Exim 4.89) (envelope-from ) id 1eJgyu-00020q-2p; Tue, 28 Nov 2017 14:35:28 +0000 From: Peter Maydell To: qemu-devel@nongnu.org Date: Tue, 28 Nov 2017 14:35:25 +0000 Message-Id: <1511879725-9576-3-git-send-email-peter.maydell@linaro.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1511879725-9576-1-git-send-email-peter.maydell@linaro.org> References: <1511879725-9576-1-git-send-email-peter.maydell@linaro.org> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:8b0:1d0::2 Subject: [Qemu-devel] [PATCH 2/2] page_unprotect(): handle calls to pages that are PAGE_WRITE X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paolo Bonzini , Riku Voipio , Richard Henderson , Laurent Vivier , patches@linaro.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" If multiple guest threads in user-mode emulation write to a page which QEMU has marked read-only because of cached TCG translations, the threads can race in page_unprotect: * threads A & B both try to do a write to a page with code in it at the same time (ie which we've made non-writeable, so SEGV) * they race into the signal handler with this faulting address * thread A happens to get to page_unprotect() first and takes the mmap lock, so thread B sits waiting for it to be done * A then finds the page, marks it PAGE_WRITE and mprotect()s it writable * A can then continue OK (returns from signal handler to retry the memory access) * ...but when B gets the mmap lock it finds that the page is already PAGE_WRITE, and so it exits page_unprotect() via the "not due to protected translation" code path, and wrongly delivers the signal to the guest rather than just retrying the access In particular, this meant that trying to run 'javac' in user-mode emulation would fail with a spurious guest SIGSEGV. Handle this by making page_unprotect() assume that a call for a page which is already PAGE_WRITE is due to a race of this sort and return a "fault handled" indication. Since this would cause an infinite loop if we ever called page_unprotect() for some other kind of fault than "write failed due to bad access permissions", tighten the condition in handle_cpu_signal() to check the signal number and si_code, and add a comment so that if somebody does ever find themselves debugging an infinite loop of faults they have some clue about why. (The trick for identifying the correct setting for current_tb_invalidated for thread B (needed to handle the precise-SMC case) is due to Richard Henderson. Paolo Bonzini suggested just relying on si_code rather than trying anything more complicated.) Signed-off-by: Peter Maydell --- accel/tcg/translate-all.c | 50 +++++++++++++++++++++++++++++--------------= ---- accel/tcg/user-exec.c | 13 +++++++++++- 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index e7f0329..96e68d5 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -2182,29 +2182,41 @@ int page_unprotect(target_ulong address, uintptr_t = pc) =20 /* if the page was really writable, then we change its protection back to writable */ - if ((p->flags & PAGE_WRITE_ORG) && !(p->flags & PAGE_WRITE)) { - host_start =3D address & qemu_host_page_mask; - host_end =3D host_start + qemu_host_page_size; - - prot =3D 0; + if (p->flags & PAGE_WRITE_ORG) { current_tb_invalidated =3D false; - for (addr =3D host_start ; addr < host_end ; addr +=3D TARGET_PAGE= _SIZE) { - p =3D page_find(addr >> TARGET_PAGE_BITS); - p->flags |=3D PAGE_WRITE; - prot |=3D p->flags; - - /* and since the content will be modified, we must invalidate - the corresponding translated code. */ - current_tb_invalidated |=3D tb_invalidate_phys_page(addr, pc); -#ifdef CONFIG_USER_ONLY - if (DEBUG_TB_CHECK_GATE) { - tb_invalidate_check(addr); + if (p->flags & PAGE_WRITE) { + /* If the page is actually marked WRITE then assume this is be= cause + * this thread raced with another one which got here first and + * set the page to PAGE_WRITE and did the TB invalidate for us. + */ +#ifdef TARGET_HAS_PRECISE_SMC + TranslationBlock *current_tb =3D tb_find_pc(pc); + if (current_tb) { + current_tb_invalidated =3D tb_cflags(current_tb) & CF_INVA= LID; } #endif + } else { + host_start =3D address & qemu_host_page_mask; + host_end =3D host_start + qemu_host_page_size; + + prot =3D 0; + for (addr =3D host_start; addr < host_end; addr +=3D TARGET_PA= GE_SIZE) { + p =3D page_find(addr >> TARGET_PAGE_BITS); + p->flags |=3D PAGE_WRITE; + prot |=3D p->flags; + + /* and since the content will be modified, we must invalid= ate + the corresponding translated code. */ + current_tb_invalidated |=3D tb_invalidate_phys_page(addr, = pc); +#ifdef CONFIG_USER_ONLY + if (DEBUG_TB_CHECK_GATE) { + tb_invalidate_check(addr); + } +#endif + } + mprotect((void *)g2h(host_start), qemu_host_page_size, + prot & PAGE_BITS); } - mprotect((void *)g2h(host_start), qemu_host_page_size, - prot & PAGE_BITS); - mmap_unlock(); /* If current TB was invalidated return to main loop */ return current_tb_invalidated ? 2 : 1; diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c index e8f26ff..c973752 100644 --- a/accel/tcg/user-exec.c +++ b/accel/tcg/user-exec.c @@ -104,7 +104,18 @@ static inline int handle_cpu_signal(uintptr_t pc, sigi= nfo_t *info, pc, address, is_write, *(unsigned long *)old_set); #endif /* XXX: locking issue */ - if (is_write && h2g_valid(address)) { + /* Note that it is important that we don't call page_unprotect() unless + * this is really a "write to nonwriteable page" fault, because + * page_unprotect() assumes that if it is called for an access to + * a page that's writeable this means we had two threads racing and + * another thread got there first and already made the page writeable; + * so we will retry the access. If we were to call page_unprotect() + * for some other kind of fault that should really be passed to the + * guest, we'd end up in an infinite loop of retrying the faulting + * access. + */ + if (is_write && info->si_signo =3D=3D SIGSEGV && info->si_code =3D=3D = SEGV_ACCERR && + h2g_valid(address)) { switch (page_unprotect(h2g(address), pc)) { case 0: /* Fault not caused by a page marked unwritable to protect --=20 2.7.4