diff options
author | Claudio Jeker <claudio@cvs.openbsd.org> | 2023-09-13 14:25:50 +0000 |
---|---|---|
committer | Claudio Jeker <claudio@cvs.openbsd.org> | 2023-09-13 14:25:50 +0000 |
commit | 9376548c529d5f72807db994abf8871dc4711704 (patch) | |
tree | 8dab82b878fed6a672dd05876d2aa34561d55304 /sys/kern/kern_exit.c | |
parent | ec6e0abf986358b29bf6bb527abcf7583474a64d (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.c | 27 |
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); |