* [PATCH 1/3] linux-user: sigaltstack lock_user_struct missing unlock
2026-03-21 13:56 [PATCH 0/3] user: recursive signal delivery fix and test Nicholas Piggin
@ 2026-03-21 13:56 ` Nicholas Piggin
2026-03-21 13:56 ` [PATCH 2/3] bsd-user, linux-user: signal: recursive signal delivery fix Nicholas Piggin
2026-03-21 13:56 ` [PATCH 3/3] tests/tcg: add recursive signal delivery tests Nicholas Piggin
2 siblings, 0 replies; 5+ messages in thread
From: Nicholas Piggin @ 2026-03-21 13:56 UTC (permalink / raw)
To: qemu-devel
Cc: Nicholas Piggin, Warner Losh, Kyle Evans, Laurent Vivier,
Pierrick Bouvier, Alex Bennée, Peter Maydell
lock_user_struct is missing a corresponding unlock.
(bsd-user does correctly unlock in the equivalent code.)
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
linux-user/signal.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 804096bd44..e4b8b28bfe 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -292,11 +292,10 @@ void target_save_altstack(target_stack_t *uss, CPUArchState *env)
__put_user(ts->sigaltstack_used.ss_size, &uss->ss_size);
}
-abi_long target_restore_altstack(target_stack_t *uss, CPUArchState *env)
+abi_long target_restore_altstack(target_stack_t *ss, CPUArchState *env)
{
TaskState *ts = get_task_state(thread_cpu);
size_t minstacksize = TARGET_MINSIGSTKSZ;
- target_stack_t ss;
#if defined(TARGET_PPC64)
/* ELF V2 for PPC64 has a 4K minimum stack size for signal handlers */
@@ -306,33 +305,29 @@ abi_long target_restore_altstack(target_stack_t *uss, CPUArchState *env)
}
#endif
- __get_user(ss.ss_sp, &uss->ss_sp);
- __get_user(ss.ss_size, &uss->ss_size);
- __get_user(ss.ss_flags, &uss->ss_flags);
-
if (on_sig_stack(get_sp_from_cpustate(env))) {
return -TARGET_EPERM;
}
- switch (ss.ss_flags) {
+ switch (ss->ss_flags) {
default:
return -TARGET_EINVAL;
case TARGET_SS_DISABLE:
- ss.ss_size = 0;
- ss.ss_sp = 0;
+ ss->ss_sp = 0;
+ ss->ss_size = 0;
break;
case TARGET_SS_ONSTACK:
case 0:
- if (ss.ss_size < minstacksize) {
+ if (ss->ss_size < minstacksize) {
return -TARGET_ENOMEM;
}
break;
}
- ts->sigaltstack_used.ss_sp = ss.ss_sp;
- ts->sigaltstack_used.ss_size = ss.ss_size;
+ ts->sigaltstack_used.ss_sp = ss->ss_sp;
+ ts->sigaltstack_used.ss_size = ss->ss_size;
return 0;
}
@@ -1140,11 +1135,17 @@ abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr,
if (uss_addr) {
target_stack_t *uss;
+ target_stack_t ss;
if (!lock_user_struct(VERIFY_READ, uss, uss_addr, 1)) {
goto out;
}
- ret = target_restore_altstack(uss, env);
+ __get_user(ss.ss_sp, &uss->ss_sp);
+ __get_user(ss.ss_size, &uss->ss_size);
+ __get_user(ss.ss_flags, &uss->ss_flags);
+ unlock_user_struct(uss, uss_addr, 0);
+
+ ret = target_restore_altstack(&ss, env);
if (ret) {
goto out;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] bsd-user, linux-user: signal: recursive signal delivery fix
2026-03-21 13:56 [PATCH 0/3] user: recursive signal delivery fix and test Nicholas Piggin
2026-03-21 13:56 ` [PATCH 1/3] linux-user: sigaltstack lock_user_struct missing unlock Nicholas Piggin
@ 2026-03-21 13:56 ` Nicholas Piggin
2026-03-21 15:51 ` Warner Losh
2026-03-21 13:56 ` [PATCH 3/3] tests/tcg: add recursive signal delivery tests Nicholas Piggin
2 siblings, 1 reply; 5+ messages in thread
From: Nicholas Piggin @ 2026-03-21 13:56 UTC (permalink / raw)
To: qemu-devel
Cc: Nicholas Piggin, Warner Losh, Kyle Evans, Laurent Vivier,
Pierrick Bouvier, Alex Bennée, Peter Maydell
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/3] tests/tcg: add recursive signal delivery tests
2026-03-21 13:56 [PATCH 0/3] user: recursive signal delivery fix and test Nicholas Piggin
2026-03-21 13:56 ` [PATCH 1/3] linux-user: sigaltstack lock_user_struct missing unlock Nicholas Piggin
2026-03-21 13:56 ` [PATCH 2/3] bsd-user, linux-user: signal: recursive signal delivery fix Nicholas Piggin
@ 2026-03-21 13:56 ` Nicholas Piggin
2 siblings, 0 replies; 5+ messages in thread
From: Nicholas Piggin @ 2026-03-21 13:56 UTC (permalink / raw)
To: qemu-devel
Cc: Nicholas Piggin, Warner Losh, Kyle Evans, Laurent Vivier,
Pierrick Bouvier, Alex Bennée, Peter Maydell
Add tests which exercise a synchronous signal being raised in
synchronous and asynchronous signal delivery paths. It does this by
delivering them with an altstack that is made inaccessible, then
catching the badframe SIGSEGV with a handler on the regular stack.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
tests/tcg/multiarch/badsig.c | 226 +++++++++++++++++++++++++++++++++++
1 file changed, 226 insertions(+)
create mode 100644 tests/tcg/multiarch/badsig.c
diff --git a/tests/tcg/multiarch/badsig.c b/tests/tcg/multiarch/badsig.c
new file mode 100644
index 0000000000..0f346f4147
--- /dev/null
+++ b/tests/tcg/multiarch/badsig.c
@@ -0,0 +1,226 @@
+/*
+ * linux-user "badframe" signal handling tests.
+ *
+ * Copyright (c) 2026 Tenstorrent USA, Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * Test "badframe" signal handling paths, which force a
+ * SIGSEGV signal from the signal handler setup code,
+ * which tests the recursive signal "restart_scan" logic
+ * in process_pending_signals().
+ */
+
+#include <stdarg.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <assert.h>
+#include <errno.h>
+#include <pthread.h>
+#include <string.h>
+#include <signal.h>
+#include <setjmp.h>
+#include <time.h>
+#include <sys/time.h>
+#include <sys/mman.h>
+
+#undef DEBUG
+#ifdef DEBUG
+#define dprintf(...) printf(__VA_ARGS__)
+#else
+#define dprintf(...)
+#endif
+
+static void error1(const char *filename, int line, const char *fmt, ...)
+{
+ va_list ap;
+ va_start(ap, fmt);
+ fprintf(stderr, "%s:%d: ", filename, line);
+ vfprintf(stderr, fmt, ap);
+ fprintf(stderr, "\n");
+ va_end(ap);
+ exit(1);
+}
+
+static int __chk_error(const char *filename, int line, int ret)
+{
+ if (ret < 0) {
+ error1(filename, line, "%m (ret=%d, errno=%d/%s)",
+ ret, errno, strerror(errno));
+ }
+ return ret;
+}
+
+#define error(fmt, ...) error1(__FILE__, __LINE__, fmt, ## __VA_ARGS__)
+
+#define chk_error(ret) __chk_error(__FILE__, __LINE__, (ret))
+
+static bool do_siglongjmp;
+static sigjmp_buf current_sigjmp_buf;
+
+static volatile int total_alarm_count;
+
+static void sig_alarm(int sig, siginfo_t *info, void *puc)
+{
+ if (sig != SIGRTMIN) {
+ error("unexpected signal");
+ }
+ dprintf("SIGRTMIN\n");
+ total_alarm_count++;
+}
+
+static volatile int total_segv_count;
+
+static void sig_segv(int sig, siginfo_t *info, void *puc)
+{
+ if (sig != SIGSEGV) {
+ error("unexpected signal");
+ }
+ dprintf("SIGSEGV\n");
+ total_segv_count++;
+ if (do_siglongjmp) {
+ dprintf("siglongjmp()\n");
+ siglongjmp(current_sigjmp_buf, 1);
+ }
+}
+
+static volatile int total_trap_count;
+
+static void sig_trap(int sig, siginfo_t *info, void *puc)
+{
+ if (sig == SIGTRAP) {
+ dprintf("SIGTRAP\n");
+ } else if (sig == SIGILL) {
+ dprintf("SIGILL\n");
+ } else if (sig == SIGABRT) {
+ dprintf("SIGABRT\n");
+ } else {
+ error("unexpected signal");
+ }
+ total_trap_count++;
+ if (do_siglongjmp) {
+ dprintf("siglongjmp()\n");
+ siglongjmp(current_sigjmp_buf, 1);
+ }
+}
+
+static void test_signals(void)
+{
+ struct sigaction act;
+ struct itimerspec it;
+ timer_t tid;
+ struct sigevent sev;
+ stack_t ss;
+ void *mem;
+
+ /* Set up SEGV handler */
+ act.sa_sigaction = sig_segv;
+ sigemptyset(&act.sa_mask);
+ act.sa_flags = SA_SIGINFO;
+ chk_error(sigaction(SIGSEGV, &act, NULL));
+
+ /* Set up an altstack */
+ mem = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ if (mem == MAP_FAILED) {
+ fprintf(stderr, "out of memory");
+ exit(EXIT_FAILURE);
+ }
+
+ ss.ss_sp = mem;
+ ss.ss_flags = 0;
+ ss.ss_size = SIGSTKSZ;
+ chk_error(sigaltstack(&ss, NULL));
+
+ /* Async signal test */
+
+ /* Set up RTMIN handler on alt stack */
+ act.sa_sigaction = sig_alarm;
+ sigemptyset(&act.sa_mask);
+ act.sa_flags = SA_SIGINFO | SA_ONSTACK;
+ chk_error(sigaction(SIGRTMIN, &act, NULL));
+
+ /* Create POSIX timer */
+ sev.sigev_notify = SIGEV_SIGNAL;
+ sev.sigev_signo = SIGRTMIN;
+ sev.sigev_value.sival_ptr = &tid;
+ chk_error(timer_create(CLOCK_REALTIME, &sev, &tid));
+
+ it.it_interval.tv_sec = 0;
+ it.it_interval.tv_nsec = 1000000;
+ it.it_value.tv_sec = 0;
+ it.it_value.tv_nsec = 1000000;
+ chk_error(timer_settime(tid, 0, &it, NULL));
+
+ while (total_alarm_count == 0) {
+ usleep(1000);
+ }
+ total_alarm_count = 0;
+
+ chk_error(timer_delete(tid));
+
+ assert(total_segv_count == 0);
+
+ /* Make the alt stack bad */
+ chk_error(mprotect(mem, SIGSTKSZ, PROT_NONE));
+
+ chk_error(timer_create(CLOCK_REALTIME, &sev, &tid));
+ chk_error(timer_settime(tid, 0, &it, NULL));
+
+ while (total_segv_count == 0) {
+ usleep(1000);
+ }
+ total_segv_count = 0;
+
+ chk_error(timer_delete(tid));
+
+ assert(total_alarm_count == 0);
+
+ /* Make the alt stack good */
+ chk_error(mprotect(mem, SIGSTKSZ, PROT_READ | PROT_WRITE));
+
+ /* Bad sync signal test */
+
+ /* Set up SIGILL/TRAP/ABRT handler on alt stack */
+ act.sa_sigaction = sig_trap;
+ sigemptyset(&act.sa_mask);
+ act.sa_flags = SA_SIGINFO | SA_ONSTACK;
+ chk_error(sigaction(SIGTRAP, &act, NULL));
+ chk_error(sigaction(SIGILL, &act, NULL));
+ chk_error(sigaction(SIGABRT, &act, NULL));
+
+ if (sigsetjmp(current_sigjmp_buf, 1) == 0) {
+ do_siglongjmp = true;
+ /* Cause a synchronous signal */
+ dprintf("__builtin_trap()\n");
+ __builtin_trap();
+ assert(0);
+ }
+ do_siglongjmp = false;
+ assert(total_trap_count == 1);
+ total_trap_count = 0;
+ assert(total_segv_count == 0);
+
+ /* Make the alt stack bad */
+ chk_error(mprotect(mem, SIGSTKSZ, PROT_NONE));
+
+ if (sigsetjmp(current_sigjmp_buf, 1) == 0) {
+ do_siglongjmp = true;
+ /* Cause a synchronous signal */
+ dprintf("__builtin_trap()\n");
+ __builtin_trap();
+ assert(0);
+ }
+ do_siglongjmp = false;
+ assert(total_segv_count == 1);
+ total_segv_count = 0;
+ assert(total_trap_count == 0);
+}
+
+int main(int argc, char **argv)
+{
+ test_signals();
+ return 0;
+}
--
2.51.0
^ permalink raw reply related [flat|nested] 5+ messages in thread