From 98f03f90302d1d2f1878c8451c5ddf1789eeafce Mon Sep 17 00:00:00 2001 From: Jake Burkholder Date: Sat, 23 Dec 2000 19:43:10 +0000 Subject: [PATCH] Protect proc.p_pptr and proc.p_children/p_sibling with the proctree_lock. linprocfs not locked pending response from informal maintainer. Reviewed by: jhb, -smp@ --- sys/alpha/alpha/sys_machdep.c | 4 ++++ sys/alpha/alpha/vm_machdep.c | 12 ++++++++++ sys/amd64/amd64/vm_machdep.c | 12 ++++++++++ sys/compat/svr4/svr4_misc.c | 24 ++++++++++++------- sys/fs/procfs/procfs_ctl.c | 14 +++++++++++ sys/fs/procfs/procfs_status.c | 2 ++ sys/i386/i386/vm_machdep.c | 12 ++++++++++ sys/ia64/ia64/vm_machdep.c | 12 ++++++++++ sys/kern/kern_exec.c | 2 ++ sys/kern/kern_exit.c | 40 ++++++++++++++++++------------- sys/kern/kern_fork.c | 2 ++ sys/kern/kern_kthread.c | 4 ++++ sys/kern/kern_ktrace.c | 6 +++-- sys/kern/kern_proc.c | 17 ++++++++++--- sys/kern/kern_prot.c | 12 +++++++++- sys/kern/kern_sig.c | 11 ++++++++- sys/kern/sys_process.c | 12 +++++++++- sys/miscfs/procfs/procfs_ctl.c | 14 +++++++++++ sys/miscfs/procfs/procfs_status.c | 2 ++ sys/powerpc/aim/vm_machdep.c | 12 ++++++++++ sys/powerpc/powerpc/vm_machdep.c | 12 ++++++++++ sys/sys/proc.h | 12 ++++++++++ 22 files changed, 218 insertions(+), 32 deletions(-) diff --git a/sys/alpha/alpha/sys_machdep.c b/sys/alpha/alpha/sys_machdep.c index 4e0c3c8c3838..f5de242e7d0d 100644 --- a/sys/alpha/alpha/sys_machdep.c +++ b/sys/alpha/alpha/sys_machdep.c @@ -173,6 +173,7 @@ alpha_set_uac(struct proc *p, char *args) if (error) return (error); + PROCTREE_LOCK(PT_SHARED); if (p->p_pptr) { s = splimp(); if (p->p_pptr) { @@ -182,6 +183,7 @@ alpha_set_uac(struct proc *p, char *args) error = ESRCH; splx(s); } + PROCTREE_LOCK(PT_RELEASE); return error; } @@ -192,6 +194,7 @@ alpha_get_uac(struct proc *p, char *args) unsigned long uac; error = ESRCH; + PROCTREE_LOCK(PT_SHARED); if (p->p_pptr) { s = splimp(); if (p->p_pptr) { @@ -200,5 +203,6 @@ alpha_get_uac(struct proc *p, char *args) } splx(s); } + PROCTREE_LOCK(PT_RELEASE); return error; } diff --git a/sys/alpha/alpha/vm_machdep.c b/sys/alpha/alpha/vm_machdep.c index ba5004d327ff..c60f971a114c 100644 --- a/sys/alpha/alpha/vm_machdep.c +++ b/sys/alpha/alpha/vm_machdep.c @@ -256,6 +256,18 @@ cpu_exit(p) mtx_enter(&sched_lock, MTX_SPIN); mtx_exit(&Giant, MTX_DEF | MTX_NOSWITCH); mtx_assert(&Giant, MA_NOTOWNED); + + /* + * We have to wait until after releasing all locks before + * changing p_stat. If we block on a mutex then we will be + * back at SRUN when we resume and our parent will never + * harvest us. + */ + p->p_stat = SZOMB; + + mp_fixme("assumption: p_pptr won't change at this time"); + wakeup(p->p_pptr); + cnt.v_swtch++; cpu_switch(); panic("cpu_exit"); diff --git a/sys/amd64/amd64/vm_machdep.c b/sys/amd64/amd64/vm_machdep.c index dcf4986da814..0a3bc5154748 100644 --- a/sys/amd64/amd64/vm_machdep.c +++ b/sys/amd64/amd64/vm_machdep.c @@ -259,6 +259,18 @@ cpu_exit(p) mtx_enter(&sched_lock, MTX_SPIN); mtx_exit(&Giant, MTX_DEF | MTX_NOSWITCH); mtx_assert(&Giant, MA_NOTOWNED); + + /* + * We have to wait until after releasing all locks before + * changing p_stat. If we block on a mutex then we will be + * back at SRUN when we resume and our parent will never + * harvest us. + */ + p->p_stat = SZOMB; + + mp_fixme("assumption: p_pptr won't change at this time"); + wakeup(p->p_pptr); + cnt.v_swtch++; cpu_throw(); panic("cpu_exit"); diff --git a/sys/compat/svr4/svr4_misc.c b/sys/compat/svr4/svr4_misc.c index 601d205905e1..172523275e7c 100644 --- a/sys/compat/svr4/svr4_misc.c +++ b/sys/compat/svr4/svr4_misc.c @@ -1205,6 +1205,7 @@ svr4_sys_waitsys(p, uap) loop: nfound = 0; + PROCTREE_LOCK(PT_SHARED); for (q = p->p_children.lh_first; q != 0; q = q->p_sibling.le_next) { if (SCARG(uap, id) != WAIT_ANY && q->p_pid != SCARG(uap, id) && @@ -1216,6 +1217,7 @@ svr4_sys_waitsys(p, uap) nfound++; if (q->p_stat == SZOMB && ((SCARG(uap, options) & (SVR4_WEXITED|SVR4_WTRAPPED)))) { + PROCTREE_LOCK(PT_RELEASE); *retval = 0; DPRINTF(("found %d\n", q->p_pid)); if ((error = svr4_setinfo(q, q->p_xstat, @@ -1236,14 +1238,18 @@ svr4_sys_waitsys(p, uap) * parent a SIGCHLD. The rest of the cleanup will be * done when the old parent waits on the child. */ - if ((q->p_flag & P_TRACED) && - q->p_oppid != q->p_pptr->p_pid) { - t = pfind(q->p_oppid); - proc_reparent(q, t ? t : initproc); - q->p_oppid = 0; - q->p_flag &= ~(P_TRACED | P_WAITED); - wakeup((caddr_t)q->p_pptr); - return 0; + if (q->p_flag & P_TRACED) { + PROCTREE_LOCK(PT_EXCLUSIVE); + if (q->p_oppid != q->p_pptr->p_pid) { + t = pfind(q->p_oppid); + proc_reparent(q, t ? t : initproc); + q->p_oppid = 0; + q->p_flag &= ~(P_TRACED | P_WAITED); + wakeup((caddr_t)q->p_pptr); + PROCTREE_LOCK(PT_RELEASE); + return 0; + } + PROCTREE_LOCK(PT_RELEASE); } q->p_xstat = 0; ruadd(&p->p_stats->p_cru, q->p_ru); @@ -1260,7 +1266,9 @@ svr4_sys_waitsys(p, uap) LIST_REMOVE(q, p_list); /* off zombproc */ ALLPROC_LOCK(AP_RELEASE); + PROCTREE_LOCK(PT_EXCLUSIVE); LIST_REMOVE(q, p_sibling); + PROCTREE_LOCK(PT_RELEASE); /* * Decrement the count of procs running with this uid. diff --git a/sys/fs/procfs/procfs_ctl.c b/sys/fs/procfs/procfs_ctl.c index f445572c9a3d..5d0ce44ba6f0 100644 --- a/sys/fs/procfs/procfs_ctl.c +++ b/sys/fs/procfs/procfs_ctl.c @@ -144,10 +144,12 @@ procfs_control(curp, p, op) p->p_flag |= P_TRACED; faultin(p); p->p_xstat = 0; /* XXX ? */ + PROCTREE_LOCK(PT_EXCLUSIVE); if (p->p_pptr != curp) { p->p_oppid = p->p_pptr->p_pid; proc_reparent(p, curp); } + PROCTREE_LOCK(PT_RELEASE); psignal(p, SIGSTOP); return (0); } @@ -164,12 +166,15 @@ procfs_control(curp, p, op) break; default: + PROCTREE_LOCK(PT_SHARED); mtx_enter(&sched_lock, MTX_SPIN); if (!TRACE_WAIT_P(curp, p)) { mtx_exit(&sched_lock, MTX_SPIN); + PROCTREE_LOCK(PT_RELEASE); return (EBUSY); } mtx_exit(&sched_lock, MTX_SPIN); + PROCTREE_LOCK(PT_RELEASE); } @@ -204,6 +209,7 @@ procfs_control(curp, p, op) SIGDELSET(p->p_siglist, SIGTRAP); /* give process back to original parent */ + PROCTREE_LOCK(PT_EXCLUSIVE); if (p->p_oppid != p->p_pptr->p_pid) { struct proc *pp; @@ -211,6 +217,7 @@ procfs_control(curp, p, op) if (pp) proc_reparent(p, pp); } + PROCTREE_LOCK(PT_RELEASE); p->p_oppid = 0; p->p_flag &= ~P_WAITED; /* XXX ? */ @@ -244,19 +251,23 @@ procfs_control(curp, p, op) case PROCFS_CTL_WAIT: error = 0; if (p->p_flag & P_TRACED) { + PROCTREE_LOCK(PT_SHARED); mtx_enter(&sched_lock, MTX_SPIN); while (error == 0 && (p->p_stat != SSTOP) && (p->p_flag & P_TRACED) && (p->p_pptr == curp)) { mtx_exit(&sched_lock, MTX_SPIN); + PROCTREE_LOCK(PT_RELEASE); error = tsleep((caddr_t) p, PWAIT|PCATCH, "procfsx", 0); + PROCTREE_LOCK(PT_SHARED); mtx_enter(&sched_lock, MTX_SPIN); } if (error == 0 && !TRACE_WAIT_P(curp, p)) error = EBUSY; mtx_exit(&sched_lock, MTX_SPIN); + PROCTREE_LOCK(PT_RELEASE); } else { mtx_enter(&sched_lock, MTX_SPIN); while (error == 0 && p->p_stat != SSTOP) { @@ -317,6 +328,7 @@ procfs_doctl(curp, p, pfs, uio) } else { nm = vfs_findname(signames, msg, xlen); if (nm) { + PROCTREE_LOCK(PT_SHARED); mtx_enter(&sched_lock, MTX_SPIN); if (TRACE_WAIT_P(curp, p)) { p->p_xstat = nm->nm_val; @@ -325,8 +337,10 @@ procfs_doctl(curp, p, pfs, uio) #endif setrunnable(p); mtx_exit(&sched_lock, MTX_SPIN); + PROCTREE_LOCK(PT_RELEASE); } else { mtx_exit(&sched_lock, MTX_SPIN); + PROCTREE_LOCK(PT_RELEASE); psignal(p, nm->nm_val); } error = 0; diff --git a/sys/fs/procfs/procfs_status.c b/sys/fs/procfs/procfs_status.c index b06a0af1aa5e..d7dd108be9fa 100644 --- a/sys/fs/procfs/procfs_status.c +++ b/sys/fs/procfs/procfs_status.c @@ -78,7 +78,9 @@ procfs_dostatus(curp, p, pfs, uio) return (EOPNOTSUPP); pid = p->p_pid; + PROCTREE_LOCK(PT_SHARED); ppid = p->p_pptr ? p->p_pptr->p_pid : 0; + PROCTREE_LOCK(PT_RELEASE); pgid = p->p_pgrp->pg_id; sess = p->p_pgrp->pg_session; sid = sess->s_leader ? sess->s_leader->p_pid : 0; diff --git a/sys/i386/i386/vm_machdep.c b/sys/i386/i386/vm_machdep.c index dcf4986da814..0a3bc5154748 100644 --- a/sys/i386/i386/vm_machdep.c +++ b/sys/i386/i386/vm_machdep.c @@ -259,6 +259,18 @@ cpu_exit(p) mtx_enter(&sched_lock, MTX_SPIN); mtx_exit(&Giant, MTX_DEF | MTX_NOSWITCH); mtx_assert(&Giant, MA_NOTOWNED); + + /* + * We have to wait until after releasing all locks before + * changing p_stat. If we block on a mutex then we will be + * back at SRUN when we resume and our parent will never + * harvest us. + */ + p->p_stat = SZOMB; + + mp_fixme("assumption: p_pptr won't change at this time"); + wakeup(p->p_pptr); + cnt.v_swtch++; cpu_throw(); panic("cpu_exit"); diff --git a/sys/ia64/ia64/vm_machdep.c b/sys/ia64/ia64/vm_machdep.c index c1cbf50ae580..be2cb4d5d5aa 100644 --- a/sys/ia64/ia64/vm_machdep.c +++ b/sys/ia64/ia64/vm_machdep.c @@ -306,6 +306,18 @@ cpu_exit(p) mtx_enter(&sched_lock, MTX_SPIN); mtx_exit(&Giant, MTX_DEF | MTX_NOSWITCH); mtx_assert(&Giant, MA_NOTOWNED); + + /* + * We have to wait until after releasing all locks before + * changing p_stat. If we block on a mutex then we will be + * back at SRUN when we resume and our parent will never + * harvest us. + */ + p->p_stat = SZOMB; + + mp_fixme("assumption: p_pptr won't change at this time"); + wakeup(p->p_pptr); + cnt.v_swtch++; cpu_switch(); panic("cpu_exit"); diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c index 6c0ef1550c5d..ec1a4973482b 100644 --- a/sys/kern/kern_exec.c +++ b/sys/kern/kern_exec.c @@ -267,10 +267,12 @@ execve(p, uap) * it that it now has its own resources back */ p->p_flag |= P_EXEC; + PROCTREE_LOCK(PT_SHARED); if (p->p_pptr && (p->p_flag & P_PPWAIT)) { p->p_flag &= ~P_PPWAIT; wakeup((caddr_t)p->p_pptr); } + PROCTREE_LOCK(PT_RELEASE); /* * Implement image setuid/setgid. diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c index 816d062dd51e..e6a640bf179d 100644 --- a/sys/kern/kern_exit.c +++ b/sys/kern/kern_exit.c @@ -267,17 +267,10 @@ exit1(p, rv) ALLPROC_LOCK(AP_EXCLUSIVE); LIST_REMOVE(p, p_list); LIST_INSERT_HEAD(&zombproc, p, p_list); - LIST_REMOVE(p, p_hash); ALLPROC_LOCK(AP_RELEASE); - /* - * We have to wait until after releasing this lock before - * changing p_stat. If we block on a mutex while waiting to - * release the allproc_lock, then we will be back at SRUN when - * we resume here and our parent will never harvest us. - */ - p->p_stat = SZOMB; + PROCTREE_LOCK(PT_EXCLUSIVE); q = LIST_FIRST(&p->p_children); if (q) /* only need this if any child is S_ZOMB */ wakeup((caddr_t) initproc); @@ -343,7 +336,7 @@ exit1(p, rv) psignal(p->p_pptr, SIGCHLD); } - wakeup((caddr_t)p->p_pptr); + PROCTREE_LOCK(PT_RELEASE); /* * Clear curproc after we've done all operations * that could block, and before tearing down the rest @@ -419,6 +412,7 @@ wait1(q, uap, compat) return (EINVAL); loop: nfound = 0; + PROCTREE_LOCK(PT_SHARED); LIST_FOREACH(p, &q->p_children, p_sibling) { if (uap->pid != WAIT_ANY && p->p_pid != uap->pid && p->p_pgid != -uap->pid) @@ -440,6 +434,7 @@ wait1(q, uap, compat) mtx_enter(&sched_lock, MTX_SPIN); if (p->p_stat == SZOMB) { mtx_exit(&sched_lock, MTX_SPIN); + PROCTREE_LOCK(PT_RELEASE); /* charge childs scheduling cpu usage to parent */ if (curproc->p_pid != 1) { @@ -466,12 +461,17 @@ wait1(q, uap, compat) * If we got the child via a ptrace 'attach', * we need to give it back to the old parent. */ - if (p->p_oppid && (t = pfind(p->p_oppid))) { - p->p_oppid = 0; - proc_reparent(p, t); - psignal(t, SIGCHLD); - wakeup((caddr_t)t); - return (0); + if (p->p_oppid) { + PROCTREE_LOCK(PT_EXCLUSIVE); + if ((t = pfind(p->p_oppid)) != NULL) { + p->p_oppid = 0; + proc_reparent(p, t); + psignal(t, SIGCHLD); + wakeup((caddr_t)t); + PROCTREE_LOCK(PT_RELEASE); + return (0); + } + PROCTREE_LOCK(PT_RELEASE); } p->p_xstat = 0; ruadd(&q->p_stats->p_cru, p->p_ru); @@ -519,10 +519,14 @@ wait1(q, uap, compat) * Unlink it from its process group and free it. */ leavepgrp(p); + ALLPROC_LOCK(AP_EXCLUSIVE); LIST_REMOVE(p, p_list); /* off zombproc */ ALLPROC_LOCK(AP_RELEASE); + + PROCTREE_LOCK(PT_EXCLUSIVE); LIST_REMOVE(p, p_sibling); + PROCTREE_LOCK(PT_RELEASE); if (--p->p_procsig->ps_refcnt == 0) { if (p->p_sigacts != &p->p_addr->u_sigacts) @@ -545,6 +549,7 @@ wait1(q, uap, compat) if (p->p_stat == SSTOP && (p->p_flag & P_WAITED) == 0 && (p->p_flag & P_TRACED || uap->options & WUNTRACED)) { mtx_exit(&sched_lock, MTX_SPIN); + PROCTREE_LOCK(PT_RELEASE); p->p_flag |= P_WAITED; q->p_retval[0] = p->p_pid; #ifdef COMPAT_43 @@ -563,6 +568,7 @@ wait1(q, uap, compat) } mtx_exit(&sched_lock, MTX_SPIN); } + PROCTREE_LOCK(PT_RELEASE); if (nfound == 0) return (ECHILD); if (uap->options & WNOHANG) { @@ -575,7 +581,8 @@ wait1(q, uap, compat) } /* - * make process 'parent' the new parent of process 'child'. + * Make process 'parent' the new parent of process 'child'. + * Must be called with an exclusive hold of proctree lock. */ void proc_reparent(child, parent) @@ -583,6 +590,7 @@ proc_reparent(child, parent) register struct proc *parent; { + PROCTREE_ASSERT(PT_EXCLUSIVE); if (child->p_pptr == parent) return; diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c index 60171a5a854a..1c8cb3853c86 100644 --- a/sys/kern/kern_fork.c +++ b/sys/kern/kern_fork.c @@ -476,8 +476,10 @@ fork1(p1, flags, procp) pptr = initproc; else pptr = p1; + PROCTREE_LOCK(PT_EXCLUSIVE); p2->p_pptr = pptr; LIST_INSERT_HEAD(&pptr->p_children, p2, p_sibling); + PROCTREE_LOCK(PT_RELEASE); LIST_INIT(&p2->p_children); LIST_INIT(&p2->p_heldmtx); LIST_INIT(&p2->p_contested); diff --git a/sys/kern/kern_kthread.c b/sys/kern/kern_kthread.c index 6d0146a225f4..8292a8d455f8 100644 --- a/sys/kern/kern_kthread.c +++ b/sys/kern/kern_kthread.c @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -113,7 +114,10 @@ kthread_create(void (*func)(void *), void *arg, void kthread_exit(int ecode) { + + PROCTREE_LOCK(PT_EXCLUSIVE); proc_reparent(curproc, initproc); + PROCTREE_LOCK(PT_RELEASE); exit1(curproc, W_EXITCODE(ecode, 0)); } diff --git a/sys/kern/kern_ktrace.c b/sys/kern/kern_ktrace.c index 59321fe2423d..f0f187f56cd6 100644 --- a/sys/kern/kern_ktrace.c +++ b/sys/kern/kern_ktrace.c @@ -317,7 +317,6 @@ ktrace(curp, uap) ret |= ktrsetchildren(curp, p, ops, facs, vp); else ret |= ktrops(curp, p, ops, facs, vp); - } else { /* * by pid @@ -426,6 +425,7 @@ ktrsetchildren(curp, top, ops, facs, vp) register int ret = 0; p = top; + PROCTREE_LOCK(PT_SHARED); for (;;) { ret |= ktrops(curp, p, ops, facs, vp); /* @@ -436,8 +436,10 @@ ktrsetchildren(curp, top, ops, facs, vp) if (!LIST_EMPTY(&p->p_children)) p = LIST_FIRST(&p->p_children); else for (;;) { - if (p == top) + if (p == top) { + PROCTREE_LOCK(PT_RELEASE); return (ret); + } if (LIST_NEXT(p, p_sibling)) { p = LIST_NEXT(p, p_sibling); break; diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c index 814a29e45da4..83ba993addbd 100644 --- a/sys/kern/kern_proc.c +++ b/sys/kern/kern_proc.c @@ -73,6 +73,7 @@ u_long pgrphash; struct proclist allproc; struct proclist zombproc; struct lock allproc_lock; +struct lock proctree_lock; vm_zone_t proc_zone; vm_zone_t ithread_zone; @@ -84,6 +85,7 @@ procinit() { lockinit(&allproc_lock, PZERO, "allproc", 0, 0); + lockinit(&proctree_lock, PZERO, "proctree", 0, 0); LIST_INIT(&allproc); LIST_INIT(&zombproc); pidhashtbl = hashinit(maxproc / 4, M_PROC, &pidhash); @@ -106,11 +108,16 @@ int inferior(p) register struct proc *p; { + int rval = 1; + PROCTREE_LOCK(PT_SHARED); for (; p != curproc; p = p->p_pptr) - if (p->p_pid == 0) - return (0); - return (1); + if (p->p_pid == 0) { + rval = 0; + break; + } + PROCTREE_LOCK(PT_RELEASE); + return (rval); } /* @@ -281,6 +288,7 @@ fixjobc(p, pgrp, entering) * Check p's parent to see whether p qualifies its own process * group; if so, adjust count for p's process group. */ + PROCTREE_LOCK(PT_SHARED); if ((hispgrp = p->p_pptr->p_pgrp) != pgrp && hispgrp->pg_session == mysession) { if (entering) @@ -303,6 +311,7 @@ fixjobc(p, pgrp, entering) else if (--hispgrp->pg_jobc == 0) orphanpg(hispgrp); } + PROCTREE_LOCK(PT_RELEASE); } /* @@ -414,8 +423,10 @@ fill_kinfo_proc(p, kp) kp->ki_rtprio = p->p_rtprio; kp->ki_runtime = p->p_runtime; kp->ki_pid = p->p_pid; + PROCTREE_LOCK(PT_SHARED); if (p->p_pptr) kp->ki_ppid = p->p_pptr->p_pid; + PROCTREE_LOCK(PT_RELEASE); sp = NULL; if (p->p_pgrp) { kp->ki_pgid = p->p_pgrp->pg_id; diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c index 5e11a3f0b2ae..51ca9192d508 100644 --- a/sys/kern/kern_prot.c +++ b/sys/kern/kern_prot.c @@ -50,6 +50,7 @@ #include #include #include +#include #include #include #include @@ -65,8 +66,9 @@ struct getpid_args { #endif /* - * NOT MP SAFE due to p_pptr access + * getpid - MP SAFE */ + /* ARGSUSED */ int getpid(p, uap) @@ -76,11 +78,17 @@ getpid(p, uap) p->p_retval[0] = p->p_pid; #if defined(COMPAT_43) || defined(COMPAT_SUNOS) + PROCTREE_LOCK(PT_SHARED); p->p_retval[1] = p->p_pptr->p_pid; + PROCTREE_LOCK(PT_RELEASE); #endif return (0); } +/* + * getppid - MP SAFE + */ + #ifndef _SYS_SYSPROTO_H_ struct getppid_args { int dummy; @@ -93,7 +101,9 @@ getppid(p, uap) struct getppid_args *uap; { + PROCTREE_LOCK(PT_SHARED); p->p_retval[0] = p->p_pptr->p_pid; + PROCTREE_LOCK(PT_RELEASE); return (0); } diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index c6daeede402b..d52b1fc3ffea 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -1158,9 +1158,11 @@ psignal(p, sig) goto out; SIGDELSET(p->p_siglist, sig); p->p_xstat = sig; + PROCTREE_LOCK(PT_SHARED); if ((p->p_pptr->p_procsig->ps_flag & PS_NOCLDSTOP) == 0) psignal(p->p_pptr, SIGCHLD); stop(p); + PROCTREE_LOCK(PT_RELEASE); goto out; } else goto runfast; @@ -1296,14 +1298,17 @@ issignal(p) * stopped until released by the parent. */ p->p_xstat = sig; + PROCTREE_LOCK(PT_SHARED); psignal(p->p_pptr, SIGCHLD); do { stop(p); + PROCTREE_LOCK(PT_RELEASE); mtx_enter(&sched_lock, MTX_SPIN); DROP_GIANT_NOSWITCH(); mi_switch(); mtx_exit(&sched_lock, MTX_SPIN); PICKUP_GIANT(); + PROCTREE_LOCK(PT_SHARED); } while (!trace_req(p) && p->p_flag & P_TRACED); @@ -1369,9 +1374,11 @@ issignal(p) prop & SA_TTYSTOP)) break; /* == ignore */ p->p_xstat = sig; + PROCTREE_LOCK(PT_SHARED); stop(p); if ((p->p_pptr->p_procsig->ps_flag & PS_NOCLDSTOP) == 0) psignal(p->p_pptr, SIGCHLD); + PROCTREE_LOCK(PT_RELEASE); mtx_enter(&sched_lock, MTX_SPIN); DROP_GIANT_NOSWITCH(); mi_switch(); @@ -1414,13 +1421,15 @@ issignal(p) /* * Put the argument process into the stopped state and notify the parent * via wakeup. Signals are handled elsewhere. The process must not be - * on the run queue. + * on the run queue. Must be called with at least a shared hold of the + * proctree lock. */ void stop(p) register struct proc *p; { + PROCTREE_ASSERT(PT_SHARED); mtx_enter(&sched_lock, MTX_SPIN); p->p_stat = SSTOP; p->p_flag &= ~P_WAITED; diff --git a/sys/kern/sys_process.c b/sys/kern/sys_process.c index 5bdeb78dc11e..f6622b355ad5 100644 --- a/sys/kern/sys_process.c +++ b/sys/kern/sys_process.c @@ -276,8 +276,12 @@ ptrace(curp, uap) return EPERM; /* not being traced by YOU */ - if (p->p_pptr != curp) + PROCTREE_LOCK(PT_SHARED); + if (p->p_pptr != curp) { + PROCTREE_LOCK(PT_RELEASE); return EBUSY; + } + PROCTREE_LOCK(PT_RELEASE); /* not currently stopped */ mtx_enter(&sched_lock, MTX_SPIN); @@ -311,15 +315,19 @@ ptrace(curp, uap) case PT_TRACE_ME: /* set my trace flag and "owner" so it can read/write me */ p->p_flag |= P_TRACED; + PROCTREE_LOCK(PT_SHARED); p->p_oppid = p->p_pptr->p_pid; + PROCTREE_LOCK(PT_RELEASE); return 0; case PT_ATTACH: /* security check done above */ p->p_flag |= P_TRACED; + PROCTREE_LOCK(PT_EXCLUSIVE); p->p_oppid = p->p_pptr->p_pid; if (p->p_pptr != curp) proc_reparent(p, curp); + PROCTREE_LOCK(PT_RELEASE); uap->data = SIGSTOP; goto sendsig; /* in PT_CONTINUE below */ @@ -350,12 +358,14 @@ ptrace(curp, uap) if (uap->req == PT_DETACH) { /* reset process parent */ + PROCTREE_LOCK(PT_EXCLUSIVE); if (p->p_oppid != p->p_pptr->p_pid) { struct proc *pp; pp = pfind(p->p_oppid); proc_reparent(p, pp ? pp : initproc); } + PROCTREE_LOCK(PT_RELEASE); p->p_flag &= ~(P_TRACED | P_WAITED); p->p_oppid = 0; diff --git a/sys/miscfs/procfs/procfs_ctl.c b/sys/miscfs/procfs/procfs_ctl.c index f445572c9a3d..5d0ce44ba6f0 100644 --- a/sys/miscfs/procfs/procfs_ctl.c +++ b/sys/miscfs/procfs/procfs_ctl.c @@ -144,10 +144,12 @@ procfs_control(curp, p, op) p->p_flag |= P_TRACED; faultin(p); p->p_xstat = 0; /* XXX ? */ + PROCTREE_LOCK(PT_EXCLUSIVE); if (p->p_pptr != curp) { p->p_oppid = p->p_pptr->p_pid; proc_reparent(p, curp); } + PROCTREE_LOCK(PT_RELEASE); psignal(p, SIGSTOP); return (0); } @@ -164,12 +166,15 @@ procfs_control(curp, p, op) break; default: + PROCTREE_LOCK(PT_SHARED); mtx_enter(&sched_lock, MTX_SPIN); if (!TRACE_WAIT_P(curp, p)) { mtx_exit(&sched_lock, MTX_SPIN); + PROCTREE_LOCK(PT_RELEASE); return (EBUSY); } mtx_exit(&sched_lock, MTX_SPIN); + PROCTREE_LOCK(PT_RELEASE); } @@ -204,6 +209,7 @@ procfs_control(curp, p, op) SIGDELSET(p->p_siglist, SIGTRAP); /* give process back to original parent */ + PROCTREE_LOCK(PT_EXCLUSIVE); if (p->p_oppid != p->p_pptr->p_pid) { struct proc *pp; @@ -211,6 +217,7 @@ procfs_control(curp, p, op) if (pp) proc_reparent(p, pp); } + PROCTREE_LOCK(PT_RELEASE); p->p_oppid = 0; p->p_flag &= ~P_WAITED; /* XXX ? */ @@ -244,19 +251,23 @@ procfs_control(curp, p, op) case PROCFS_CTL_WAIT: error = 0; if (p->p_flag & P_TRACED) { + PROCTREE_LOCK(PT_SHARED); mtx_enter(&sched_lock, MTX_SPIN); while (error == 0 && (p->p_stat != SSTOP) && (p->p_flag & P_TRACED) && (p->p_pptr == curp)) { mtx_exit(&sched_lock, MTX_SPIN); + PROCTREE_LOCK(PT_RELEASE); error = tsleep((caddr_t) p, PWAIT|PCATCH, "procfsx", 0); + PROCTREE_LOCK(PT_SHARED); mtx_enter(&sched_lock, MTX_SPIN); } if (error == 0 && !TRACE_WAIT_P(curp, p)) error = EBUSY; mtx_exit(&sched_lock, MTX_SPIN); + PROCTREE_LOCK(PT_RELEASE); } else { mtx_enter(&sched_lock, MTX_SPIN); while (error == 0 && p->p_stat != SSTOP) { @@ -317,6 +328,7 @@ procfs_doctl(curp, p, pfs, uio) } else { nm = vfs_findname(signames, msg, xlen); if (nm) { + PROCTREE_LOCK(PT_SHARED); mtx_enter(&sched_lock, MTX_SPIN); if (TRACE_WAIT_P(curp, p)) { p->p_xstat = nm->nm_val; @@ -325,8 +337,10 @@ procfs_doctl(curp, p, pfs, uio) #endif setrunnable(p); mtx_exit(&sched_lock, MTX_SPIN); + PROCTREE_LOCK(PT_RELEASE); } else { mtx_exit(&sched_lock, MTX_SPIN); + PROCTREE_LOCK(PT_RELEASE); psignal(p, nm->nm_val); } error = 0; diff --git a/sys/miscfs/procfs/procfs_status.c b/sys/miscfs/procfs/procfs_status.c index b06a0af1aa5e..d7dd108be9fa 100644 --- a/sys/miscfs/procfs/procfs_status.c +++ b/sys/miscfs/procfs/procfs_status.c @@ -78,7 +78,9 @@ procfs_dostatus(curp, p, pfs, uio) return (EOPNOTSUPP); pid = p->p_pid; + PROCTREE_LOCK(PT_SHARED); ppid = p->p_pptr ? p->p_pptr->p_pid : 0; + PROCTREE_LOCK(PT_RELEASE); pgid = p->p_pgrp->pg_id; sess = p->p_pgrp->pg_session; sid = sess->s_leader ? sess->s_leader->p_pid : 0; diff --git a/sys/powerpc/aim/vm_machdep.c b/sys/powerpc/aim/vm_machdep.c index ba5004d327ff..c60f971a114c 100644 --- a/sys/powerpc/aim/vm_machdep.c +++ b/sys/powerpc/aim/vm_machdep.c @@ -256,6 +256,18 @@ cpu_exit(p) mtx_enter(&sched_lock, MTX_SPIN); mtx_exit(&Giant, MTX_DEF | MTX_NOSWITCH); mtx_assert(&Giant, MA_NOTOWNED); + + /* + * We have to wait until after releasing all locks before + * changing p_stat. If we block on a mutex then we will be + * back at SRUN when we resume and our parent will never + * harvest us. + */ + p->p_stat = SZOMB; + + mp_fixme("assumption: p_pptr won't change at this time"); + wakeup(p->p_pptr); + cnt.v_swtch++; cpu_switch(); panic("cpu_exit"); diff --git a/sys/powerpc/powerpc/vm_machdep.c b/sys/powerpc/powerpc/vm_machdep.c index ba5004d327ff..c60f971a114c 100644 --- a/sys/powerpc/powerpc/vm_machdep.c +++ b/sys/powerpc/powerpc/vm_machdep.c @@ -256,6 +256,18 @@ cpu_exit(p) mtx_enter(&sched_lock, MTX_SPIN); mtx_exit(&Giant, MTX_DEF | MTX_NOSWITCH); mtx_assert(&Giant, MA_NOTOWNED); + + /* + * We have to wait until after releasing all locks before + * changing p_stat. If we block on a mutex then we will be + * back at SRUN when we resume and our parent will never + * harvest us. + */ + p->p_stat = SZOMB; + + mp_fixme("assumption: p_pptr won't change at this time"); + wakeup(p->p_pptr); + cnt.v_swtch++; cpu_switch(); panic("cpu_exit"); diff --git a/sys/sys/proc.h b/sys/sys/proc.h index e16fdd846ab8..b0c90a2d3bc8 100644 --- a/sys/sys/proc.h +++ b/sys/sys/proc.h @@ -415,6 +415,17 @@ sigonstack(size_t sp) #define AP_EXCLUSIVE LK_EXCLUSIVE #define AP_RELEASE LK_RELEASE +/* Lock and unlock the proc child and sibling lists. */ +#define PROCTREE_LOCK(how) \ + lockmgr(&proctree_lock, (how), NULL, CURPROC) + +#define PROCTREE_ASSERT(what) \ + LOCKMGR_ASSERT(&proctree_lock, (what), CURPROC) + +#define PT_SHARED LK_SHARED +#define PT_EXCLUSIVE LK_EXCLUSIVE +#define PT_RELEASE LK_RELEASE + /* Hold process U-area in memory, normally for ptrace/procfs work. */ #define PHOLD(p) do { \ PROC_LOCK(p); \ @@ -439,6 +450,7 @@ extern LIST_HEAD(pgrphashhead, pgrp) *pgrphashtbl; extern u_long pgrphash; extern struct lock allproc_lock; +extern struct lock proctree_lock; extern struct proc proc0; /* Process slot for swapper. */ extern int hogticks; /* Limit on kernel cpu hogs. */ extern int nprocs, maxproc; /* Current and max number of procs. */