Prevent a recursion inside wakeup(9) when scheduler tracepoints are enabled.
authormpi <mpi@openbsd.org>
Sat, 30 Mar 2024 13:33:20 +0000 (13:33 +0000)
committermpi <mpi@openbsd.org>
Sat, 30 Mar 2024 13:33:20 +0000 (13:33 +0000)
Tracepoints like "sched:enqueue" and "sched:unsleep" were called from inside
the loop iterating over sleeping threads as part of wakeup_proc().  When such
tracepoints were enabled they could result in another wakeup(9) possibly
corrupting the sleepqueue.

Rewrite wakeup(9) in two stages, first dequeue threads from the sleepqueue then
call setrunnable() and possible tracepoints for each of them.

This requires moving unsleep() outside of setrunnable() because it messes with
the sleepqueue.

ok claudio@

sys/dev/pci/drm/drm_linux.c
sys/kern/kern_sig.c
sys/kern/kern_synch.c
sys/kern/sched_bsd.c
sys/kern/sys_process.c
sys/sys/proc.h

index efe4250..95947ae 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: drm_linux.c,v 1.111 2024/03/20 02:42:17 jsg Exp $     */
+/*     $OpenBSD: drm_linux.c,v 1.112 2024/03/30 13:33:20 mpi Exp $     */
 /*
  * Copyright (c) 2013 Jonathan Gray <jsg@openbsd.org>
  * Copyright (c) 2015, 2016 Mark Kettenis <kettenis@openbsd.org>
@@ -162,7 +162,7 @@ wake_up_process(struct proc *p)
        int s, rv;
 
        SCHED_LOCK(s);
-       rv = wakeup_proc(p, NULL, 0);
+       rv = wakeup_proc(p, 0);
        SCHED_UNLOCK(s);
        return rv;
 }
index ebdc602..5205c05 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kern_sig.c,v 1.322 2024/02/25 00:07:13 deraadt Exp $  */
+/*     $OpenBSD: kern_sig.c,v 1.323 2024/03/30 13:33:20 mpi Exp $      */
 /*     $NetBSD: kern_sig.c,v 1.54 1996/04/22 01:38:32 christos Exp $   */
 
 /*
@@ -1205,6 +1205,7 @@ runfast:
        if (p->p_usrpri > PUSER)
                p->p_usrpri = PUSER;
 run:
+       unsleep(p);
        setrunnable(p);
 out:
        /* finally adjust siglist */
@@ -2109,6 +2110,7 @@ single_thread_set(struct proc *p, int flags)
                if (q->p_flag & P_WEXIT) {
                        if (mode == SINGLE_EXIT) {
                                if (q->p_stat == SSTOP) {
+                                       unsleep(q);
                                        setrunnable(q);
                                        atomic_inc_int(&pr->ps_singlecount);
                                }
@@ -2130,12 +2132,14 @@ single_thread_set(struct proc *p, int flags)
                                        break;
                                }
                                /* need to unwind or exit, so wake it */
+                               unsleep(q);
                                setrunnable(q);
                        }
                        atomic_inc_int(&pr->ps_singlecount);
                        break;
                case SSTOP:
                        if (mode == SINGLE_EXIT) {
+                               unsleep(q);
                                setrunnable(q);
                                atomic_inc_int(&pr->ps_singlecount);
                        }
index b4f0c27..f6d04bc 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kern_synch.c,v 1.200 2023/09/13 14:25:49 claudio Exp $        */
+/*     $OpenBSD: kern_synch.c,v 1.201 2024/03/30 13:33:20 mpi Exp $    */
 /*     $NetBSD: kern_synch.c,v 1.37 1996/04/22 01:38:37 christos Exp $ */
 
 /*
@@ -467,25 +467,23 @@ sleep_signal_check(void)
 }
 
 int
-wakeup_proc(struct proc *p, const volatile void *chan, int flags)
+wakeup_proc(struct proc *p, int flags)
 {
        int awakened = 0;
 
        SCHED_ASSERT_LOCKED();
 
-       if (p->p_wchan != NULL &&
-          ((chan == NULL) || (p->p_wchan == chan))) {
+       if (p->p_wchan != NULL) {
                awakened = 1;
                if (flags)
                        atomic_setbits_int(&p->p_flag, flags);
-               if (p->p_stat == SSLEEP)
-                       setrunnable(p);
-               else if (p->p_stat == SSTOP)
-                       unsleep(p);
 #ifdef DIAGNOSTIC
-               else
-                       panic("wakeup: p_stat is %d", (int)p->p_stat);
+               if (p->p_stat != SSLEEP && p->p_stat != SSTOP)
+                       panic("thread %d p_stat is %d", p->p_tid, p->p_stat);
 #endif
+               unsleep(p);
+               if (p->p_stat == SSLEEP)
+                       setrunnable(p);
        }
 
        return awakened;
@@ -505,7 +503,7 @@ endtsleep(void *arg)
        int s;
 
        SCHED_LOCK(s);
-       wakeup_proc(p, NULL, P_TIMEOUT);
+       wakeup_proc(p, P_TIMEOUT);
        SCHED_UNLOCK(s);
 }
 
@@ -531,21 +529,35 @@ unsleep(struct proc *p)
 void
 wakeup_n(const volatile void *ident, int n)
 {
-       struct slpque *qp;
+       struct slpque *qp, wakeq;
        struct proc *p;
        struct proc *pnext;
        int s;
 
+       TAILQ_INIT(&wakeq);
+
        SCHED_LOCK(s);
        qp = &slpque[LOOKUP(ident)];
        for (p = TAILQ_FIRST(qp); p != NULL && n != 0; p = pnext) {
                pnext = TAILQ_NEXT(p, p_runq);
 #ifdef DIAGNOSTIC
                if (p->p_stat != SSLEEP && p->p_stat != SSTOP)
-                       panic("wakeup: p_stat is %d", (int)p->p_stat);
+                       panic("thread %d p_stat is %d", p->p_tid, p->p_stat);
 #endif
-               if (wakeup_proc(p, ident, 0))
+               KASSERT(p->p_wchan != NULL);
+               if (p->p_wchan == ident) {
+                       TAILQ_REMOVE(qp, p, p_runq);
+                       p->p_wchan = NULL;
+                       TAILQ_INSERT_TAIL(&wakeq, p, p_runq);
                        --n;
+               }
+       }
+       while ((p = TAILQ_FIRST(&wakeq))) {
+               TAILQ_REMOVE(&wakeq, p, p_runq);
+               TRACEPOINT(sched, unsleep, p->p_tid + THREAD_PID_OFFSET,
+                   p->p_p->ps_pid);
+               if (p->p_stat == SSLEEP)
+                       setrunnable(p);
        }
        SCHED_UNLOCK(s);
 }
index 89d58c6..25b221c 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: sched_bsd.c,v 1.90 2024/01/24 19:23:38 cheloha Exp $  */
+/*     $OpenBSD: sched_bsd.c,v 1.91 2024/03/30 13:33:20 mpi Exp $      */
 /*     $NetBSD: kern_synch.c,v 1.37 1996/04/22 01:38:37 christos Exp $ */
 
 /*-
@@ -500,12 +500,10 @@ setrunnable(struct proc *p)
                if ((pr->ps_flags & PS_TRACED) != 0 && pr->ps_xsig != 0)
                        atomic_setbits_int(&p->p_siglist, sigmask(pr->ps_xsig));
                prio = p->p_usrpri;
-               unsleep(p);
                setrunqueue(NULL, p, prio);
                break;
        case SSLEEP:
                prio = p->p_slppri;
-               unsleep(p);             /* e.g. when sending signals */
 
                /* if not yet asleep, don't add to runqueue */
                if (ISSET(p->p_flag, P_WSLEEP))
index 9ca68a9..a1c1e70 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: sys_process.c,v 1.95 2023/11/21 14:00:13 bluhm Exp $  */
+/*     $OpenBSD: sys_process.c,v 1.96 2024/03/30 13:33:20 mpi Exp $    */
 /*     $NetBSD: sys_process.c,v 1.55 1996/05/15 06:17:47 tls Exp $     */
 
 /*-
@@ -493,6 +493,7 @@ ptrace_ctrl(struct proc *p, int req, pid_t pid, caddr_t addr, int data)
                if (t->p_stat == SSTOP) {
                        tr->ps_xsig = data;
                        SCHED_LOCK(s);
+                       unsleep(t);
                        setrunnable(t);
                        SCHED_UNLOCK(s);
                } else {
index 122f38a..c98ea17 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: proc.h,v 1.356 2024/02/03 18:51:58 beck Exp $ */
+/*     $OpenBSD: proc.h,v 1.357 2024/03/30 13:33:21 mpi Exp $  */
 /*     $NetBSD: proc.h,v 1.44 1996/04/22 01:23:21 christos Exp $       */
 
 /*-
@@ -555,7 +555,7 @@ void        procinit(void);
 void   setpriority(struct proc *, uint32_t, uint8_t);
 void   setrunnable(struct proc *);
 void   endtsleep(void *);
-int    wakeup_proc(struct proc *, const volatile void *, int);
+int    wakeup_proc(struct proc *, int);
 void   unsleep(struct proc *);
 void   reaper(void *);
 __dead void exit1(struct proc *, int, int, int);