From patchwork Fri May 27 14:51:53 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 9138617 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2104B6075A for ; Fri, 27 May 2016 15:00:26 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 136D227B90 for ; Fri, 27 May 2016 15:00:26 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0854928096; Fri, 27 May 2016 15:00:26 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 51AB227B90 for ; Fri, 27 May 2016 15:00:25 +0000 (UTC) Received: from localhost ([::1]:46471 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b6JFQ-0007aV-9j for patchwork-qemu-devel@patchwork.kernel.org; Fri, 27 May 2016 11:00:24 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49544) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b6J9W-0002PA-PT for qemu-devel@nongnu.org; Fri, 27 May 2016 10:54:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b6J9U-0005n2-DY for qemu-devel@nongnu.org; Fri, 27 May 2016 10:54:17 -0400 Received: from orth.archaic.org.uk ([2001:8b0:1d0::2]:57388) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b6J9U-0005mS-2F for qemu-devel@nongnu.org; Fri, 27 May 2016 10:54:16 -0400 Received: from pm215 by orth.archaic.org.uk with local (Exim 4.84_2) (envelope-from ) id 1b6J7P-0004ks-IA; Fri, 27 May 2016 15:52:07 +0100 From: Peter Maydell To: qemu-devel@nongnu.org Date: Fri, 27 May 2016 15:51:53 +0100 Message-Id: <1464360721-14359-12-git-send-email-peter.maydell@linaro.org> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1464360721-14359-1-git-send-email-peter.maydell@linaro.org> References: <1464360721-14359-1-git-send-email-peter.maydell@linaro.org> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:8b0:1d0::2 Subject: [Qemu-devel] [PATCH v2 11/19] linux-user: Queue synchronous signals separately 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: Riku Voipio , Timothy Edward Baldwin , patches@linaro.org Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP From: Timothy E Baldwin If a synchronous signal and an asynchronous signal arrive near simultaneously, and the signal number of the asynchronous signal is lower than that of the synchronous signal the the handler for the asynchronous would be called first, and then the handler for the synchronous signal would be called within or after the first handler with an incorrect context. This is fixed by queuing synchronous signals separately. Note that this does risk delaying a asynchronous signal until the synchronous signal handler returns rather than handling the signal on another thread, but this seems unlikely to cause problems for real guest programs and is unavoidable unless we could guarantee to roll back and reexecute whatever guest instruction caused the synchronous signal (which would be a bit odd if we've already logged its execution, for instance, and would require careful analysis of all guest CPUs to check it was possible in all cases). Signed-off-by: Timothy Edward Baldwin Message-id: 1441497448-32489-24-git-send-email-T.E.Baldwin99@members.leeds.ac.uk [PMM: added a comment] Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- linux-user/qemu.h | 1 + linux-user/signal.c | 74 ++++++++++++++++++++++++++++++----------------------- 2 files changed, 43 insertions(+), 32 deletions(-) diff --git a/linux-user/qemu.h b/linux-user/qemu.h index b201f90..6bd7b32 100644 --- a/linux-user/qemu.h +++ b/linux-user/qemu.h @@ -119,6 +119,7 @@ typedef struct TaskState { struct image_info *info; struct linux_binprm *bprm; + struct emulated_sigtable sync_signal; struct emulated_sigtable sigtab[TARGET_NSIG]; /* This thread's signal mask, as requested by the guest program. * The actual signal mask of this thread may differ: diff --git a/linux-user/signal.c b/linux-user/signal.c index 5db1c0b..f489028 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -502,18 +502,11 @@ int queue_signal(CPUArchState *env, int sig, target_siginfo_t *info) { CPUState *cpu = ENV_GET_CPU(env); TaskState *ts = cpu->opaque; - struct emulated_sigtable *k; trace_user_queue_signal(env, sig); - k = &ts->sigtab[sig - 1]; - - /* we queue exactly one signal */ - if (k->pending) { - return 0; - } - k->info = *info; - k->pending = 1; + ts->sync_signal.info = *info; + ts->sync_signal.pending = sig; /* signal that a new signal is pending */ atomic_set(&ts->signal_pending, 1); return 1; /* indicates that the signal was queued */ @@ -530,9 +523,13 @@ static void host_signal_handler(int host_signum, siginfo_t *info, void *puc) { CPUArchState *env = thread_cpu->env_ptr; + CPUState *cpu = ENV_GET_CPU(env); + TaskState *ts = cpu->opaque; + int sig; target_siginfo_t tinfo; ucontext_t *uc = puc; + struct emulated_sigtable *k; /* the CPU emulator uses some host signals to detect exceptions, we forward to it some signals */ @@ -551,20 +548,23 @@ static void host_signal_handler(int host_signum, siginfo_t *info, rewind_if_in_safe_syscall(puc); host_to_target_siginfo_noswap(&tinfo, info); - if (queue_signal(env, sig, &tinfo) == 1) { - /* Block host signals until target signal handler entered. We - * can't block SIGSEGV or SIGBUS while we're executing guest - * code in case the guest code provokes one in the window between - * now and it getting out to the main loop. Signals will be - * unblocked again in process_pending_signals(). - */ - sigfillset(&uc->uc_sigmask); - sigdelset(&uc->uc_sigmask, SIGSEGV); - sigdelset(&uc->uc_sigmask, SIGBUS); + k = &ts->sigtab[sig - 1]; + k->info = tinfo; + k->pending = sig; + ts->signal_pending = 1; + + /* Block host signals until target signal handler entered. We + * can't block SIGSEGV or SIGBUS while we're executing guest + * code in case the guest code provokes one in the window between + * now and it getting out to the main loop. Signals will be + * unblocked again in process_pending_signals(). + */ + sigfillset(&uc->uc_sigmask); + sigdelset(&uc->uc_sigmask, SIGSEGV); + sigdelset(&uc->uc_sigmask, SIGBUS); - /* interrupt the virtual CPU as soon as possible */ - cpu_exit(thread_cpu); - } + /* interrupt the virtual CPU as soon as possible */ + cpu_exit(thread_cpu); } /* do_sigaltstack() returns target values and errnos. */ @@ -5761,14 +5761,6 @@ static void handle_pending_signal(CPUArchState *cpu_env, int sig) handler = sa->_sa_handler; } - if (sig == TARGET_SIGSEGV && sigismember(&ts->signal_mask, SIGSEGV)) { - /* Guest has blocked SIGSEGV but we got one anyway. Assume this - * is a forced SIGSEGV (ie one the kernel handles via force_sig_info - * because it got a real MMU fault), and treat as if default handler. - */ - handler = TARGET_SIG_DFL; - } - if (handler == TARGET_SIG_DFL) { /* default handler : ignore some signal. The other are job control or fatal */ if (sig == TARGET_SIGTSTP || sig == TARGET_SIGTTIN || sig == TARGET_SIGTTOU) { @@ -5841,14 +5833,32 @@ void process_pending_signals(CPUArchState *cpu_env) sigfillset(&set); sigprocmask(SIG_SETMASK, &set, 0); + sig = ts->sync_signal.pending; + if (sig) { + /* Synchronous signals are forced, + * see force_sig_info() and callers in Linux + * Note that not all of our queue_signal() calls in QEMU correspond + * to force_sig_info() calls in Linux (some are send_sig_info()). + * However it seems like a kernel bug to me to allow the process + * to block a synchronous signal since it could then just end up + * looping round and round indefinitely. + */ + if (sigismember(&ts->signal_mask, target_to_host_signal_table[sig]) + || sigact_table[sig - 1]._sa_handler == TARGET_SIG_IGN) { + sigdelset(&ts->signal_mask, target_to_host_signal_table[sig]); + sigact_table[sig - 1]._sa_handler = TARGET_SIG_DFL; + } + + handle_pending_signal(cpu_env, sig); + } + for (sig = 1; sig <= TARGET_NSIG; sig++) { blocked_set = ts->in_sigsuspend ? &ts->sigsuspend_mask : &ts->signal_mask; if (ts->sigtab[sig - 1].pending && (!sigismember(blocked_set, - target_to_host_signal_table[sig]) - || sig == TARGET_SIGSEGV)) { + target_to_host_signal_table[sig]))) { handle_pending_signal(cpu_env, sig); /* Restart scan from the beginning */ sig = 1;