[PATCH v3 0/4] linux-user: fix use of SIGRTMIN

[PATCH v3 0/4] linux-user: fix use of SIGRTMIN

[Laurent Vivier]

This series fixes the problem of the first real-time signals already
in use by the glibc that are not available for the target glibc.

Instead of reverting the first and last real-time signals we rely on
the value provided by the glibc (SIGRTMIN) to know the first available
signal and we map all the signals from this value to SIGRTMAX on top
of TARGET_SIGRTMIN. So the consequence is we have less available signals
in the target (generally 2) but all seems fine as at least 30 signals are
still available.

This has been tested with Go (golang 1.10.1 linux/arm64, bionic) on x86_64
fedora 31. We can avoid the failure in this case allowing the unsupported
signals when we don't provide the "act" parameters to sigaction, only the
"oldact" one. I have also run the LTP suite with several target and debian
based distros.

v3: use trace_event_get_state_backends()
    update comments
    Add R-b

v2: tested with golang 1.12.10 linux/arm64, eoan)
    Ignore unsupported signals rather than returning an error
    replace i, j by target_sig, host_sig

Laurent Vivier (4):
  linux-user: add missing TARGET_SIGRTMIN for hppa
  linux-user: cleanup signal.c
  linux-user: fix TARGET_NSIG and _NSIG uses
  linux-user: fix use of SIGRTMIN

 linux-user/hppa/target_signal.h |   1 +
 linux-user/signal.c             | 134 ++++++++++++++++++++++++--------
 linux-user/trace-events         |   3 +
 3 files changed, 106 insertions(+), 32 deletions(-)

-- 
2.24.1

[PATCH v3 1/4] linux-user: add missing TARGET_SIGRTMIN for hppa

[Laurent Vivier]

This signal is defined for all other targets and we will need it later

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
[pm: that this was actually an ABI change in the hppa kernel (at kernel
version 3.17, kernel commit 1f25df2eff5b25f52c139d). Before that
SIGRTMIN was 37...
All our other HPPA TARGET_SIG* values are for the updated
ABI following that commit, so using 32 for SIGRTMIN is
the right thing for us.]
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---

Notes:
    v3: Add Rb and comment from Peter

 linux-user/hppa/target_signal.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/linux-user/hppa/target_signal.h b/linux-user/hppa/target_signal.h
index ba159ff8d006..c2a0102ed73d 100644
--- a/linux-user/hppa/target_signal.h
+++ b/linux-user/hppa/target_signal.h
@@ -34,6 +34,7 @@
 #define TARGET_SIGURG          29
 #define TARGET_SIGXFSZ         30
 #define TARGET_SIGSYS          31
+#define TARGET_SIGRTMIN        32
 
 #define TARGET_SIG_BLOCK       0
 #define TARGET_SIG_UNBLOCK     1
-- 
2.24.1

[PATCH v3 2/4] linux-user: cleanup signal.c

[Laurent Vivier]

No functional changes. Prepare the field for future fixes.

Remove memset(.., 0, ...) that is useless on a static array

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---

Notes:
    v2: replace i, j by target_sig, host_sig

 linux-user/signal.c | 48 ++++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 5ca6d62b15d3..246315571c09 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -66,12 +66,6 @@ static uint8_t host_to_target_signal_table[_NSIG] = {
     [SIGPWR] = TARGET_SIGPWR,
     [SIGSYS] = TARGET_SIGSYS,
     /* next signals stay the same */
-    /* Nasty hack: Reverse SIGRTMIN and SIGRTMAX to avoid overlap with
-       host libpthread signals.  This assumes no one actually uses SIGRTMAX :-/
-       To fix this properly we need to do manual signal delivery multiplexed
-       over a single host signal.  */
-    [__SIGRTMIN] = __SIGRTMAX,
-    [__SIGRTMAX] = __SIGRTMIN,
 };
 static uint8_t target_to_host_signal_table[_NSIG];
 
@@ -480,31 +474,45 @@ static int core_dump_signal(int sig)
     }
 }
 
+static void signal_table_init(void)
+{
+    int host_sig, target_sig;
+
+    /*
+     * Nasty hack: Reverse SIGRTMIN and SIGRTMAX to avoid overlap with
+     * host libpthread signals.  This assumes no one actually uses SIGRTMAX :-/
+     * To fix this properly we need to do manual signal delivery multiplexed
+     * over a single host signal.
+     */
+    host_to_target_signal_table[__SIGRTMIN] = __SIGRTMAX;
+    host_to_target_signal_table[__SIGRTMAX] = __SIGRTMIN;
+
+    /* generate signal conversion tables */
+    for (host_sig = 1; host_sig < _NSIG; host_sig++) {
+        if (host_to_target_signal_table[host_sig] == 0) {
+            host_to_target_signal_table[host_sig] = host_sig;
+        }
+    }
+    for (host_sig = 1; host_sig < _NSIG; host_sig++) {
+        target_sig = host_to_target_signal_table[host_sig];
+        target_to_host_signal_table[target_sig] = host_sig;
+    }
+}
+
 void signal_init(void)
 {
     TaskState *ts = (TaskState *)thread_cpu->opaque;
     struct sigaction act;
     struct sigaction oact;
-    int i, j;
+    int i;
     int host_sig;
 
-    /* generate signal conversion tables */
-    for(i = 1; i < _NSIG; i++) {
-        if (host_to_target_signal_table[i] == 0)
-            host_to_target_signal_table[i] = i;
-    }
-    for(i = 1; i < _NSIG; i++) {
-        j = host_to_target_signal_table[i];
-        target_to_host_signal_table[j] = i;
-    }
+    /* initialize signal conversion tables */
+    signal_table_init();
 
     /* Set the signal mask from the host mask. */
     sigprocmask(0, 0, &ts->signal_mask);
 
-    /* set all host signal handlers. ALL signals are blocked during
-       the handlers to serialize them. */
-    memset(sigact_table, 0, sizeof(sigact_table));
-
     sigfillset(&act.sa_mask);
     act.sa_flags = SA_SIGINFO;
     act.sa_sigaction = host_signal_handler;
-- 
2.24.1

[PATCH v3 3/4] linux-user: fix TARGET_NSIG and _NSIG uses

[Laurent Vivier]

Valid signal numbers are between 1 (SIGHUP) and SIGRTMAX.

System includes define _NSIG to SIGRTMAX + 1, but
QEMU (like kernel) defines TARGET_NSIG to TARGET_SIGRTMAX.

Fix all the checks involving the signal range.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---

Notes:
    v2: replace i, j by target_sig, host_sig

 linux-user/signal.c | 52 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 15 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 246315571c09..c1e664f97a7c 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -30,6 +30,15 @@ static struct target_sigaction sigact_table[TARGET_NSIG];
 static void host_signal_handler(int host_signum, siginfo_t *info,
                                 void *puc);
 
+
+/*
+ * System includes define _NSIG as SIGRTMAX + 1,
+ * but qemu (like the kernel) defines TARGET_NSIG as TARGET_SIGRTMAX
+ * and the first signal is SIGHUP defined as 1
+ * Signal number 0 is reserved for use as kill(pid, 0), to test whether
+ * a process exists without sending it a signal.
+ */
+QEMU_BUILD_BUG_ON(__SIGRTMAX + 1 != _NSIG);
 static uint8_t host_to_target_signal_table[_NSIG] = {
     [SIGHUP] = TARGET_SIGHUP,
     [SIGINT] = TARGET_SIGINT,
@@ -67,19 +76,24 @@ static uint8_t host_to_target_signal_table[_NSIG] = {
     [SIGSYS] = TARGET_SIGSYS,
     /* next signals stay the same */
 };
-static uint8_t target_to_host_signal_table[_NSIG];
 
+static uint8_t target_to_host_signal_table[TARGET_NSIG + 1];
+
+/* valid sig is between 1 and _NSIG - 1 */
 int host_to_target_signal(int sig)
 {
-    if (sig < 0 || sig >= _NSIG)
+    if (sig < 1 || sig >= _NSIG) {
         return sig;
+    }
     return host_to_target_signal_table[sig];
 }
 
+/* valid sig is between 1 and TARGET_NSIG */
 int target_to_host_signal(int sig)
 {
-    if (sig < 0 || sig >= _NSIG)
+    if (sig < 1 || sig > TARGET_NSIG) {
         return sig;
+    }
     return target_to_host_signal_table[sig];
 }
 
@@ -100,11 +114,15 @@ static inline int target_sigismember(const target_sigset_t *set, int signum)
 void host_to_target_sigset_internal(target_sigset_t *d,
                                     const sigset_t *s)
 {
-    int i;
+    int host_sig, target_sig;
     target_sigemptyset(d);
-    for (i = 1; i <= TARGET_NSIG; i++) {
-        if (sigismember(s, i)) {
-            target_sigaddset(d, host_to_target_signal(i));
+    for (host_sig = 1; host_sig < _NSIG; host_sig++) {
+        target_sig = host_to_target_signal(host_sig);
+        if (target_sig < 1 || target_sig > TARGET_NSIG) {
+            continue;
+        }
+        if (sigismember(s, host_sig)) {
+            target_sigaddset(d, target_sig);
         }
     }
 }
@@ -122,11 +140,15 @@ void host_to_target_sigset(target_sigset_t *d, const sigset_t *s)
 void target_to_host_sigset_internal(sigset_t *d,
                                     const target_sigset_t *s)
 {
-    int i;
+    int host_sig, target_sig;
     sigemptyset(d);
-    for (i = 1; i <= TARGET_NSIG; i++) {
-        if (target_sigismember(s, i)) {
-            sigaddset(d, target_to_host_signal(i));
+    for (target_sig = 1; target_sig <= TARGET_NSIG; target_sig++) {
+        host_sig = target_to_host_signal(target_sig);
+        if (host_sig < 1 || host_sig >= _NSIG) {
+            continue;
+        }
+        if (target_sigismember(s, target_sig)) {
+            sigaddset(d, host_sig);
         }
     }
 }
@@ -492,10 +514,10 @@ static void signal_table_init(void)
         if (host_to_target_signal_table[host_sig] == 0) {
             host_to_target_signal_table[host_sig] = host_sig;
         }
-    }
-    for (host_sig = 1; host_sig < _NSIG; host_sig++) {
         target_sig = host_to_target_signal_table[host_sig];
-        target_to_host_signal_table[target_sig] = host_sig;
+        if (target_sig <= TARGET_NSIG) {
+            target_to_host_signal_table[target_sig] = host_sig;
+        }
     }
 }
 
@@ -518,7 +540,7 @@ void signal_init(void)
     act.sa_sigaction = host_signal_handler;
     for(i = 1; i <= TARGET_NSIG; i++) {
 #ifdef TARGET_GPROF
-        if (i == SIGPROF) {
+        if (i == TARGET_SIGPROF) {
             continue;
         }
 #endif
-- 
2.24.1

[PATCH v3 4/4] linux-user: fix use of SIGRTMIN

[Laurent Vivier]

Some RT signals can be in use by glibc,
it's why SIGRTMIN (34) is generally greater than __SIGRTMIN (32).

So SIGRTMIN cannot be mapped to TARGET_SIGRTMIN.

Instead of swapping only SIGRTMIN and SIGRTMAX, map all the
range [TARGET_SIGRTMIN ... TARGET_SIGRTMAX - X] to
      [__SIGRTMIN + X ... SIGRTMAX ]
(SIGRTMIN is __SIGRTMIN + X).

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Taylor Simson <tsimpson@quicinc.com>
---

Notes:
    v3: use trace_event_get_state_backends()
        update comments
    
    v2: ignore error when target sig <= TARGET_NSIG but host sig > SIGRTMAX
        replace i, j by target_sig, host_sig
        update signal_table_init() trace message

 linux-user/signal.c     | 50 ++++++++++++++++++++++++++++++++++++-----
 linux-user/trace-events |  3 +++
 2 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index c1e664f97a7c..046159dd0c5b 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -498,18 +498,30 @@ static int core_dump_signal(int sig)
 
 static void signal_table_init(void)
 {
-    int host_sig, target_sig;
+    int host_sig, target_sig, count;
 
     /*
-     * Nasty hack: Reverse SIGRTMIN and SIGRTMAX to avoid overlap with
-     * host libpthread signals.  This assumes no one actually uses SIGRTMAX :-/
+     * Signals are supported starting from TARGET_SIGRTMIN and going up
+     * until we run out of host realtime signals.
+     * glibc at least uses only the lower 2 rt signals and probably
+     * nobody's using the upper ones.
+     * it's why SIGRTMIN (34) is generally greater than __SIGRTMIN (32)
      * To fix this properly we need to do manual signal delivery multiplexed
      * over a single host signal.
+     * Attempts for configure "missing" signals via sigaction will be
+     * silently ignored.
      */
-    host_to_target_signal_table[__SIGRTMIN] = __SIGRTMAX;
-    host_to_target_signal_table[__SIGRTMAX] = __SIGRTMIN;
+    for (host_sig = SIGRTMIN; host_sig <= SIGRTMAX; host_sig++) {
+        target_sig = host_sig - SIGRTMIN + TARGET_SIGRTMIN;
+        if (target_sig <= TARGET_NSIG) {
+            host_to_target_signal_table[host_sig] = target_sig;
+        }
+    }
 
     /* generate signal conversion tables */
+    for (target_sig = 1; target_sig <= TARGET_NSIG; target_sig++) {
+        target_to_host_signal_table[target_sig] = _NSIG; /* poison */
+    }
     for (host_sig = 1; host_sig < _NSIG; host_sig++) {
         if (host_to_target_signal_table[host_sig] == 0) {
             host_to_target_signal_table[host_sig] = host_sig;
@@ -519,6 +531,15 @@ static void signal_table_init(void)
             target_to_host_signal_table[target_sig] = host_sig;
         }
     }
+
+    if (trace_event_get_state_backends(TRACE_SIGNAL_TABLE_INIT)) {
+        for (target_sig = 1, count = 0; target_sig <= TARGET_NSIG; target_sig++) {
+            if (target_to_host_signal_table[target_sig] == _NSIG) {
+                count++;
+            }
+        }
+        trace_signal_table_init(count);
+    }
 }
 
 void signal_init(void)
@@ -817,6 +838,8 @@ int do_sigaction(int sig, const struct target_sigaction *act,
     int host_sig;
     int ret = 0;
 
+    trace_signal_do_sigaction_guest(sig, TARGET_NSIG);
+
     if (sig < 1 || sig > TARGET_NSIG || sig == TARGET_SIGKILL || sig == TARGET_SIGSTOP) {
         return -TARGET_EINVAL;
     }
@@ -847,6 +870,23 @@ int do_sigaction(int sig, const struct target_sigaction *act,
 
         /* we update the host linux signal state */
         host_sig = target_to_host_signal(sig);
+        trace_signal_do_sigaction_host(host_sig, TARGET_NSIG);
+        if (host_sig > SIGRTMAX) {
+            /* we don't have enough host signals to map all target signals */
+            qemu_log_mask(LOG_UNIMP, "Unsupported target signal #%d, ignored\n",
+                          sig);
+            /*
+             * we don't return an error here because some programs try to
+             * register an handler for all possible rt signals even if they
+             * don't need it.
+             * An error here can abort them whereas there can be no problem
+             * to not have the signal available later.
+             * This is the case for golang,
+             *   See https://github.com/golang/go/issues/33746
+             * So we silently ignore the error.
+             */
+            return 0;
+        }
         if (host_sig != SIGSEGV && host_sig != SIGBUS) {
             sigfillset(&act1.sa_mask);
             act1.sa_flags = SA_SIGINFO;
diff --git a/linux-user/trace-events b/linux-user/trace-events
index f6de1b8befc0..0296133daeb6 100644
--- a/linux-user/trace-events
+++ b/linux-user/trace-events
@@ -1,6 +1,9 @@
 # See docs/devel/tracing.txt for syntax documentation.
 
 # signal.c
+signal_table_init(int i) "number of unavailable signals: %d"
+signal_do_sigaction_guest(int sig, int max) "target signal %d (MAX %d)"
+signal_do_sigaction_host(int sig, int max) "host signal %d (MAX %d)"
 # */signal.c
 user_setup_frame(void *env, uint64_t frame_addr) "env=%p frame_addr=0x%"PRIx64
 user_setup_rt_frame(void *env, uint64_t frame_addr) "env=%p frame_addr=0x%"PRIx64
-- 
2.24.1

Re: [PATCH v3 4/4] linux-user: fix use of SIGRTMIN

[Peter Maydell]

On Wed, 12 Feb 2020 at 12:57, Laurent Vivier <laurent@vivier.eu> wrote:
>
> Some RT signals can be in use by glibc,
> it's why SIGRTMIN (34) is generally greater than __SIGRTMIN (32).
>
> So SIGRTMIN cannot be mapped to TARGET_SIGRTMIN.
>
> Instead of swapping only SIGRTMIN and SIGRTMAX, map all the
> range [TARGET_SIGRTMIN ... TARGET_SIGRTMAX - X] to
>       [__SIGRTMIN + X ... SIGRTMAX ]
> (SIGRTMIN is __SIGRTMIN + X).
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> Reviewed-by: Taylor Simson <tsimpson@quicinc.com>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

RE: [PATCH v3 0/4] linux-user: fix use of SIGRTMIN

[Taylor Simpson]

Tested-by: Taylor Simpson <tsimpson@quicinc.com>

> -----Original Message-----
> From: Laurent Vivier <laurent@vivier.eu>
> Sent: Wednesday, February 12, 2020 6:57 AM
> To: qemu-devel@nongnu.org
> Cc: Aleksandar Markovic <aleksandar.markovic@rt-rk.com>; Laurent Vivier
> <laurent@vivier.eu>; Matus Kysel <mkysel@tachyum.com>;
> milos.stojanovic@rt-rk.com; Riku Voipio <riku.voipio@iki.fi>; Josh Kunz
> <jkz@google.com>; Taylor Simpson <tsimpson@quicinc.com>; Peter Maydell
> <peter.maydell@linaro.org>; Marlies Ruck <marlies.ruck@gmail.com>
> Subject: [PATCH v3 0/4] linux-user: fix use of SIGRTMIN
>
> This series fixes the problem of the first real-time signals already
> in use by the glibc that are not available for the target glibc.
>
> Instead of reverting the first and last real-time signals we rely on
> the value provided by the glibc (SIGRTMIN) to know the first available
> signal and we map all the signals from this value to SIGRTMAX on top
> of TARGET_SIGRTMIN. So the consequence is we have less available signals
> in the target (generally 2) but all seems fine as at least 30 signals are
> still available.
>
> This has been tested with Go (golang 1.10.1 linux/arm64, bionic) on x86_64
> fedora 31. We can avoid the failure in this case allowing the unsupported
> signals when we don't provide the "act" parameters to sigaction, only the
> "oldact" one. I have also run the LTP suite with several target and debian
> based distros.
>
> v3: use trace_event_get_state_backends()
>     update comments
>     Add R-b
>
> v2: tested with golang 1.12.10 linux/arm64, eoan)
>     Ignore unsupported signals rather than returning an error
>     replace i, j by target_sig, host_sig
>
> Laurent Vivier (4):
>   linux-user: add missing TARGET_SIGRTMIN for hppa
>   linux-user: cleanup signal.c
>   linux-user: fix TARGET_NSIG and _NSIG uses
>   linux-user: fix use of SIGRTMIN
>
>  linux-user/hppa/target_signal.h |   1 +
>  linux-user/signal.c             | 134 ++++++++++++++++++++++++--------
>  linux-user/trace-events         |   3 +
>  3 files changed, 106 insertions(+), 32 deletions(-)
>
> --
> 2.24.1