Adjust lock requirements for ps_pptr, ps_ppid and ps_oppid.
authorclaudio <claudio@openbsd.org>
Tue, 8 Oct 2024 09:05:40 +0000 (09:05 +0000)
committerclaudio <claudio@openbsd.org>
Tue, 8 Oct 2024 09:05:40 +0000 (09:05 +0000)
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
sys/kern/kern_fork.c
sys/kern/kern_prot.c
sys/kern/sys_process.c
sys/sys/proc.h

index fd2d057..47b3ea6 100644 (file)
@@ -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;
 }
index 224ed7f..5be080b 100644 (file)
@@ -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.
index a2b22cd..6178bb5 100644 (file)
@@ -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);
 }
 
index 8d9daeb..36c4021 100644 (file)
@@ -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);
index 1472a16..86cd44d 100644 (file)
@@ -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 */