On 14/10/17(Sat) 22:07, Philip Guenther wrote:
>
> The diff below adds proctreelk, an rwlock protecting the links of the
> process tree and related bits, as well as uidinfolk, an rwlock protecting
> the uidinfo hash table.
>
> Parts of this are based on FreeBSD's proctree_lock, particularly the
> reorganization of enterpgrp() into enternewpgrp() and enterthispgrp() and
> the splitting out of killjobc() from exit1().
>
> This diff should address the previously reported crashes seen when using
> ktrace(2) while creating/exiting processes.
>
> This has been stable for quite a while under my usage; please test and
> report any issues.
First of all, I'm very happy to see this diff. Thanks Philip.
I have been running this diff on my amd64 NFS client/server build
machine since you posted it. So far no issue, so it is stable for
this usage as well.
I'd however appreciate if you could commit the killjobc() and
enter*grp() refactoring first. Because in case of revert this
would be less pain.
That's also for this reason that I introduced a macro for the
NET_LOCK(). So I could enable/disable it without having to revert
N files. No idea if this could be useful there two.
I like the way you annotate the protected elements in the structure.
I'll try to do the same for bpf.
I'm also suggesting you commit the `uidinfolk' bits first. This seems
quite safe. In this exact part, what about introducing a uid_release(),
a wrapper around rw_exit_write(&uidinfolk), to be called after uid_find()?
This way you can keep the lock local.
I have other questions, but I'll keep them for the upcoming smaller
diffs :)
> Index: sys/proc.h
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/sys/proc.h,v
> retrieving revision 1.240
> diff -u -p -r1.240 proc.h
> --- sys/proc.h 29 Aug 2017 02:51:27 -0000 1.240
> +++ sys/proc.h 1 Sep 2017 05:48:15 -0000
> @@ -44,6 +44,7 @@
> #include <sys/selinfo.h> /* For struct selinfo */
> #include <sys/syslimits.h> /* For LOGIN_NAME_MAX */
> #include <sys/queue.h>
> +#include <sys/rwlock.h> /* For struct rwlock */
> #include <sys/timeout.h> /* For struct timeout */
> #include <sys/event.h> /* For struct klist */
> #include <sys/mutex.h> /* For struct mutex */
> @@ -55,16 +56,25 @@
> #endif
>
> /*
> + * Locks used to protect struct members in this file:
> + * I immutable after creation
> + * t proctreelk
> + *
> + * If multiple locks are listed then all are required for writes,
> + * but any one of them is sufficient for reads.
> + */
> +
> +/*
> * One structure allocated per session.
> */
> struct process;
> struct session {
> int s_count; /* Ref cnt; pgrps in session. */
> - struct process *s_leader; /* Session leader. */
> + struct process *s_leader; /* [t] Session leader. */
> struct vnode *s_ttyvp; /* Vnode of controlling terminal. */
> - struct tty *s_ttyp; /* Controlling terminal. */
> - char s_login[LOGIN_NAME_MAX]; /* Setlogin() name. */
> - pid_t s_verauthppid;
> + struct tty *s_ttyp; /* [t] Controlling terminal. */
> + char s_login[LOGIN_NAME_MAX];/* setlogin() name. */
> + pid_t s_verauthppid; /* TIOCSETVERAUTH info */
> uid_t s_verauthuid;
> struct timeout s_verauthto;
> };
> @@ -75,10 +85,10 @@ void zapverauth(/* struct session */ voi
> * One structure allocated per process group.
> */
> struct pgrp {
> - LIST_ENTRY(pgrp) pg_hash; /* Hash chain. */
> - LIST_HEAD(, process) pg_members;/* Pointer to pgrp members. */
> - struct session *pg_session; /* Pointer to session. */
> - pid_t pg_id; /* Pgrp id. */
> + LIST_ENTRY(pgrp) pg_hash; /* [t] Hash chain. */
> + LIST_HEAD(, process) pg_members;/* [t] Pointer to pgrp members. */
> + struct session *pg_session; /* [I] Pointer to session. */
> + pid_t pg_id; /* [I] Pgrp id. */
> int pg_jobc; /* # procs qualifying pgrp for job control */
> };
>
> @@ -156,17 +166,17 @@ struct process {
> LIST_ENTRY(process) ps_list; /* List of all processes. */
> TAILQ_HEAD(,proc) ps_threads; /* Threads in this process. */
>
> - LIST_ENTRY(process) ps_pglist; /* List of processes in pgrp. */
> - struct process *ps_pptr; /* 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_pglist; /* [t] List of processes in pgrp. */
> + struct process *ps_pptr; /* [t] Pointer to parent process. */
> + LIST_ENTRY(process) ps_sibling; /* [t] List of sibling processes. */
> + LIST_HEAD(, process) ps_children;/* [t] Pointer to list of children. */
> LIST_ENTRY(process) ps_hash; /* Hash chain. */
>
> struct sigacts *ps_sigacts; /* Signal actions, state */
> 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. */
>
> /* The following fields are all zeroed upon creation in process_new. */
> #define ps_startzero ps_klist
> @@ -180,7 +190,7 @@ struct process {
> struct vnode *ps_tracevp; /* Trace to vnode. */
> struct ucred *ps_tracecred; /* Creds for writing trace */
>
> - pid_t ps_oppid; /* Save parent pid during ptrace. */
> + pid_t ps_oppid; /* [t] Save parent pid during ptrace. */
> int ps_ptmask; /* Ptrace event mask */
> struct ptrace_state *ps_ptstat;/* Ptrace state */
>
> @@ -197,7 +207,7 @@ struct process {
> /* The following fields are all copied upon creation in process_new. */
> #define ps_startcopy ps_limit
> struct plimit *ps_limit; /* Process limits. */
> - struct pgrp *ps_pgrp; /* Pointer to process group. */
> + struct pgrp *ps_pgrp; /* [t] Pointer to process group. */
> struct emul *ps_emul; /* Emulation information */
>
> char ps_comm[MAXCOMLEN+1];
> @@ -407,6 +417,7 @@ struct uidinfo {
> };
>
> struct uidinfo *uid_find(uid_t);
> +extern struct rwlock uidinfolk; /* locked when uid_find()
> returns */
>
> /*
> * We use process IDs <= PID_MAX; PID_MAX + 1 must also fit in a pid_t,
> @@ -473,6 +484,7 @@ LIST_HEAD(processlist, process);
> extern struct processlist allprocess; /* List of all processes. */
> extern struct processlist zombprocess; /* List of zombie processes. */
> extern struct proclist allproc; /* List of all threads. */
> +extern struct rwlock proctreelk; /* parent/child, pgrp, session */
>
> extern struct process *initprocess; /* Process slot for init. */
> extern struct proc *reaperproc; /* Thread slot for reaper. */
> @@ -495,12 +507,12 @@ void proc_printit(struct proc *p, const
> int (*pr)(const char *, ...));
>
> int chgproccnt(uid_t uid, int diff);
> -int enterpgrp(struct process *, pid_t, struct pgrp *, struct session *);
> -void fixjobc(struct process *, struct pgrp *, int);
> +void enternewpgrp(struct process *, struct pgrp *, struct session *);
> +void enterthispgrp(struct process *, struct pgrp *);
> int inferior(struct process *, struct process *);
> void leavepgrp(struct process *);
> +void killjobc(struct process *);
> void preempt(void);
> -void pgdelete(struct pgrp *);
> void procinit(void);
> void resetpriority(struct proc *);
> void setrunnable(struct proc *);
> Index: sys/ucred.h
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/sys/ucred.h,v
> retrieving revision 1.11
> diff -u -p -r1.11 ucred.h
> --- sys/ucred.h 2 Mar 2015 20:46:50 -0000 1.11
> +++ sys/ucred.h 4 Sep 2017 03:19:13 -0000
> @@ -67,8 +67,10 @@ struct xucred {
> gid_t cr_groups[NGROUPS_MAX]; /* groups */
> };
>
> -#ifdef _KERNEL
> -#define crhold(cr) (cr)->cr_ref++
> +#if defined(_KERNEL) && !defined(_STANDALONE)
> +#include <sys/atomic.h>
> +
> +#define crhold(cr) atomic_inc_int(&(cr)->cr_ref)
>
> #define SUSER_NOACCT 0x1 /* don't mark accounting flags */
>
> @@ -81,6 +83,6 @@ struct ucred *crget(void);
> int suser(struct proc *p, u_int flags);
> int suser_ucred(struct ucred *cred);
>
> -#endif /* _KERNEL */
> +#endif /* _KERNEL && !_STANDALONE */
>
> #endif /* !_SYS_UCRED_H_ */
> Index: kern/exec_elf.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/exec_elf.c,v
> retrieving revision 1.141
> diff -u -p -r1.141 exec_elf.c
> --- kern/exec_elf.c 7 Sep 2017 11:32:14 -0000 1.141
> +++ kern/exec_elf.c 9 Sep 2017 18:20:47 -0000
> @@ -1168,12 +1168,14 @@ coredump_notes_elf(struct proc *p, void
> cpi.cpi_sigcatch = pr->ps_sigacts->ps_sigcatch;
>
> cpi.cpi_pid = pr->ps_pid;
> + rw_enter_read(&proctreelk);
> cpi.cpi_ppid = pr->ps_pptr->ps_pid;
> cpi.cpi_pgrp = pr->ps_pgid;
> if (pr->ps_session->s_leader)
> cpi.cpi_sid = pr->ps_session->s_leader->ps_pid;
> else
> cpi.cpi_sid = 0;
> + rw_exit_read(&proctreelk);
>
> cpi.cpi_ruid = p->p_ucred->cr_ruid;
> cpi.cpi_euid = p->p_ucred->cr_uid;
> Index: kern/kern_acct.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/kern_acct.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 kern_acct.c
> --- kern/kern_acct.c 21 Jan 2017 05:42:03 -0000 1.33
> +++ kern/kern_acct.c 29 Jul 2017 21:11:23 -0000
> @@ -206,11 +206,13 @@ acct_process(struct proc *p)
> acct.ac_gid = pr->ps_ucred->cr_rgid;
>
> /* (7) The terminal from which the process was started */
> + rw_enter_read(&proctreelk);
> if ((pr->ps_flags & PS_CONTROLT) &&
> pr->ps_pgrp->pg_session->s_ttyp)
> acct.ac_tty = pr->ps_pgrp->pg_session->s_ttyp->t_dev;
> else
> acct.ac_tty = NODEV;
> + rw_exit_read(&proctreelk);
>
> /* (8) The boolean flags that tell how the process terminated, etc. */
> acct.ac_flag = pr->ps_acflag;
> Index: kern/kern_exec.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/kern_exec.c,v
> retrieving revision 1.189
> diff -u -p -r1.189 kern_exec.c
> --- kern/kern_exec.c 29 Aug 2017 02:51:27 -0000 1.189
> +++ kern/kern_exec.c 29 Aug 2017 16:02:01 -0000
> @@ -503,9 +503,11 @@ sys_execve(struct proc *p, void *v, regi
>
> atomic_setbits_int(&pr->ps_flags, PS_EXEC);
> if (pr->ps_flags & PS_PPWAIT) {
> + rw_enter_read(&proctreelk);
> atomic_clearbits_int(&pr->ps_flags, PS_PPWAIT);
> atomic_clearbits_int(&pr->ps_pptr->ps_flags, PS_ISPWAIT);
> wakeup(pr->ps_pptr);
> + rw_exit_read(&proctreelk);
> }
>
> /*
> Index: kern/kern_exit.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/kern_exit.c,v
> retrieving revision 1.161
> diff -u -p -r1.161 kern_exit.c
> --- kern/kern_exit.c 29 Aug 2017 02:51:27 -0000 1.161
> +++ kern/kern_exit.c 29 Aug 2017 16:02:01 -0000
> @@ -118,7 +118,6 @@ exit1(struct proc *p, int rv, int flags)
> {
> struct process *pr, *qr, *nqr;
> struct rusage *rup;
> - struct vnode *ovp;
>
> atomic_setbits_int(&p->p_flag, P_WEXIT);
>
> @@ -185,44 +184,7 @@ exit1(struct proc *p, int rv, int flags)
> #ifdef SYSVSEM
> semexit(pr);
> #endif
> - if (SESS_LEADER(pr)) {
> - struct session *sp = pr->ps_session;
> -
> - if (sp->s_ttyvp) {
> - /*
> - * Controlling process.
> - * Signal foreground pgrp,
> - * drain controlling terminal
> - * and revoke access to controlling terminal.
> - */
> - if (sp->s_ttyp->t_session == sp) {
> - if (sp->s_ttyp->t_pgrp)
> - pgsignal(sp->s_ttyp->t_pgrp,
> - SIGHUP, 1);
> - ttywait(sp->s_ttyp);
> - /*
> - * The tty could have been revoked
> - * if we blocked.
> - */
> - if (sp->s_ttyvp)
> - VOP_REVOKE(sp->s_ttyvp,
> - REVOKEALL);
> - }
> - ovp = sp->s_ttyvp;
> - sp->s_ttyvp = NULL;
> - if (ovp)
> - vrele(ovp);
> - /*
> - * s_ttyp is not zero'd; we use this to
> - * indicate that the session once had a
> - * controlling terminal. (for logging and
> - * informational purposes)
> - */
> - }
> - sp->s_leader = NULL;
> - }
> - fixjobc(pr, pr->ps_pgrp, 0);
> -
> + killjobc(pr);
> #ifdef ACCOUNTING
> acct_process(p);
> #endif
> @@ -251,11 +213,7 @@ exit1(struct proc *p, int rv, int flags)
> * thread of a process that isn't PS_NOZOMBIE, we'll put
> * the process on the zombprocess list below.
> */
> - /*
> - * NOTE: WE ARE NO LONGER ALLOWED TO SLEEP!
> - */
> - p->p_stat = SDEAD;
> -
> + rw_enter_write(&proctreelk);
> LIST_REMOVE(p, p_hash);
> LIST_REMOVE(p, p_list);
>
> @@ -303,6 +261,7 @@ exit1(struct proc *p, int rv, int flags)
> }
> }
> }
> + rw_exit_write(&proctreelk);
>
> /* add thread's accumulated rusage into the process's total */
> ruadd(rup, &p->p_ru);
> @@ -331,9 +290,13 @@ exit1(struct proc *p, int rv, int flags)
> * wait4() to return ECHILD.
> */
> if (pr->ps_flags & PS_NOZOMBIE) {
> - struct process *ppr = pr->ps_pptr;
> + struct process *ppr;
> +
> + rw_enter_write(&proctreelk);
> + ppr = pr->ps_pptr;
> proc_reparent(pr, initprocess);
> wakeup(ppr);
> + rw_exit_write(&proctreelk);
> }
>
> /*
> @@ -351,6 +314,11 @@ exit1(struct proc *p, int rv, int flags)
> }
>
> /*
> + * NOTE: WE ARE NO LONGER ALLOWED TO SLEEP!
> + */
> + p->p_stat = SDEAD;
> +
> + /*
> * Other substructures are freed from reaper and wait().
> */
>
> @@ -451,6 +419,7 @@ reaper(void)
> /* Release the rest of the process's vmspace */
> uvm_exit(pr);
>
> + rw_enter_write(&proctreelk);
> if ((pr->ps_flags & PS_NOZOMBIE) == 0) {
> /* Process is now a true zombie. */
> atomic_setbits_int(&pr->ps_flags, PS_ZOMBIE);
> @@ -458,6 +427,7 @@ reaper(void)
>
> /* Wake up the parent so it can get exit
> status. */
> wakeup(pr->ps_pptr);
> + rw_exit_write(&proctreelk);
> } else {
> /* No one will wait for us. Just zap the
> process now */
> process_zap(pr);
> @@ -511,6 +481,7 @@ dowait4(struct proc *q, pid_t pid, int *
> return (EINVAL);
>
> loop:
> + rw_enter_write(&proctreelk);
> nfound = 0;
> LIST_FOREACH(pr, &q->p_p->ps_children, ps_sibling) {
> if ((pr->ps_flags & PS_NOZOMBIE) ||
> @@ -536,6 +507,7 @@ loop:
> (pr->ps_flags & PS_WAITED) == 0 && pr->ps_single &&
> pr->ps_single->p_stat == SSTOP &&
> (pr->ps_single->p_flag & P_SUSPSINGLE) == 0) {
> + rw_exit_write(&proctreelk);
> single_thread_wait(pr);
>
> atomic_setbits_int(&pr->ps_flags, PS_WAITED);
> @@ -559,6 +531,7 @@ loop:
> *statusp = W_STOPCODE(p->p_xstat);
> if (rusage != NULL)
> memset(rusage, 0, sizeof(*rusage));
> + rw_exit_write(&proctreelk);
> return (0);
> }
> if ((options & WCONTINUED) && (p->p_flag & P_CONTINUED)) {
> @@ -569,16 +542,21 @@ loop:
> *statusp = _WCONTINUED;
> if (rusage != NULL)
> memset(rusage, 0, sizeof(*rusage));
> + rw_exit_write(&proctreelk);
> return (0);
> }
> }
> - if (nfound == 0)
> + if (nfound == 0) {
> + rw_exit_write(&proctreelk);
> return (ECHILD);
> + }
> if (options & WNOHANG) {
> + rw_exit_write(&proctreelk);
> retval[0] = 0;
> return (0);
> }
> - if ((error = tsleep(q->p_p, PWAIT | PCATCH, "wait", 0)) != 0)
> + if ((error = rwsleep(q->p_p, &proctreelk, PWAIT | PCATCH | PNORELOCK,
> + "wait", 0)) != 0)
> return (error);
> goto loop;
> }
> @@ -589,6 +567,8 @@ proc_finish_wait(struct proc *waiter, st
> struct process *pr, *tr;
> struct rusage *rup;
>
> + rw_assert_wrlock(&proctreelk);
> +
> /*
> * If we got the child via a ptrace 'attach',
> * we need to give it back to the old parent.
> @@ -600,6 +580,7 @@ proc_finish_wait(struct proc *waiter, st
> proc_reparent(pr, tr);
> prsignal(tr, SIGCHLD);
> wakeup(tr);
> + rw_exit_write(&proctreelk);
> } else {
> scheduler_wait_hook(waiter, p);
> p->p_xstat = 0;
> @@ -618,6 +599,7 @@ void
> proc_reparent(struct process *child, struct process *parent)
> {
>
> + rw_assert_wrlock(&proctreelk);
> if (child->ps_pptr == parent)
> return;
>
> @@ -632,12 +614,15 @@ process_zap(struct process *pr)
> struct vnode *otvp;
> struct proc *p = pr->ps_mainproc;
>
> + rw_assert_wrlock(&proctreelk);
> +
> /*
> * Finally finished with old proc entry.
> * Unlink it from its process group and free it.
> */
> leavepgrp(pr);
> LIST_REMOVE(pr, ps_sibling);
> + rw_exit_write(&proctreelk);
>
> /*
> * Decrement the count of procs running with this uid.
> Index: kern/kern_fork.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/kern_fork.c,v
> retrieving revision 1.200
> diff -u -p -r1.200 kern_fork.c
> --- kern/kern_fork.c 27 Sep 2017 06:45:00 -0000 1.200
> +++ kern/kern_fork.c 15 Oct 2017 04:28:16 -0000
> @@ -225,7 +225,6 @@ process_new(struct proc *p, struct proce
> (caddr_t)&pr->ps_endcopy - (caddr_t)&pr->ps_startcopy);
>
> process_initialize(pr, p);
> - pr->ps_pid = allocpid();
>
> /* post-copy fixups */
> pr->ps_pptr = parent;
> @@ -270,10 +269,9 @@ process_new(struct proc *p, struct proce
> /* mark as embryo to protect against others */
> pr->ps_flags |= PS_EMBRYO;
>
> - /* Force visibility of all of the above changes */
> - membar_producer();
> -
> /* it's sufficiently inited to be globally visible */
> + rw_enter_write(&proctreelk);
> + pr->ps_pid = allocpid();
> LIST_INSERT_HEAD(&allprocess, pr, ps_list);
>
> return pr;
> @@ -374,6 +372,7 @@ fork1(struct proc *curp, int flags, void
>
> /*
> * From now on, we're committed to the fork and cannot fail.
> + * process_new() returns with proctreelk held!
> */
> p = thread_new(curp, uaddr);
> pr = process_new(p, curpr, flags);
> @@ -448,6 +447,7 @@ fork1(struct proc *curp, int flags, void
> pr->ps_ptstat->pe_other_pid = curpr->ps_pid;
> }
> }
> + rw_exit_write(&proctreelk);
>
> /*
> * For new processes, set accounting bits and mark as complete.
> Index: kern/kern_ktrace.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/kern_ktrace.c,v
> retrieving revision 1.92
> diff -u -p -r1.92 kern_ktrace.c
> --- kern/kern_ktrace.c 12 Aug 2017 00:03:10 -0000 1.92
> +++ kern/kern_ktrace.c 12 Aug 2017 00:09:06 -0000
> @@ -448,6 +448,7 @@ doktrace(struct vnode *vp, int ops, int
> /*
> * do it
> */
> + rw_enter_read(&proctreelk);
> if (pid < 0) {
> /*
> * by process group
> @@ -455,7 +456,7 @@ doktrace(struct vnode *vp, int ops, int
> pg = pgfind(-pid);
> if (pg == NULL) {
> error = ESRCH;
> - goto done;
> + goto done2;
> }
> LIST_FOREACH(pr, &pg->pg_members, ps_pglist) {
> if (descend)
> @@ -471,7 +472,7 @@ doktrace(struct vnode *vp, int ops, int
> pr = prfind(pid);
> if (pr == NULL) {
> error = ESRCH;
> - goto done;
> + goto done2;
> }
> if (descend)
> ret |= ktrsetchildren(p, pr, ops, facs, vp, cred);
> @@ -480,6 +481,8 @@ doktrace(struct vnode *vp, int ops, int
> }
> if (!ret)
> error = EPERM;
> +done2:
> + rw_exit_read(&proctreelk);
> done:
> return (error);
> }
> @@ -578,6 +581,8 @@ ktrsetchildren(struct proc *curp, struct
> {
> struct process *pr;
> int ret = 0;
> +
> + rw_assert_rdlock(&proctreelk);
>
> pr = top;
> for (;;) {
> Index: kern/kern_proc.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/kern_proc.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 kern_proc.c
> --- kern/kern_proc.c 29 Sep 2017 09:36:04 -0000 1.77
> +++ kern/kern_proc.c 15 Oct 2017 04:28:16 -0000
> @@ -40,6 +40,7 @@
> #include <sys/acct.h>
> #include <sys/wait.h>
> #include <sys/file.h>
> +#include <sys/rwlock.h>
> #include <ufs/ufs/quota.h>
> #include <sys/uio.h>
> #include <sys/malloc.h>
> @@ -48,6 +49,7 @@
> #include <sys/tty.h>
> #include <sys/signalvar.h>
> #include <sys/pool.h>
> +#include <sys/vnode.h>
>
> #define UIHASH(uid) (&uihashtbl[(uid) & uihash])
> LIST_HEAD(uihashhead, uidinfo) *uihashtbl;
> @@ -65,6 +67,8 @@ u_long pgrphash;
> struct processlist allprocess;
> struct processlist zombprocess;
> struct proclist allproc;
> +struct rwlock proctreelk;
> +struct rwlock uidinfolk;
>
> struct pool proc_pool;
> struct pool process_pool;
> @@ -73,6 +77,9 @@ struct pool ucred_pool;
> struct pool pgrp_pool;
> struct pool session_pool;
>
> +void pgdelete(struct pgrp *);
> +void fixjobc(struct process *, struct pgrp *, int);
> +
> static void orphanpg(struct pgrp *);
> #ifdef DEBUG
> void pgrpdump(void);
> @@ -87,7 +94,8 @@ procinit(void)
> LIST_INIT(&allprocess);
> LIST_INIT(&zombprocess);
> LIST_INIT(&allproc);
> -
> + rw_init(&proctreelk, "proctree");
> + rw_init(&uidinfolk, "uidinfo");
>
> tidhashtbl = hashinit(maxthread / 4, M_PROC, M_NOWAIT, &tidhash);
> pidhashtbl = hashinit(maxprocess / 4, M_PROC, M_NOWAIT, &pidhash);
> @@ -110,6 +118,10 @@ procinit(void)
> PR_WAITOK, "sessionpl", NULL);
> }
>
> +/*
> + * This returns with uidinfolk held: caller must unlock it after
> + * making whatever change they needed.
> + */
> struct uidinfo *
> uid_find(uid_t uid)
> {
> @@ -117,16 +129,20 @@ uid_find(uid_t uid)
> struct uihashhead *uipp;
>
> uipp = UIHASH(uid);
> + rw_enter_write(&uidinfolk);
> LIST_FOREACH(uip, uipp, ui_hash)
> if (uip->ui_uid == uid)
> break;
> if (uip)
> return (uip);
> + rw_exit_write(&uidinfolk);
> nuip = malloc(sizeof(*nuip), M_PROC, M_WAITOK|M_ZERO);
> + rw_enter_write(&uidinfolk);
> LIST_FOREACH(uip, uipp, ui_hash)
> if (uip->ui_uid == uid)
> break;
> if (uip) {
> + /* XXX unlock uidinfolk across the free()? */
> free(nuip, M_PROC, sizeof(*nuip));
> return (uip);
> }
> @@ -144,12 +160,14 @@ int
> chgproccnt(uid_t uid, int diff)
> {
> struct uidinfo *uip;
> + long count;
>
> uip = uid_find(uid);
> - uip->ui_proccnt += diff;
> - if (uip->ui_proccnt < 0)
> + count = (uip->ui_proccnt += diff);
> + rw_exit_write(&uidinfolk);
> + if (count < 0)
> panic("chgproccnt: procs < 0");
> - return (uip->ui_proccnt);
> + return count;
> }
>
> /*
> @@ -159,6 +177,7 @@ int
> inferior(struct process *pr, struct process *parent)
> {
>
> + rw_assert_wrlock(&proctreelk);
> for (; pr != parent; pr = pr->ps_pptr)
> if (pr->ps_pid == 0 || pr->ps_pid == 1)
> return (0);
> @@ -201,6 +220,7 @@ pgfind(pid_t pgid)
> {
> struct pgrp *pgrp;
>
> + rw_assert_anylock(&proctreelk);
> LIST_FOREACH(pgrp, PGRPHASH(pgid), pg_hash)
> if (pgrp->pg_id == pgid)
> return (pgrp);
> @@ -222,67 +242,58 @@ zombiefind(pid_t pid)
> }
>
> /*
> - * Move p to a new or existing process group (and session)
> - * Caller provides a pre-allocated pgrp and session that should
> - * be freed if they are not used.
> - * XXX need proctree lock
> + * Move process to a new process group. If a session is provided
> + * then it's a new session to contain this process group; otherwise
> + * the process is staying within its existing session.
> */
> -int
> -enterpgrp(struct process *pr, pid_t pgid, struct pgrp *newpgrp,
> - struct session *newsess)
> +void
> +enternewpgrp(struct process *pr, struct pgrp *pgrp, struct session *newsess)
> {
> - struct pgrp *pgrp = pgfind(pgid);
> + rw_assert_wrlock(&proctreelk);
>
> #ifdef DIAGNOSTIC
> - if (pgrp != NULL && newsess) /* firewalls */
> - panic("enterpgrp: setsid into non-empty pgrp");
> if (SESS_LEADER(pr))
> - panic("enterpgrp: session leader attempted setpgrp");
> + panic("%s: session leader attempted setpgrp", __func__);
> #endif
> - if (pgrp == NULL) {
> +
> + if (newsess != NULL) {
> /*
> - * new process group
> + * New session. Initialize it completely
> */
> + timeout_set(&newsess->s_verauthto, zapverauth, newsess);
> + newsess->s_leader = pr;
> + newsess->s_count = 1;
> + newsess->s_ttyvp = NULL;
> + newsess->s_ttyp = NULL;
> + memcpy(newsess->s_login, pr->ps_session->s_login,
> + sizeof(newsess->s_login));
> + atomic_clearbits_int(&pr->ps_flags, PS_CONTROLT);
> + pgrp->pg_session = newsess;
> #ifdef DIAGNOSTIC
> - if (pr->ps_pid != pgid)
> - panic("enterpgrp: new pgrp and pid != pgid");
> -#endif
> -
> - pgrp = newpgrp;
> - if (newsess) {
> - /*
> - * new session
> - */
> - newsess->s_leader = pr;
> - newsess->s_count = 1;
> - newsess->s_ttyvp = NULL;
> - newsess->s_ttyp = NULL;
> - memcpy(newsess->s_login, pr->ps_session->s_login,
> - sizeof(newsess->s_login));
> - atomic_clearbits_int(&pr->ps_flags, PS_CONTROLT);
> - pgrp->pg_session = newsess;
> -#ifdef DIAGNOSTIC
> - if (pr != curproc->p_p)
> - panic("enterpgrp: mksession but not curproc");
> + if (pr != curproc->p_p)
> + panic("%s: mksession but not curproc", __func__);
> #endif
> - } else {
> - pgrp->pg_session = pr->ps_session;
> - pgrp->pg_session->s_count++;
> - }
> - pgrp->pg_id = pgid;
> - LIST_INIT(&pgrp->pg_members);
> - LIST_INSERT_HEAD(PGRPHASH(pgid), pgrp, pg_hash);
> - pgrp->pg_jobc = 0;
> - } else if (pgrp == pr->ps_pgrp) {
> - if (newsess)
> - pool_put(&session_pool, newsess);
> - pool_put(&pgrp_pool, newpgrp);
> - return (0);
> } else {
> - if (newsess)
> - pool_put(&session_pool, newsess);
> - pool_put(&pgrp_pool, newpgrp);
> + pgrp->pg_session = pr->ps_session;
> + pgrp->pg_session->s_count++;
> }
> + pgrp->pg_id = pr->ps_pid;
> + LIST_INIT(&pgrp->pg_members);
> + LIST_INSERT_HEAD(PGRPHASH(pr->ps_pid), pgrp, pg_hash);
> + pgrp->pg_jobc = 0;
> +
> + enterthispgrp(pr, pgrp);
> +}
> +
> +/*
> + * move process to an existing process group
> + */
> +void
> +enterthispgrp(struct process *pr, struct pgrp *pgrp)
> +{
> + struct pgrp *savepgrp = pr->ps_pgrp;
> +
> + rw_assert_wrlock(&proctreelk);
>
> /*
> * Adjust eligibility of affected pgrps to participate in job control.
> @@ -290,14 +301,14 @@ enterpgrp(struct process *pr, pid_t pgid
> * could reach 0 spuriously during the first call.
> */
> fixjobc(pr, pgrp, 1);
> - fixjobc(pr, pr->ps_pgrp, 0);
> + fixjobc(pr, savepgrp, 0);
>
> LIST_REMOVE(pr, ps_pglist);
> - if (LIST_EMPTY(&pr->ps_pgrp->pg_members))
> - pgdelete(pr->ps_pgrp);
> pr->ps_pgrp = pgrp;
> +
> LIST_INSERT_HEAD(&pgrp->pg_members, pr, ps_pglist);
> - return (0);
> + if (LIST_EMPTY(&savepgrp->pg_members))
> + pgdelete(savepgrp);
> }
>
> /*
> @@ -307,6 +318,7 @@ void
> leavepgrp(struct process *pr)
> {
>
> + rw_assert_wrlock(&proctreelk);
> if (pr->ps_session->s_verauthppid == pr->ps_pid)
> zapverauth(pr->ps_session);
> LIST_REMOVE(pr, ps_pglist);
> @@ -322,6 +334,7 @@ void
> pgdelete(struct pgrp *pgrp)
> {
>
> + rw_assert_wrlock(&proctreelk);
> if (pgrp->pg_session->s_ttyp != NULL &&
> pgrp->pg_session->s_ttyp->t_pgrp == pgrp)
> pgrp->pg_session->s_ttyp->t_pgrp = NULL;
> @@ -355,6 +368,8 @@ fixjobc(struct process *pr, struct pgrp
> struct pgrp *hispgrp;
> struct session *mysession = pgrp->pg_session;
>
> + rw_assert_wrlock(&proctreelk);
> +
> /*
> * Check pr's parent to see whether pr qualifies its own process
> * group; if so, adjust count for pr's process group.
> @@ -383,6 +398,52 @@ fixjobc(struct process *pr, struct pgrp
> }
> }
>
> +void
> +killjobc(struct process *pr)
> +{
> + rw_enter_write(&proctreelk);
> + if (SESS_LEADER(pr)) {
> + struct session *sp = pr->ps_session;
> +
> + if (sp->s_ttyvp != NULL) {
> + struct vnode *ovp;
> +
> + /*
> + * Controlling process.
> + * Signal foreground pgrp,
> + * drain controlling terminal
> + * and revoke access to controlling terminal.
> + */
> + if (sp->s_ttyp->t_session == sp) {
> + if (sp->s_ttyp->t_pgrp)
> + pgsignal(sp->s_ttyp->t_pgrp,
> + SIGHUP, 1);
> + ttywait(sp->s_ttyp);
> + /*
> + * The tty could have been revoked
> + * if we blocked.
> + */
> + if (sp->s_ttyvp)
> + VOP_REVOKE(sp->s_ttyvp,
> + REVOKEALL);
> + }
> + ovp = sp->s_ttyvp;
> + sp->s_ttyvp = NULL;
> + if (ovp != NULL)
> + vrele(ovp);
> + /*
> + * s_ttyp is not zero'd; we use this to
> + * indicate that the session once had a
> + * controlling terminal. (for logging and
> + * informational purposes)
> + */
> + }
> + sp->s_leader = NULL;
> + }
> + fixjobc(pr, pr->ps_pgrp, 0);
> + rw_exit_write(&proctreelk);
> +}
> +
> /*
> * A process group has become orphaned;
> * if there are any stopped processes in the group,
> @@ -393,6 +454,7 @@ orphanpg(struct pgrp *pg)
> {
> struct process *pr;
>
> + rw_assert_wrlock(&proctreelk);
> LIST_FOREACH(pr, &pg->pg_members, ps_pglist) {
> if (pr->ps_mainproc->p_stat == SSTOP) {
> LIST_FOREACH(pr, &pg->pg_members, ps_pglist) {
> Index: kern/kern_prot.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/kern_prot.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 kern_prot.c
> --- kern/kern_prot.c 27 Sep 2017 06:45:00 -0000 1.71
> +++ kern/kern_prot.c 15 Oct 2017 04:28:16 -0000
> @@ -83,7 +83,9 @@ int
> sys_getppid(struct proc *p, void *v, register_t *retval)
> {
>
> + rw_enter_read(&proctreelk);
> *retval = p->p_p->ps_pptr->ps_pid;
> + rw_exit_read(&proctreelk);
> return (0);
> }
>
> @@ -92,7 +94,9 @@ int
> sys_getpgrp(struct proc *p, void *v, register_t *retval)
> {
>
> + rw_enter_read(&proctreelk);
> *retval = p->p_p->ps_pgrp->pg_id;
> + rw_exit_read(&proctreelk);
> return (0);
> }
>
> @@ -106,16 +110,23 @@ sys_getpgid(struct proc *curp, void *v,
> syscallarg(pid_t) pid;
> } */ *uap = v;
> struct process *targpr = curp->p_p;
> + int error = 0;
>
> - if (SCARG(uap, pid) == 0 || SCARG(uap, pid) == targpr->ps_pid)
> - goto found;
> + if (SCARG(uap, pid) == 0 || SCARG(uap, pid) == targpr->ps_pid) {
> + rw_enter_read(&proctreelk);
> + *retval = targpr->ps_pgid;
> + rw_exit_read(&proctreelk);
> + return 0;
> + }
> + rw_enter_read(&proctreelk);
> if ((targpr = prfind(SCARG(uap, pid))) == NULL)
> - return (ESRCH);
> - if (targpr->ps_session != curp->p_p->ps_session)
> - return (EPERM);
> -found:
> - *retval = targpr->ps_pgid;
> - return (0);
> + error = ESRCH;
> + else if (targpr->ps_session != curp->p_p->ps_session)
> + error = EPERM;
> + else
> + *retval = targpr->ps_pgid;
> + rw_exit_read(&proctreelk);
> + return error;
> }
>
> int
> @@ -125,7 +136,9 @@ sys_getsid(struct proc *curp, void *v, r
> syscallarg(pid_t) pid;
> } */ *uap = v;
> struct process *targpr = curp->p_p;
> + int error = 0;
>
> + rw_enter_read(&proctreelk);
> if (SCARG(uap, pid) == 0 || SCARG(uap, pid) == targpr->ps_pid)
> goto found;
> if ((targpr = prfind(SCARG(uap, pid))) == NULL)
> @@ -135,9 +148,12 @@ sys_getsid(struct proc *curp, void *v, r
> found:
> /* Skip exiting processes */
> if (targpr->ps_pgrp->pg_session->s_leader == NULL)
> - return (ESRCH);
> - *retval = targpr->ps_pgrp->pg_session->s_leader->ps_pid;
> - return (0);
> + error = ESRCH;
> + else
> + *retval = targpr->ps_pgrp->pg_session->s_leader->ps_pid;
> + rw_exit_read(&proctreelk);
> +
> + return error;
> }
>
> int
> @@ -222,15 +238,17 @@ sys_setsid(struct proc *p, void *v, regi
> pid_t pid = pr->ps_pid;
>
> newsess = pool_get(&session_pool, PR_WAITOK);
> - timeout_set(&newsess->s_verauthto, zapverauth, newsess);
> newpgrp = pool_get(&pgrp_pool, PR_WAITOK);
>
> - if (pr->ps_pgid == pid || pgfind(pid)) {
> + rw_enter_write(&proctreelk);
> + if (pr->ps_pgid == pid || pgfind(pid) != NULL) {
> + rw_exit_write(&proctreelk);
> pool_put(&pgrp_pool, newpgrp);
> pool_put(&session_pool, newsess);
> return (EPERM);
> } else {
> - (void) enterpgrp(pr, pid, newpgrp, newsess);
> + enternewpgrp(pr, newpgrp, newsess);
> + rw_exit_write(&proctreelk);
> *retval = pid;
> return (0);
> }
> @@ -270,6 +288,7 @@ sys_setpgid(struct proc *curp, void *v,
>
> newpgrp = pool_get(&pgrp_pool, PR_WAITOK);
>
> + rw_enter_write(&proctreelk);
> if (pid != 0 && pid != curpr->ps_pid) {
> if ((targpr = prfind(pid)) == 0 || !inferior(targpr, curpr)) {
> error = ESRCH;
> @@ -291,15 +310,27 @@ sys_setpgid(struct proc *curp, void *v,
> }
> if (pgid == 0)
> pgid = targpr->ps_pid;
> - else if (pgid != targpr->ps_pid)
> - if ((pgrp = pgfind(pgid)) == 0 ||
> - pgrp->pg_session != curpr->ps_session) {
> +
> + error = 0;
> + if ((pgrp = pgfind(pgid)) == NULL) {
> + /* can only create a new process group with pgid == pid */
> + if (pgid != targpr->ps_pid)
> error = EPERM;
> - goto out;
> + else {
> + enternewpgrp(targpr, newpgrp, NULL);
> + newpgrp = NULL;
> }
> - return (enterpgrp(targpr, pgid, newpgrp, NULL));
> + } else if (pgrp != targpr->ps_pgrp) { /* anything to do? */
> + if (pgid != targpr->ps_pid &&
> + pgrp->pg_session != curpr->ps_session)
> + error = EPERM;
> + else
> + enterthispgrp(targpr, pgrp);
> + }
> out:
> - pool_put(&pgrp_pool, newpgrp);
> + rw_exit_write(&proctreelk);
> + if (newpgrp != NULL)
> + pool_put(&pgrp_pool, newpgrp);
> return (error);
> }
>
> Index: kern/kern_resource.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/kern_resource.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 kern_resource.c
> --- kern/kern_resource.c 15 Sep 2016 02:00:16 -0000 1.57
> +++ kern/kern_resource.c 29 Jul 2017 22:26:00 -0000
> @@ -90,13 +90,17 @@ sys_getpriority(struct proc *curp, void
> case PRIO_PGRP: {
> struct pgrp *pg;
>
> + rw_enter_read(&proctreelk);
> if (SCARG(uap, who) == 0)
> pg = curp->p_p->ps_pgrp;
> - else if ((pg = pgfind(SCARG(uap, who))) == NULL)
> + else if ((pg = pgfind(SCARG(uap, who))) == NULL) {
> + rw_exit_read(&proctreelk);
> break;
> + }
> LIST_FOREACH(pr, &pg->pg_members, ps_pglist)
> if (pr->ps_nice < low)
> low = pr->ps_nice;
> + rw_exit_read(&proctreelk);
> break;
> }
>
> @@ -145,14 +149,18 @@ sys_setpriority(struct proc *curp, void
> case PRIO_PGRP: {
> struct pgrp *pg;
>
> + rw_enter_read(&proctreelk);
> if (SCARG(uap, who) == 0)
> pg = curp->p_p->ps_pgrp;
> - else if ((pg = pgfind(SCARG(uap, who))) == NULL)
> + else if ((pg = pgfind(SCARG(uap, who))) == NULL) {
> + rw_exit_read(&proctreelk);
> break;
> + }
> LIST_FOREACH(pr, &pg->pg_members, ps_pglist) {
> error = donice(curp, pr, SCARG(uap, prio));
> found++;
> }
> + rw_exit_read(&proctreelk);
> break;
> }
>
> Index: kern/kern_sig.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/kern_sig.c,v
> retrieving revision 1.212
> diff -u -p -r1.212 kern_sig.c
> --- kern/kern_sig.c 8 Jun 2017 17:14:02 -0000 1.212
> +++ kern/kern_sig.c 31 Jul 2017 21:39:34 -0000
> @@ -661,6 +661,7 @@ killpg1(struct proc *cp, int signum, int
> prsignal(pr, signum);
> }
> } else {
> + rw_enter_read(&proctreelk);
> if (pgid == 0)
> /*
> * zero pgid means send to my process group.
> @@ -668,8 +669,10 @@ killpg1(struct proc *cp, int signum, int
> pgrp = cp->p_p->ps_pgrp;
> else {
> pgrp = pgfind(pgid);
> - if (pgrp == NULL)
> + if (pgrp == NULL) {
> + rw_exit_read(&proctreelk);
> return (ESRCH);
> + }
> }
> LIST_FOREACH(pr, &pgrp->pg_members, ps_pglist) {
> if (pr->ps_pid <= 1 || pr->ps_flags & PS_SYSTEM ||
> @@ -679,6 +682,7 @@ killpg1(struct proc *cp, int signum, int
> if (signum)
> prsignal(pr, signum);
> }
> + rw_exit_read(&proctreelk);
> }
> return (nfound ? 0 : ESRCH);
> }
> @@ -1314,6 +1318,7 @@ proc_stop_sweep(void *v)
> {
> struct process *pr;
>
> + rw_enter_read(&proctreelk);
> LIST_FOREACH(pr, &allprocess, ps_list) {
> if ((pr->ps_flags & PS_STOPPED) == 0)
> continue;
> @@ -1323,6 +1328,7 @@ proc_stop_sweep(void *v)
> prsignal(pr->ps_pptr, SIGCHLD);
> wakeup(pr->ps_pptr);
> }
> + rw_exit_read(&proctreelk);
> }
>
> /*
> Index: kern/kern_sysctl.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/kern_sysctl.c,v
> retrieving revision 1.330
> diff -u -p -r1.330 kern_sysctl.c
> --- kern/kern_sysctl.c 11 Aug 2017 21:24:19 -0000 1.330
> +++ kern/kern_sysctl.c 12 Aug 2017 00:08:14 -0000
> @@ -1506,6 +1506,7 @@ again:
> break;
>
> case KERN_PROC_TTY:
> + /* XXX proctreelk? */
> if ((pr->ps_flags & PS_CONTROLT) == 0 ||
> pr->ps_session->s_ttyp == NULL ||
> pr->ps_session->s_ttyp->t_dev != (dev_t)arg)
> @@ -1612,6 +1613,7 @@ fill_kproc(struct process *pr, struct ki
> if (s->s_leader)
> ki->p_sid = s->s_leader->ps_pid;
>
> + /* XXX proctreelk */
> if ((pr->ps_flags & PS_CONTROLT) && (tp = s->s_ttyp)) {
> ki->p_tdev = tp->t_dev;
> ki->p_tpgid = tp->t_pgrp ? tp->t_pgrp->pg_id : -1;
> Index: kern/subr_prf.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/subr_prf.c,v
> retrieving revision 1.91
> diff -u -p -r1.91 subr_prf.c
> --- kern/subr_prf.c 30 Apr 2017 16:45:46 -0000 1.91
> +++ kern/subr_prf.c 29 Jul 2017 21:15:31 -0000
> @@ -362,6 +362,7 @@ uprintf(const char *fmt, ...)
> struct process *pr = curproc->p_p;
> va_list ap;
>
> + /* XXX proctreelk? */
> if (pr->ps_flags & PS_CONTROLT && pr->ps_session->s_ttyvp) {
> va_start(ap, fmt);
> kprintf(fmt, TOTTY, pr->ps_session->s_ttyp, NULL, ap);
> Index: kern/sys_process.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/sys_process.c,v
> retrieving revision 1.78
> diff -u -p -r1.78 sys_process.c
> --- kern/sys_process.c 14 Oct 2017 10:17:08 -0000 1.78
> +++ kern/sys_process.c 15 Oct 2017 04:30:18 -0000
> @@ -276,8 +276,13 @@ ptrace_ctrl(struct proc *p, int req, pid
> struct proc *t; /* target thread */
> struct process *tr; /* target process */
> int error = 0;
> + int proctree_locked;
> int s;
>
> + /* Lock proctree before looking up the process. */
> + rw_enter_write(&proctreelk);
> + proctree_locked = 1;
> +
> switch (req) {
> case PT_TRACE_ME:
> /* Just set the trace flag. */
> @@ -288,6 +293,7 @@ ptrace_ctrl(struct proc *p, int req, pid
> tr->ps_ptstat = malloc(sizeof(*tr->ps_ptstat),
> M_SUBPROC, M_WAITOK);
> memset(tr->ps_ptstat, 0, sizeof(*tr->ps_ptstat));
> + rw_exit_write(&proctreelk);
> return 0;
>
> /* calls that only operate on the PID */
> @@ -295,8 +301,10 @@ ptrace_ctrl(struct proc *p, int req, pid
> case PT_ATTACH:
> case PT_DETACH:
> /* Find the process we're supposed to be operating on. */
> - if ((tr = prfind(pid)) == NULL)
> - return (ESRCH);
> + if ((tr = prfind(pid)) == NULL) {
> + error = ESRCH;
> + goto fail;
> + }
> t = TAILQ_FIRST(&tr->ps_threads);
> break;
>
> @@ -305,20 +313,24 @@ ptrace_ctrl(struct proc *p, int req, pid
> #ifdef PT_STEP
> case PT_STEP:
> #endif
> - if ((tr = process_tprfind(pid, &t)) == NULL)
> - return ESRCH;
> + if ((tr = process_tprfind(pid, &t)) == NULL) {
> + error = ESRCH;
> + goto fail;
> + }
> break;
> }
>
> /* Check permissions/state */
> if (req != PT_ATTACH) {
> /* Check that the data is a valid signal number or zero. */
> - if (req != PT_KILL && (data < 0 || data >= NSIG))
> - return EINVAL;
> + if (req != PT_KILL && (data < 0 || data >= NSIG)) {
> + error = EINVAL;
> + goto fail;
> + }
>
> /* Most operations require the target to already be traced */
> if ((error = process_checktracestate(p->p_p, tr, t)))
> - return error;
> + goto fail;
>
> /* Do single-step fixup if needed. */
> FIX_SSTEP(t);
> @@ -327,26 +339,34 @@ ptrace_ctrl(struct proc *p, int req, pid
> * PT_ATTACH is the opposite; you can't attach to a process if:
> * (1) it's the process that's doing the attaching,
> */
> - if (tr == p->p_p)
> - return (EINVAL);
> + if (tr == p->p_p) {
> + error = EINVAL;
> + goto fail;
> + }
>
> /*
> * (2) it's a system process
> */
> - if (ISSET(tr->ps_flags, PS_SYSTEM))
> - return (EPERM);
> + if (ISSET(tr->ps_flags, PS_SYSTEM)) {
> + error = EPERM;
> + goto fail;
> + }
>
> /*
> * (3) it's already being traced, or
> */
> - if (ISSET(tr->ps_flags, PS_TRACED))
> - return (EBUSY);
> + if (ISSET(tr->ps_flags, PS_TRACED)) {
> + error = EBUSY;
> + goto fail;
> + }
>
> /*
> * (4) it's in the middle of execve(2)
> */
> - if (ISSET(tr->ps_flags, PS_INEXEC))
> - return (EAGAIN);
> + if (ISSET(tr->ps_flags, PS_INEXEC)) {
> + error = EAGAIN;
> + goto fail;
> + }
>
> /*
> * (5) it's not owned by you, or the last exec
> @@ -362,14 +382,14 @@ ptrace_ctrl(struct proc *p, int req, pid
> if ((tr->ps_ucred->cr_ruid != p->p_ucred->cr_ruid ||
> ISSET(tr->ps_flags, PS_SUGIDEXEC | PS_SUGID)) &&
> (error = suser(p, 0)) != 0)
> - return (error);
> + goto fail;
>
> /*
> * (5.5) it's not a child of the tracing process.
> */
> if (global_ptrace == 0 && !inferior(tr, p->p_p) &&
> (error = suser(p, 0)) != 0)
> - return (error);
> + goto fail;
>
> /*
> * (6) ...it's init, which controls the security level
> @@ -377,16 +397,20 @@ ptrace_ctrl(struct proc *p, int req, pid
> * compiled with permanently insecure mode turned
> * on.
> */
> - if ((tr->ps_pid == 1) && (securelevel > -1))
> - return (EPERM);
> + if ((tr->ps_pid == 1) && (securelevel > -1)) {
> + error = EPERM;
> + goto fail;
> + }
>
> /*
> * (7) it's an ancestor of the current process and
> * not init (because that would create a loop in
> * the process graph).
> */
> - if (tr->ps_pid != 1 && inferior(p->p_p, tr))
> - return (EINVAL);
> + if (tr->ps_pid != 1 && inferior(p->p_p, tr)) {
> + error = EINVAL;
> + goto fail;
> + }
> }
>
> switch (req) {
> @@ -419,7 +443,7 @@ ptrace_ctrl(struct proc *p, int req, pid
> /* If the address parameter is not (int *)1, set the pc. */
> if ((int *)addr != (int *)1)
> if ((error = process_set_pc(t, addr)) != 0)
> - return error;
> + goto fail;
>
> #ifdef PT_STEP
> /*
> @@ -427,7 +451,7 @@ ptrace_ctrl(struct proc *p, int req, pid
> */
> error = process_sstep(t, req == PT_STEP);
> if (error)
> - return error;
> + goto fail;
> #endif
> goto sendsig;
>
> @@ -453,7 +477,7 @@ ptrace_ctrl(struct proc *p, int req, pid
> */
> error = process_sstep(t, 0);
> if (error)
> - return error;
> + goto fail;
> #endif
>
> /* give process back to original parent or init */
> @@ -469,6 +493,10 @@ ptrace_ctrl(struct proc *p, int req, pid
> atomic_clearbits_int(&tr->ps_flags, PS_TRACED|PS_WAITED);
>
> sendsig:
> + KASSERT(proctree_locked);
> + proctree_locked = 0;
> + rw_exit_write(&proctreelk);
> +
> memset(tr->ps_ptstat, 0, sizeof(*tr->ps_ptstat));
>
> /* Finally, deliver the requested signal (or none). */
> @@ -515,6 +543,9 @@ ptrace_ctrl(struct proc *p, int req, pid
> break;
> }
>
> +fail:
> + if (proctree_locked)
> + rw_exit_write(&proctreelk);
> return error;
> }
>
> Index: kern/syscalls.master
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/syscalls.master,v
> retrieving revision 1.178
> diff -u -p -r1.178 syscalls.master
> --- kern/syscalls.master 25 Sep 2017 23:00:33 -0000 1.178
> +++ kern/syscalls.master 15 Oct 2017 04:28:16 -0000
> @@ -107,7 +107,7 @@
> 36 STD { void sys_sync(void); }
> 37 OBSOL o58_kill
> 38 STD { int sys_stat(const char *path, struct stat *ub); }
> -39 STD { pid_t sys_getppid(void); }
> +39 STD NOLOCK { pid_t sys_getppid(void); }
> 40 STD { int sys_lstat(const char *path, struct stat *ub); }
> 41 STD { int sys_dup(int fd); }
> 42 STD { int sys_fstatat(int fd, const char *path, \
> @@ -185,7 +185,7 @@
> gid_t *gidset); }
> 80 STD { int sys_setgroups(int gidsetsize, \
> const gid_t *gidset); }
> -81 STD { int sys_getpgrp(void); }
> +81 STD NOLOCK { int sys_getpgrp(void); }
> 82 STD { int sys_setpgid(pid_t pid, pid_t pgid); }
> 83 STD NOLOCK { int sys_futex(uint32_t *f, int op, int val, \
> const struct timespec *timeout, uint32_t *g); }
> Index: kern/tty.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/tty.c,v
> retrieving revision 1.134
> diff -u -p -r1.134 tty.c
> --- kern/tty.c 29 Jun 2017 04:10:07 -0000 1.134
> +++ kern/tty.c 10 Aug 2017 14:23:06 -0000
> @@ -973,10 +973,13 @@ ttioctl(struct tty *tp, u_long cmd, cadd
> break;
> case TIOCSCTTY: /* become controlling tty */
> /* Session ctty vnode pointer set in vnode layer. */
> + rw_enter_write(&proctreelk);
> if (!SESS_LEADER(pr) ||
> ((pr->ps_session->s_ttyvp || tp->t_session) &&
> - (tp->t_session != pr->ps_session)))
> + (tp->t_session != pr->ps_session))) {
> + rw_exit_write(&proctreelk);
> return (EPERM);
> + }
> if (tp->t_session)
> SESSRELE(tp->t_session);
> SESSHOLD(pr->ps_session);
> @@ -984,10 +987,14 @@ ttioctl(struct tty *tp, u_long cmd, cadd
> tp->t_pgrp = pr->ps_pgrp;
> pr->ps_session->s_ttyp = tp;
> atomic_setbits_int(&pr->ps_flags, PS_CONTROLT);
> + rw_exit_write(&proctreelk);
> break;
> case TIOCSPGRP: { /* set pgrp of tty */
> - struct pgrp *pgrp = pgfind(*(int *)data);
> + struct pgrp *pgrp;
>
> + rw_enter_write(&proctreelk);
> + pgrp = pgfind(*(int *)data);
> + rw_exit_write(&proctreelk);
> if (!isctty(pr, tp))
> return (ENOTTY);
> else if (pgrp == NULL)
> @@ -2125,6 +2132,7 @@ process_sum(struct process *pr, fixpt_t
>
> /*
> * Report on state of foreground process group.
> + * XXX needs proctreelk?
> */
> void
> ttyinfo(struct tty *tp)
> Index: kern/vfs_lockf.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/kern/vfs_lockf.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 vfs_lockf.c
> --- kern/vfs_lockf.c 7 Nov 2016 00:26:33 -0000 1.24
> +++ kern/vfs_lockf.c 29 Jul 2017 21:08:40 -0000
> @@ -106,9 +106,12 @@ lf_alloc(uid_t uid, int allowfail)
>
> uip = uid_find(uid);
> if (uid && allowfail && uip->ui_lockcnt >
> - (allowfail == 1 ? maxlocksperuid : (maxlocksperuid * 2)))
> + (allowfail == 1 ? maxlocksperuid : (maxlocksperuid * 2))) {
> + rw_exit_write(&uidinfolk);
> return (NULL);
> + }
> uip->ui_lockcnt++;
> + rw_exit_write(&uidinfolk);
> lock = pool_get(&lockfpool, PR_WAITOK);
> lock->lf_uid = uid;
> return (lock);
> @@ -121,6 +124,7 @@ lf_free(struct lockf *lock)
>
> uip = uid_find(lock->lf_uid);
> uip->ui_lockcnt--;
> + rw_exit_write(&uidinfolk);
> pool_put(&lockfpool, lock);
> }
>
>
