[PULL 1/1] linux-user/sparc: Take pending signals in sparc64_set_context()

[Helge Deller <deller@kernel.org>]

From: Peter Maydell <peter.maydell@linaro.org>

Every callsite of block_signals() checks its return value, except
the one in sparc64_set_context(). Generally you need to check,
because the standard pattern is:

    if (block_signals()) {
        return -QEMU_ERESTARTSYS;
    }
    /* do some blocking syscall */

and we need to take any pending signal before we do the blocking
operation, not afterwards.

The use in sparc64_set_context() doesn't do this.  It doesn't have to
because the operations it is doing aren't blocking, so it won't get
into "we didn't take the signal that we should have" races that
blocking syscalls do.  But it does make this way of updating the
signal mask inconsistent with how we do it in do_sigprocmask().
do_sigprocmask() does the usual "return -QEMU_ERESTARTSYS", so a
pending signal that was not blocked by the old signal mask and which
will be blocked by the new mask we're about to install will be taken
before we change the mask.  sparc64_set_context() doesn't check the
return value, so we won't take that pending signal.  That's not
wrong, because it just means the signal lost the race with the
executing code.  But it seems clearer to behave the same way as
do_sigprocmask(), not differently.

Make sparc64_set_context() check the return value of block_signals()
and return early if there's a pending signal to take.  We don't need
to return a separate return code to indicate this because the main
loop handles it the same either way.

Coverity CID: 1660058

Fixes: e0f0ce88eb9 ("linux-user/sparc: call block_signals() before set_sigmask() in setcontext")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Matt Turner <mattst88@gmail.com>
Signed-off-by: Helge Deller <deller@gmx.de>
---
 linux-user/sparc/cpu_loop.c | 10 ++++++++++
 linux-user/sparc/signal.c   | 30 +++++++++++++++++++++---------
 2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c
index 0aacda9448..eaf388c167 100644
--- a/linux-user/sparc/cpu_loop.c
+++ b/linux-user/sparc/cpu_loop.c
@@ -282,6 +282,16 @@ void cpu_loop (CPUSPARCState *env)
             break;
         case TT_TRAP + 0x6f:
             flush_windows(env);
+            /*
+             * If we have a pending signal, sparc64_set_context() may
+             * return early without changing register state (like a
+             * syscall that returns -QEMU_ERESTARTSYS). We will then
+             * take the pending signal via process_pending_signals()
+             * below and eventually re-execute the trap. We don't need
+             * the function to return a different value for the
+             * "restart" case because this main loop code does the
+             * same thing in both cases.
+             */
             sparc64_set_context(env);
             break;
 #endif
diff --git a/linux-user/sparc/signal.c b/linux-user/sparc/signal.c
index ba692c3123..4baf983ed8 100644
--- a/linux-user/sparc/signal.c
+++ b/linux-user/sparc/signal.c
@@ -594,6 +594,27 @@ void sparc64_set_context(CPUSPARCState *env)
     unsigned int i;
     unsigned char fenab;
 
+    if (env->regwptr[WREG_O1]) {
+        /*
+         * We're going to set the signal mask; we need to call
+         * block_signals() first, so that process_pending_signals() is
+         * guaranteed to run after the mask change.  Without this, a
+         * guest signal that is pending-and-blocked at setcontext time
+         * is left undelivered even after its mask bit is cleared,
+         * because signal_pending stays 0 and the post-trap
+         * process_pending_signals() loop never enters.
+         *
+         * If block_signals() returns true, this means we have a
+         * pending signal that we could take now; we return early so
+         * the cpu_loop takes that signal.  Eventually the guest will
+         * re-execute the trap insn and we'll come back here to have
+         * another go at set_context. This is the same way that
+         * do_sigprocmask() handles setting the signal mask.
+         */
+        if (block_signals()) {
+            return;
+        }
+    }
     ucp_addr = env->regwptr[WREG_O0];
     if (!lock_user_struct(VERIFY_READ, ucp, ucp_addr, 1)) {
         goto do_sigsegv;
@@ -619,15 +640,6 @@ void sparc64_set_context(CPUSPARCState *env)
             }
         }
         target_to_host_sigset_internal(&set, &target_set);
-        /*
-         * set_sigmask() requires the caller to have first called
-         * block_signals() so that process_pending_signals() is guaranteed
-         * to run after the mask change.  Without this, a guest signal that
-         * is pending-and-blocked at setcontext time is left undelivered
-         * even after its mask bit is cleared, because signal_pending stays
-         * 0 and the post-trap process_pending_signals() loop never enters.
-         */
-        block_signals();
         set_sigmask(&set);
     }
     env->pc = pc;
-- 
2.54.0