summaryrefslogtreecommitdiff
path: root/sys/kern/kern_exit.c
diff options
context:
space:
mode:
authorClaudio Jeker <claudio@cvs.openbsd.org>2023-09-13 14:25:50 +0000
committerClaudio Jeker <claudio@cvs.openbsd.org>2023-09-13 14:25:50 +0000
commit9376548c529d5f72807db994abf8871dc4711704 (patch)
tree8dab82b878fed6a672dd05876d2aa34561d55304 /sys/kern/kern_exit.c
parentec6e0abf986358b29bf6bb527abcf7583474a64d (diff)
Revert commitid: yfAefyNWibUyjkU2, ESyyH5EKxtrXGkS6 and itscfpFvJLOj8mHB;
The change to the single thread API results in crashes inside exit1() as found by Syzkaller. There seems to be a race in the exit codepath. What exactly fails is not really clear therefor revert for now. This should fix the following Syzkaller reports: Reported-by: syzbot+38efb425eada701ca8bb@syzkaller.appspotmail.com Reported-by: syzbot+ecc0e8628b3db39b5b17@syzkaller.appspotmail.com and maybe more. Reverted commits: ---------------------------- Protect ps_single, ps_singlecnt and ps_threadcnt by the process mutex. The single thread API needs to lock the process to enter single thread mode and does not need to stop the scheduler. This code changes ps_singlecount from a count down to zero to ps_singlecnt which counts up until equal to ps_threadcnt (in which case all threads are properly asleep). Tested by phessler@, OK mpi@ cheloha@ ---------------------------- Change how ps_threads and p_thr_link are locked away from using SCHED_LOCK. The per process thread list can be traversed (read) by holding either the KERNEL_LOCK or the per process ps_mtx (instead of SCHED_LOCK). Abusing the SCHED_LOCK for this makes it impossible to split up the scheduler lock into something more fine grained. Tested by phessler@, ok mpi@ ---------------------------- Fix SCHED_LOCK() leak in single_thread_set() In the (q->p_flag & P_WEXIT) branch is a continue that did not release the SCHED_LOCK. Refactor the code a bit to simplify the places SCHED_LOCK is grabbed and released. Reported-by: syzbot+ea26d351acfad3bb3f15@syzkaller.appspotmail.com OK kettenis@
Diffstat (limited to 'sys/kern/kern_exit.c')
-rw-r--r--sys/kern/kern_exit.c27
1 files changed, 9 insertions, 18 deletions
diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c
index 460833afc4f..4d7bfd5fddb 100644
--- a/sys/kern/kern_exit.c
+++ b/sys/kern/kern_exit.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: kern_exit.c,v 1.214 2023/09/08 09:06:31 claudio Exp $ */
+/* $OpenBSD: kern_exit.c,v 1.215 2023/09/13 14:25:49 claudio Exp $ */
/* $NetBSD: kern_exit.c,v 1.39 1996/04/22 01:38:25 christos Exp $ */
/*
@@ -119,7 +119,7 @@ exit1(struct proc *p, int xexit, int xsig, int flags)
struct process *pr, *qr, *nqr;
struct rusage *rup;
struct timespec ts;
- int s, wake;
+ int s;
atomic_setbits_int(&p->p_flag, P_WEXIT);
@@ -157,22 +157,14 @@ exit1(struct proc *p, int xexit, int xsig, int flags)
}
/* unlink ourselves from the active threads */
- mtx_enter(&pr->ps_mtx);
+ SCHED_LOCK(s);
TAILQ_REMOVE(&pr->ps_threads, p, p_thr_link);
- pr->ps_threadcnt--;
-
- wake = (pr->ps_single && pr->ps_singlecnt == pr->ps_threadcnt);
- mtx_leave(&pr->ps_mtx);
- if (wake)
- wakeup(&pr->ps_singlecnt);
+ SCHED_UNLOCK(s);
if ((p->p_flag & P_THREAD) == 0) {
/* main thread gotta wait because it has the pid, et al */
- mtx_enter(&pr->ps_mtx);
- while (pr->ps_threadcnt > 0)
- msleep_nsec(&pr->ps_threads, &pr->ps_mtx, PWAIT,
- "thrdeath", INFSLP);
- mtx_leave(&pr->ps_mtx);
+ while (pr->ps_threadcnt > 1)
+ tsleep_nsec(&pr->ps_threads, PWAIT, "thrdeath", INFSLP);
if (pr->ps_flags & PS_PROFIL)
stopprofclock(pr);
}
@@ -345,10 +337,9 @@ exit1(struct proc *p, int xexit, int xsig, int flags)
/* just a thread? detach it from its process */
if (p->p_flag & P_THREAD) {
/* scheduler_wait_hook(pr->ps_mainproc, p); XXX */
- mtx_enter(&pr->ps_mtx);
- if (pr->ps_threadcnt == 0)
+ if (--pr->ps_threadcnt == 1)
wakeup(&pr->ps_threads);
- mtx_leave(&pr->ps_mtx);
+ KASSERT(pr->ps_threadcnt > 0);
}
/* Release the thread's read reference of resource limit structure. */
@@ -832,7 +823,7 @@ process_zap(struct process *pr)
if (otvp)
vrele(otvp);
- KASSERT(pr->ps_threadcnt == 0);
+ KASSERT(pr->ps_threadcnt == 1);
if (pr->ps_ptstat != NULL)
free(pr->ps_ptstat, M_SUBPROC, sizeof(*pr->ps_ptstat));
pool_put(&rusage_pool, pr->ps_ru);