[Qemu-devel] [PATCH 23/34] linux-user: Queue synchronous signals separately

[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, however this still
risks delaying a asynchronous signal until the synchronous signal handler
returns rather than handling the signal on another thread.

Signed-off-by: Timothy Edward Baldwin <address@hidden>

Conflicts:
        linux-user/signal.c
---
 linux-user/qemu.h   |  1 +
 linux-user/signal.c | 63 +++++++++++++++++++++++++++++------------------------
 2 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 5f43692..331c48f 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -123,6 +123,7 @@ typedef struct TaskState {
     struct image_info *info;
     struct linux_binprm *bprm;
 
+    struct emulated_sigtable sync_signal;
     struct emulated_sigtable sigtab[TARGET_NSIG];
     sigset_t signal_mask;
 
diff --git a/linux-user/signal.c b/linux-user/signal.c
index aa08c72..15bd082 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -491,22 +491,14 @@ 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;
 
 #if defined(DEBUG_SIGNAL)
     fprintf(stderr, "queue_signal: sig=%d\n",
             sig);
 #endif
-    k = &ts->sigtab[sig - 1];
-
-    /* we queue exactly one signal */
-    if (k->pending) {
-        return 0;
-    }
 
-    k->info = *info;
-    k->pending = 1;
-    /* signal that a new signal is pending */
+    ts->sync_signal.info = *info;
+    ts->sync_signal.pending = sig;
     ts->signal_pending = 1;
     return 1; /* indicates that the signal was queued */
 }
@@ -519,8 +511,12 @@ 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;
+    struct emulated_sigtable *k;
 
     /* the CPU emulator uses some host signals to detect exceptions,
        we forward to it some signals */
@@ -546,15 +542,18 @@ static void host_signal_handler(int host_signum, 
siginfo_t *info,
 #endif
 
     host_to_target_siginfo_noswap(&tinfo, info);
-    if (queue_signal(env, sig, &tinfo) == 1) {
-        /* Block host signals until target signal handler entered */
-        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;
 
-        /* interrupt the virtual CPU as soon as possible */
-        cpu_exit(thread_cpu);
-    }
+    /* Block host signals until target signal handler entered */
+    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);
 }
 
 /* do_sigaltstack() returns target values and errnos. */
@@ -5590,12 +5589,26 @@ restart:
     sigfillset(&set);
     sigprocmask(SIG_SETMASK, &set, 0);
 
+    if (ts->sync_signal.pending) {
+        k = &ts->sync_signal;
+        sig = k->pending;
+
+        /* Synchronous signals are forced,
+         * see force_sig_info() and callers in Linux */
+        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;
+        }
+
+       goto handle_signal;
+    }
+
  next_signal:
     k = ts->sigtab;
     for(sig = 1; sig <= TARGET_NSIG; sig++) {
-        if (k->pending && (
-                    !sigismember(&ts->signal_mask, 
target_to_host_signal_table[sig])
-                    || sig == TARGET_SIGSEGV)) {
+        if (k->pending &&
+             !sigismember(&ts->signal_mask, target_to_host_signal_table[sig])) 
{
             goto handle_signal;
         }
         k++;
@@ -5625,14 +5638,6 @@ restart:
         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) {
-- 
2.1.4