From 33040064d5ebba69926e42603be268869271dd9f Mon Sep 17 00:00:00 2001 From: Claudio Jeker Date: Tue, 8 Oct 2024 09:05:41 +0000 Subject: Adjust lock requirements for ps_pptr, ps_ppid and ps_oppid. ps_pptr, ps_ppid and ps_oppid require the KERNEL_LOCK and the process mutex to be modified. At the same time either KERNEL_LOCK or process mutex needs to be taken to read the values. This is needed to further unlock ptsignal(). OK kettenis@, mvs@ --- sys/kern/kern_exit.c | 14 +++++++++++++- sys/kern/kern_fork.c | 4 +++- sys/kern/kern_prot.c | 4 +++- sys/kern/sys_process.c | 12 ++++++++++-- sys/sys/proc.h | 10 +++++----- 5 files changed, 34 insertions(+), 10 deletions(-) diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c index fd2d05778fc..47b3ea68468 100644 --- a/sys/kern/kern_exit.c +++ b/sys/kern/kern_exit.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_exit.c,v 1.234 2024/09/30 12:32:26 claudio Exp $ */ +/* $OpenBSD: kern_exit.c,v 1.235 2024/10/08 09:05:40 claudio Exp $ */ /* $NetBSD: kern_exit.c,v 1.39 1996/04/22 01:38:25 christos Exp $ */ /* @@ -308,9 +308,11 @@ exit1(struct proc *p, int xexit, int xsig, int flags) * Traced processes are killed since their * existence means someone is screwing up. */ + mtx_enter(&qr->ps_mtx); if (qr->ps_flags & PS_TRACED && !(qr->ps_flags & PS_EXITING)) { process_untrace(qr); + mtx_leave(&qr->ps_mtx); /* * If single threading is active, @@ -324,6 +326,7 @@ exit1(struct proc *p, int xexit, int xsig, int flags) prsignal(qr, SIGKILL); } else { process_reparent(qr, initprocess); + mtx_leave(&qr->ps_mtx); } } @@ -331,9 +334,11 @@ exit1(struct proc *p, int xexit, int xsig, int flags) * Make sure orphans won't remember the exiting process. */ while ((qr = LIST_FIRST(&pr->ps_orphans)) != NULL) { + mtx_enter(&qr->ps_mtx); KASSERT(qr->ps_oppid == pr->ps_pid); qr->ps_oppid = 0; process_clear_orphan(qr); + mtx_leave(&qr->ps_mtx); } } @@ -359,11 +364,13 @@ exit1(struct proc *p, int xexit, int xsig, int flags) * we can wake our original parent to possibly unblock * wait4() to return ECHILD. */ + mtx_enter(&pr->ps_mtx); if (pr->ps_flags & PS_NOZOMBIE) { struct process *ppr = pr->ps_pptr; process_reparent(pr, initprocess); wakeup(ppr); } + mtx_leave(&pr->ps_mtx); } /* just a thread? check if last one standing. */ @@ -746,14 +753,17 @@ proc_finish_wait(struct proc *waiter, struct process *pr) * If we got the child via a ptrace 'attach', * we need to give it back to the old parent. */ + mtx_enter(&pr->ps_mtx); if (pr->ps_oppid != 0 && (pr->ps_oppid != pr->ps_ppid) && (tr = prfind(pr->ps_oppid))) { pr->ps_oppid = 0; atomic_clearbits_int(&pr->ps_flags, PS_TRACED); process_reparent(pr, tr); + mtx_leave(&pr->ps_mtx); prsignal(tr, SIGCHLD); wakeup(tr); } else { + mtx_leave(&pr->ps_mtx); scheduler_wait_hook(waiter, pr->ps_mainproc); rup = &waiter->p_p->ps_cru; ruadd(rup, pr->ps_ru); @@ -772,6 +782,7 @@ process_untrace(struct process *pr) struct process *ppr = NULL; KASSERT(pr->ps_flags & PS_TRACED); + MUTEX_ASSERT_LOCKED(&pr->ps_mtx); if (pr->ps_oppid != 0 && (pr->ps_oppid != pr->ps_ppid)) @@ -814,6 +825,7 @@ process_reparent(struct process *child, struct process *parent) LIST_INSERT_HEAD(&child->ps_pptr->ps_orphans, child, ps_orphan); } + MUTEX_ASSERT_LOCKED(&child->ps_mtx); child->ps_pptr = parent; child->ps_ppid = parent->ps_pid; } diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c index 224ed7fff7a..5be080bf618 100644 --- a/sys/kern/kern_fork.c +++ b/sys/kern/kern_fork.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_fork.c,v 1.265 2024/08/21 03:07:45 deraadt Exp $ */ +/* $OpenBSD: kern_fork.c,v 1.266 2024/10/08 09:05:40 claudio Exp $ */ /* $NetBSD: kern_fork.c,v 1.29 1996/02/09 18:59:34 christos Exp $ */ /* @@ -457,6 +457,7 @@ fork1(struct proc *curp, int flags, void (*func)(void *), void *arg, LIST_INSERT_AFTER(curpr, pr, ps_pglist); LIST_INSERT_HEAD(&curpr->ps_children, pr, ps_sibling); + mtx_enter(&pr->ps_mtx); if (pr->ps_flags & PS_TRACED) { pr->ps_oppid = curpr->ps_pid; process_reparent(pr, curpr->ps_pptr); @@ -473,6 +474,7 @@ fork1(struct proc *curp, int flags, void (*func)(void *), void *arg, pr->ps_ptstat->pe_other_pid = curpr->ps_pid; } } + mtx_leave(&pr->ps_mtx); /* * For new processes, set accounting bits and mark as complete. diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c index a2b22cd760a..6178bb5bd0e 100644 --- a/sys/kern/kern_prot.c +++ b/sys/kern/kern_prot.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_prot.c,v 1.82 2023/01/09 02:12:13 guenther Exp $ */ +/* $OpenBSD: kern_prot.c,v 1.83 2024/10/08 09:05:40 claudio Exp $ */ /* $NetBSD: kern_prot.c,v 1.33 1996/02/09 18:59:42 christos Exp $ */ /* @@ -83,7 +83,9 @@ int sys_getppid(struct proc *p, void *v, register_t *retval) { + mtx_enter(&p->p_p->ps_mtx); *retval = p->p_p->ps_ppid; + mtx_leave(&p->p_p->ps_mtx); return (0); } diff --git a/sys/kern/sys_process.c b/sys/kern/sys_process.c index 8d9daeb1120..36c402135ae 100644 --- a/sys/kern/sys_process.c +++ b/sys/kern/sys_process.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sys_process.c,v 1.100 2024/10/01 08:28:34 claudio Exp $ */ +/* $OpenBSD: sys_process.c,v 1.101 2024/10/08 09:05:40 claudio Exp $ */ /* $NetBSD: sys_process.c,v 1.55 1996/05/15 06:17:47 tls Exp $ */ /*- @@ -288,10 +288,14 @@ ptrace_ctrl(struct proc *p, int req, pid_t pid, caddr_t addr, int data) case PT_TRACE_ME: /* Just set the trace flag. */ tr = p->p_p; - if (ISSET(tr->ps_flags, PS_TRACED)) + mtx_enter(&tr->ps_mtx); + if (ISSET(tr->ps_flags, PS_TRACED)) { + mtx_leave(&tr->ps_mtx); return EBUSY; + } atomic_setbits_int(&tr->ps_flags, PS_TRACED); tr->ps_oppid = tr->ps_ppid; + mtx_leave(&tr->ps_mtx); if (tr->ps_ptstat == NULL) tr->ps_ptstat = malloc(sizeof(*tr->ps_ptstat), M_SUBPROC, M_WAITOK); @@ -489,8 +493,10 @@ ptrace_ctrl(struct proc *p, int req, pid_t pid, caddr_t addr, int data) goto fail; #endif + mtx_enter(&tr->ps_mtx); process_untrace(tr); atomic_clearbits_int(&tr->ps_flags, PS_WAITED); + mtx_leave(&tr->ps_mtx); sendsig: memset(tr->ps_ptstat, 0, sizeof(*tr->ps_ptstat)); @@ -526,9 +532,11 @@ ptrace_ctrl(struct proc *p, int req, pid_t pid, caddr_t addr, int data) * proc gets to see all the action. * Stop the target. */ + mtx_enter(&tr->ps_mtx); atomic_setbits_int(&tr->ps_flags, PS_TRACED); tr->ps_oppid = tr->ps_ppid; process_reparent(tr, p->p_p); + mtx_leave(&tr->ps_mtx); if (tr->ps_ptstat == NULL) tr->ps_ptstat = malloc(sizeof(*tr->ps_ptstat), M_SUBPROC, M_WAITOK); diff --git a/sys/sys/proc.h b/sys/sys/proc.h index 1472a167da0..86cd44d1946 100644 --- a/sys/sys/proc.h +++ b/sys/sys/proc.h @@ -1,4 +1,4 @@ -/* $OpenBSD: proc.h,v 1.372 2024/10/01 08:28:34 claudio Exp $ */ +/* $OpenBSD: proc.h,v 1.373 2024/10/08 09:05:40 claudio Exp $ */ /* $NetBSD: proc.h,v 1.44 1996/04/22 01:23:21 christos Exp $ */ /*- @@ -157,7 +157,7 @@ struct process { TAILQ_HEAD(,proc) ps_threads; /* [K|m] Threads in this process. */ LIST_ENTRY(process) ps_pglist; /* List of processes in pgrp. */ - struct process *ps_pptr; /* Pointer to parent process. */ + struct process *ps_pptr; /* [K|m] Pointer to parent process. */ LIST_ENTRY(process) ps_sibling; /* List of sibling processes. */ LIST_HEAD(, process) ps_children;/* Pointer to list of children. */ LIST_ENTRY(process) ps_hash; /* Hash chain. */ @@ -176,7 +176,7 @@ struct process { struct vnode *ps_textvp; /* Vnode of executable. */ struct filedesc *ps_fd; /* Ptr to open files structure */ struct vmspace *ps_vmspace; /* Address space */ - pid_t ps_pid; /* Process identifier. */ + pid_t ps_pid; /* [I] Process identifier. */ struct futex_list ps_ftlist; /* futexes attached to this process */ struct tslpqueue ps_tslpqueue; /* [p] queue of threads in thrsleep */ @@ -200,8 +200,8 @@ struct process { u_int ps_xexit; /* Exit status for wait */ int ps_xsig; /* Stopping or killing signal */ - pid_t ps_ppid; /* [a] Cached parent pid */ - pid_t ps_oppid; /* [a] Save parent pid during ptrace. */ + pid_t ps_ppid; /* [K|m] Cached parent pid */ + pid_t ps_oppid; /* [K|m] Old parent pid during ptrace */ int ps_ptmask; /* Ptrace event mask */ struct ptrace_state *ps_ptstat;/* Ptrace state */ -- cgit v1.2.3