[PATCH 2/3] bsd-user, linux-user: signal: recursive signal delivery fix

Nicholas Piggin posted 3 patches 1 day ago
Maintainers: Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>, Laurent Vivier <laurent@vivier.eu>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, "Alex Bennée" <alex.bennee@linaro.org>
[PATCH 2/3] bsd-user, linux-user: signal: recursive signal delivery fix
Posted by Nicholas Piggin 1 day ago
Synchronous signals must accommodate a synchronous signal being
raised during delivery, as asynchronous ones do. For example
badframe errors during delivery will cause SIGSEGV to be raised.

Without this fix, cpu_loop() runs process_pending_signals() which
delivers the first synchronous signal (e.g., SIGILL) which fails
to set the handler and forces SIGSEGV, but that is not picked up.
process_pending_signals() returns. Then cpu_loop() runs cpu_exec()
again, which attempts to execute the same instruction, another
SIGILL.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 bsd-user/signal.c   | 10 ++++++----
 linux-user/signal.c |  9 ++++++---
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/bsd-user/signal.c b/bsd-user/signal.c
index dadcc037dc..3e5e41e1b1 100644
--- a/bsd-user/signal.c
+++ b/bsd-user/signal.c
@@ -998,7 +998,12 @@ void process_pending_signals(CPUArchState *env)
                 sigdelset(&ts->signal_mask, target_to_host_signal(sig));
                 sigact_table[sig - 1]._sa_handler = TARGET_SIG_DFL;
             }
+            /*
+             * Restart scan from the beginning, as handle_pending_signal
+             * might have resulted in a new synchronous signal (eg SIGSEGV).
+             */
             handle_pending_signal(env, sig, &ts->sync_signal);
+            goto restart_scan;
         }
 
         k = ts->sigtab;
@@ -1008,10 +1013,7 @@ void process_pending_signals(CPUArchState *env)
             if (k->pending &&
                 !sigismember(blocked_set, target_to_host_signal(sig))) {
                 handle_pending_signal(env, sig, k);
-                /*
-                 * Restart scan from the beginning, as handle_pending_signal
-                 * might have resulted in a new synchronous signal (eg SIGSEGV).
-                 */
+                /* Restart scan, explained above. */
                 goto restart_scan;
             }
         }
diff --git a/linux-user/signal.c b/linux-user/signal.c
index e4b8b28bfe..9d43e080ce 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1385,6 +1385,11 @@ void process_pending_signals(CPUArchState *cpu_env)
             }
 
             handle_pending_signal(cpu_env, sig, &ts->sync_signal);
+            /*
+             * Restart scan from the beginning, as handle_pending_signal
+             * might have resulted in a new synchronous signal (eg SIGSEGV).
+             */
+            goto restart_scan;
         }
 
         for (sig = 1; sig <= TARGET_NSIG; sig++) {
@@ -1395,9 +1400,7 @@ void process_pending_signals(CPUArchState *cpu_env)
                 (!sigismember(blocked_set,
                               target_to_host_signal_table[sig]))) {
                 handle_pending_signal(cpu_env, sig, &ts->sigtab[sig - 1]);
-                /* Restart scan from the beginning, as handle_pending_signal
-                 * might have resulted in a new synchronous signal (eg SIGSEGV).
-                 */
+                /* Restart scan, explained above. */
                 goto restart_scan;
             }
         }
-- 
2.51.0
Re: [PATCH 2/3] bsd-user, linux-user: signal: recursive signal delivery fix
Posted by Warner Losh 22 hours ago
On Sat, Mar 21, 2026 at 7:56 AM Nicholas Piggin <npiggin@gmail.com> wrote:

> Synchronous signals must accommodate a synchronous signal being
> raised during delivery, as asynchronous ones do. For example
> badframe errors during delivery will cause SIGSEGV to be raised.
>
> Without this fix, cpu_loop() runs process_pending_signals() which
> delivers the first synchronous signal (e.g., SIGILL) which fails
> to set the handler and forces SIGSEGV, but that is not picked up.
> process_pending_signals() returns. Then cpu_loop() runs cpu_exec()
> again, which attempts to execute the same instruction, another
> SIGILL.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  bsd-user/signal.c   | 10 ++++++----
>  linux-user/signal.c |  9 ++++++---
>  2 files changed, 12 insertions(+), 7 deletions(-)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

Interesting edge case...

Warner


> diff --git a/bsd-user/signal.c b/bsd-user/signal.c
> index dadcc037dc..3e5e41e1b1 100644
> --- a/bsd-user/signal.c
> +++ b/bsd-user/signal.c
> @@ -998,7 +998,12 @@ void process_pending_signals(CPUArchState *env)
>                  sigdelset(&ts->signal_mask, target_to_host_signal(sig));
>                  sigact_table[sig - 1]._sa_handler = TARGET_SIG_DFL;
>              }
> +            /*
> +             * Restart scan from the beginning, as handle_pending_signal
> +             * might have resulted in a new synchronous signal (eg
> SIGSEGV).
> +             */
>              handle_pending_signal(env, sig, &ts->sync_signal);
> +            goto restart_scan;
>          }
>
>          k = ts->sigtab;
> @@ -1008,10 +1013,7 @@ void process_pending_signals(CPUArchState *env)
>              if (k->pending &&
>                  !sigismember(blocked_set, target_to_host_signal(sig))) {
>                  handle_pending_signal(env, sig, k);
> -                /*
> -                 * Restart scan from the beginning, as
> handle_pending_signal
> -                 * might have resulted in a new synchronous signal (eg
> SIGSEGV).
> -                 */
> +                /* Restart scan, explained above. */
>                  goto restart_scan;
>              }
>          }
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index e4b8b28bfe..9d43e080ce 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -1385,6 +1385,11 @@ void process_pending_signals(CPUArchState *cpu_env)
>              }
>
>              handle_pending_signal(cpu_env, sig, &ts->sync_signal);
> +            /*
> +             * Restart scan from the beginning, as handle_pending_signal
> +             * might have resulted in a new synchronous signal (eg
> SIGSEGV).
> +             */
> +            goto restart_scan;
>          }
>
>          for (sig = 1; sig <= TARGET_NSIG; sig++) {
> @@ -1395,9 +1400,7 @@ void process_pending_signals(CPUArchState *cpu_env)
>                  (!sigismember(blocked_set,
>                                target_to_host_signal_table[sig]))) {
>                  handle_pending_signal(cpu_env, sig, &ts->sigtab[sig - 1]);
> -                /* Restart scan from the beginning, as
> handle_pending_signal
> -                 * might have resulted in a new synchronous signal (eg
> SIGSEGV).
> -                 */
> +                /* Restart scan, explained above. */
>                  goto restart_scan;
>              }
>          }
> --
> 2.51.0
>
>