Diff below introduce multiple 'goto fail' in ptrace_ctrl(). It is extracted from guenther@'s proctreelk diff because it doesn't introduce any change in behavior. I'd like to get it in to shrink the locking diff.
ok? Index: kern/sys_process.c =================================================================== RCS file: /cvs/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 9 Feb 2018 20:37:58 -0000 @@ -295,8 +295,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 +307,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 +333,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 +376,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 +391,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 +437,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 +445,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 +471,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 */ @@ -515,6 +533,7 @@ ptrace_ctrl(struct proc *p, int req, pid break; } +fail: return error; }