From de92f022b000940561918115be1183c1ac9f2e18 Mon Sep 17 00:00:00 2001 From: claudio Date: Tue, 19 Sep 2023 10:43:33 +0000 Subject: [PATCH] Before coredump or in pledge_fail use SINGLE_UNWIND to stop all threads. SINGLE_UNWIND unwinds to the kernel boundary. On the other hand SINGLE_SUSPEND will sleep inside tsleep(9) and other sleep functions. Since the code will exit1() very soon after it is better to already unwind. Now one could argue that for coredumps all threads should stop asap to get a clean dump. Using SINGLE_UNWIND the sleep will fail with ERESTART and no copyout should happen in that case. This is a bit of a workaround since SINGLE_SUSPEND has a small race where single_thread_wait() returns before all threads are really stopped. When SINGLE_EXIT is called quickly after this can blow up inside sleep_finish. Reported-by: syzbot+3ef066fcfaf991f2ac2c@syzkaller.appspotmail.com OK mpi@ kettenis@ --- sys/kern/kern_pledge.c | 4 ++-- sys/kern/kern_sig.c | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/sys/kern/kern_pledge.c b/sys/kern/kern_pledge.c index 8dcf97b7a33..7f2446e6318 100644 --- a/sys/kern/kern_pledge.c +++ b/sys/kern/kern_pledge.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_pledge.c,v 1.307 2023/08/20 15:13:43 visa Exp $ */ +/* $OpenBSD: kern_pledge.c,v 1.308 2023/09/19 10:43:33 claudio Exp $ */ /* * Copyright (c) 2015 Nicholas Marriott @@ -580,7 +580,7 @@ pledge_fail(struct proc *p, int error, uint64_t code) /* Stop threads immediately, because this process is suspect */ if (P_HASSIBLING(p)) - single_thread_set(p, SINGLE_SUSPEND, 1); + single_thread_set(p, SINGLE_UNWIND, 1); /* Send uncatchable SIGABRT for coredump */ sigabort(p); diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index 06b6a9e27d0..2bffc053225 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_sig.c,v 1.317 2023/09/13 14:25:49 claudio Exp $ */ +/* $OpenBSD: kern_sig.c,v 1.318 2023/09/19 10:43:33 claudio Exp $ */ /* $NetBSD: kern_sig.c,v 1.54 1996/04/22 01:38:32 christos Exp $ */ /* @@ -1559,7 +1559,7 @@ sigexit(struct proc *p, int signum) /* if there are other threads, pause them */ if (P_HASSIBLING(p)) - single_thread_set(p, SINGLE_SUSPEND, 1); + single_thread_set(p, SINGLE_UNWIND, 1); if (coredump(p) == 0) signum |= WCOREFLAG; @@ -2059,10 +2059,10 @@ single_thread_check(struct proc *p, int deep) /* * Stop other threads in the process. The mode controls how and * where the other threads should stop: - * - SINGLE_SUSPEND: stop wherever they are, will later either be told to exit - * (by setting to SINGLE_EXIT) or be released (via single_thread_clear()) + * - SINGLE_SUSPEND: stop wherever they are, will later be released (via + * single_thread_clear()) * - SINGLE_UNWIND: just unwind to kernel boundary, will be told to exit - * or released as with SINGLE_SUSPEND + * (by setting to SINGLE_EXIT) or released as with SINGLE_SUSPEND * - SINGLE_EXIT: unwind to kernel boundary and exit */ int -- 2.20.1