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;
}
