From fa3935bcead67c76f79255e5f4834296cefc87e4 Mon Sep 17 00:00:00 2001
From: Mikolaj Golub <trociny@FreeBSD.org>
Date: Sat, 17 Dec 2011 16:59:22 +0000
Subject: [PATCH] On start most of sysctl_kern_proc functions use the same
 pattern: locate a process calling pfind() and do some additional checks like
 p_candebug(). To reduce this code duplication a new function pget() is
 introduced and used.

As the function may be useful not only in kern_proc.c it is in the
kernel name space.

Suggested by:	kib
Reviewed by:	kib
MFC after:	2 weeks
---
 sys/kern/kern_proc.c | 192 ++++++++++++++++++-------------------------
 sys/sys/proc.h       |  14 ++++
 2 files changed, 94 insertions(+), 112 deletions(-)

diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c
index 83da94eeb74..b7986410b33 100644
--- a/sys/kern/kern_proc.c
+++ b/sys/kern/kern_proc.c
@@ -329,6 +329,55 @@ pgfind(pgid)
 	return (NULL);
 }
 
+/*
+ * Locate process and do additional manipulations, depending on flags.
+ */
+int
+pget(pid_t pid, int flags, struct proc **pp)
+{
+	struct proc *p;
+	int error;
+
+	p = pfind(pid);
+	if (p == NULL)
+		return (ESRCH);
+	if ((flags & PGET_CANSEE) != 0) {
+		error = p_cansee(curthread, p);
+		if (error != 0)
+			goto errout;
+	}
+	if ((flags & PGET_CANDEBUG) != 0) {
+		error = p_candebug(curthread, p);
+		if (error != 0)
+			goto errout;
+	}
+	if ((flags & PGET_ISCURRENT) != 0 && curproc != p) {
+		error = EPERM;
+		goto errout;
+	}
+	if ((flags & PGET_NOTWEXIT) != 0 && (p->p_flag & P_WEXIT) != 0) {
+		error = ESRCH;
+		goto errout;
+	}
+	if ((flags & PGET_NOTINEXEC) != 0 && (p->p_flag & P_INEXEC) != 0) {
+		/*
+		 * XXXRW: Not clear ESRCH is the right error during proc
+		 * execve().
+		 */
+		error = ESRCH;
+		goto errout;
+	}
+	if ((flags & PGET_HOLD) != 0) {
+		_PHOLD(p);
+		PROC_UNLOCK(p);
+	}
+	*pp = p;
+	return (0);
+errout:
+	PROC_UNLOCK(p);
+	return (error);
+}
+
 /*
  * Create a new process group.
  * pgid must be equal to the pid of p.
@@ -1177,13 +1226,9 @@ sysctl_kern_proc(SYSCTL_HANDLER_ARGS)
 		error = sysctl_wire_old_buffer(req, 0);
 		if (error)
 			return (error);		
-		p = pfind((pid_t)name[0]);
-		if (!p)
-			return (ESRCH);
-		if ((error = p_cansee(curthread, p))) {
-			PROC_UNLOCK(p);
+		error = pget((pid_t)name[0], PGET_CANSEE, &p);
+		if (error != 0)
 			return (error);
-		}
 		error = sysctl_out_proc(p, req, flags);
 		return (error);
 	}
@@ -1661,24 +1706,17 @@ sysctl_kern_proc_args(SYSCTL_HANDLER_ARGS)
 	struct pargs *newpa, *pa;
 	struct proc *p;
 	struct sbuf sb;
-	int error = 0, error2;
+	int flags, error = 0, error2;
 
 	if (namelen != 1) 
 		return (EINVAL);
 
-	p = pfind((pid_t)name[0]);
-	if (!p)
-		return (ESRCH);
-
-	if ((error = p_cansee(curthread, p)) != 0) {
-		PROC_UNLOCK(p);
+	flags = PGET_CANSEE;
+	if (req->newptr != NULL)
+		flags |= PGET_ISCURRENT;
+	error = pget((pid_t)name[0], flags, &p);
+	if (error)
 		return (error);
-	}
-
-	if (req->newptr && curproc != p) {
-		PROC_UNLOCK(p);
-		return (EPERM);
-	}
 
 	pa = p->p_args;
 	if (pa != NULL) {
@@ -1733,23 +1771,14 @@ sysctl_kern_proc_env(SYSCTL_HANDLER_ARGS)
 	if (namelen != 1)
 		return (EINVAL);
 
-	p = pfind((pid_t)name[0]);
-	if (p == NULL)
-		return (ESRCH);
-	if ((p->p_flag & P_WEXIT) != 0) {
-		PROC_UNLOCK(p);
-		return (ESRCH);
-	}
-	if ((error = p_candebug(curthread, p)) != 0) {
-		PROC_UNLOCK(p);
+	error = pget((pid_t)name[0], PGET_WANTREAD, &p);
+	if (error != 0)
 		return (error);
-	}
 	if ((p->p_flag & P_SYSTEM) != 0) {
-		PROC_UNLOCK(p);
+		PRELE(p);
 		return (0);
 	}
-	_PHOLD(p);
-	PROC_UNLOCK(p);
+
 	sbuf_new_for_sysctl(&sb, NULL, GET_PS_STRINGS_CHUNK_SZ, req);
 	error = proc_getenvv(curthread, p, &sb, req->oldlen);
 	error2 = sbuf_finish(&sb);
@@ -1775,24 +1804,13 @@ sysctl_kern_proc_auxv(SYSCTL_HANDLER_ARGS)
 	if (namelen != 1)
 		return (EINVAL);
 
-	p = pfind((pid_t)name[0]);
-	if (p == NULL)
-		return (ESRCH);
-	if (p->p_flag & P_WEXIT) {
-		PROC_UNLOCK(p);
-		return (ESRCH);
-	}
-	error = p_candebug(curthread, p);
-	if (error != 0) {
-		PROC_UNLOCK(p);
+	error = pget((pid_t)name[0], PGET_WANTREAD, &p);
+	if (error != 0)
 		return (error);
-	}
 	if ((p->p_flag & P_SYSTEM) != 0) {
-		PROC_UNLOCK(p);
+		PRELE(p);
 		return (0);
 	}
-	_PHOLD(p);
-	PROC_UNLOCK(p);
 	error = get_proc_vector(curthread, p, &auxv, &vsize, PROC_AUX);
 	if (error == 0) {
 #ifdef COMPAT_FREEBSD32
@@ -1829,13 +1847,9 @@ sysctl_kern_proc_pathname(SYSCTL_HANDLER_ARGS)
 	if (*pidp == -1) {	/* -1 means this process */
 		p = req->td->td_proc;
 	} else {
-		p = pfind(*pidp);
-		if (p == NULL)
-			return (ESRCH);
-		if ((error = p_cansee(curthread, p)) != 0) {
-			PROC_UNLOCK(p);
+		error = pget(*pidp, PGET_CANSEE, &p);
+		if (error != 0)
 			return (error);
-		}
 	}
 
 	vp = p->p_textvp;
@@ -1872,12 +1886,9 @@ sysctl_kern_proc_sv_name(SYSCTL_HANDLER_ARGS)
 		return (EINVAL);
 
 	name = (int *)arg1;
-	if ((p = pfind((pid_t)name[0])) == NULL)
-		return (ESRCH);
-	if ((error = p_cansee(curthread, p))) {
-		PROC_UNLOCK(p);
+	error = pget((pid_t)name[0], PGET_CANSEE, &p);
+	if (error != 0)
 		return (error);
-	}
 	sv_name = p->p_sysent->sv_name;
 	PROC_UNLOCK(p);
 	return (sysctl_handle_string(oidp, sv_name, 0, req));
@@ -1904,18 +1915,9 @@ sysctl_kern_proc_ovmmap(SYSCTL_HANDLER_ARGS)
 	struct vmspace *vm;
 
 	name = (int *)arg1;
-	if ((p = pfind((pid_t)name[0])) == NULL)
-		return (ESRCH);
-	if (p->p_flag & P_WEXIT) {
-		PROC_UNLOCK(p);
-		return (ESRCH);
-	}
-	if ((error = p_candebug(curthread, p))) {
-		PROC_UNLOCK(p);
+	error = pget((pid_t)name[0], PGET_WANTREAD, &p);
+	if (error != 0)
 		return (error);
-	}
-	_PHOLD(p);
-	PROC_UNLOCK(p);
 	vm = vmspace_acquire_ref(p);
 	if (vm == NULL) {
 		PRELE(p);
@@ -2082,18 +2084,9 @@ sysctl_kern_proc_vmmap(SYSCTL_HANDLER_ARGS)
 	vm_map_t map;
 
 	name = (int *)arg1;
-	if ((p = pfind((pid_t)name[0])) == NULL)
-		return (ESRCH);
-	if (p->p_flag & P_WEXIT) {
-		PROC_UNLOCK(p);
-		return (ESRCH);
-	}
-	if ((error = p_candebug(curthread, p))) {
-		PROC_UNLOCK(p);
+	error = pget((pid_t)name[0], PGET_WANTREAD, &p);
+	if (error != 0)
 		return (error);
-	}
-	_PHOLD(p);
-	PROC_UNLOCK(p);
 	vm = vmspace_acquire_ref(p);
 	if (vm == NULL) {
 		PRELE(p);
@@ -2267,19 +2260,9 @@ sysctl_kern_proc_kstack(SYSCTL_HANDLER_ARGS)
 	struct proc *p;
 
 	name = (int *)arg1;
-	if ((p = pfind((pid_t)name[0])) == NULL)
-		return (ESRCH);
-	/* XXXRW: Not clear ESRCH is the right error during proc execve(). */
-	if (p->p_flag & P_WEXIT || p->p_flag & P_INEXEC) {
-		PROC_UNLOCK(p);
-		return (ESRCH);
-	}
-	if ((error = p_candebug(curthread, p))) {
-		PROC_UNLOCK(p);
+	error = pget((pid_t)name[0], PGET_NOTINEXEC | PGET_WANTREAD, &p);
+	if (error != 0)
 		return (error);
-	}
-	_PHOLD(p);
-	PROC_UNLOCK(p);
 
 	kkstp = malloc(sizeof(*kkstp), M_TEMP, M_WAITOK);
 	st = stack_create();
@@ -2374,13 +2357,9 @@ sysctl_kern_proc_groups(SYSCTL_HANDLER_ARGS)
 	if (*pidp == -1) {	/* -1 means this process */
 		p = req->td->td_proc;
 	} else {
-		p = pfind(*pidp);
-		if (p == NULL)
-			return (ESRCH);
-		if ((error = p_cansee(curthread, p)) != 0) {
-			PROC_UNLOCK(p);
+		error = pget(*pidp, PGET_CANSEE, &p);
+		if (error != 0)
 			return (error);
-		}
 	}
 
 	cred = crhold(p->p_ucred);
@@ -2409,15 +2388,9 @@ sysctl_kern_proc_rlimit(SYSCTL_HANDLER_ARGS)
 	if (namelen != 1)
 		return (EINVAL);
 
-	p = pfind((pid_t)name[0]);
-	if (p == NULL)
-		return (ESRCH);
-
-	if ((error = p_cansee(curthread, p)) != 0) {
-		PROC_UNLOCK(p);
+	error = pget((pid_t)name[0], PGET_CANSEE, &p);
+	if (error != 0)
 		return (error);
-	}
-
 	/*
 	 * Check the request size.  We alow sizes smaller rlimit array for
 	 * backward binary compatibility: the number of resource limits may
@@ -2454,14 +2427,9 @@ sysctl_kern_proc_ps_strings(SYSCTL_HANDLER_ARGS)
 	if (namelen != 1)
 		return (EINVAL);
 
-	p = pfind((pid_t)name[0]);
-	if (p == NULL)
-		return (ESRCH);
-	error = p_candebug(curthread, p);
-	if (error != 0) {
-		PROC_UNLOCK(p);
+	error = pget((pid_t)name[0], PGET_CANDEBUG, &p);
+	if (error != 0)
 		return (error);
-	}
 #ifdef COMPAT_FREEBSD32
 	if ((req->flags & SCTL_MASK32) != 0) {
 		/*
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index efb5889957e..a0d942c7423 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -818,6 +818,20 @@ struct	proc *pfind(pid_t);		/* Find process by id. */
 struct	pgrp *pgfind(pid_t);		/* Find process group by id. */
 struct	proc *zpfind(pid_t);		/* Find zombie process by id. */
 
+/*
+ * pget() flags.
+ */
+#define	PGET_HOLD	0x00001	/* Hold the process. */
+#define	PGET_CANSEE	0x00002	/* Check against p_cansee(). */
+#define	PGET_CANDEBUG	0x00004	/* Check against p_candebug(). */
+#define	PGET_ISCURRENT	0x00008	/* Check that the found process is current. */
+#define	PGET_NOTWEXIT	0x00010	/* Check that the process is not in P_WEXIT. */
+#define	PGET_NOTINEXEC	0x00020	/* Check that the process is not in P_INEXEC. */
+
+#define	PGET_WANTREAD	(PGET_HOLD | PGET_CANDEBUG | PGET_NOTWEXIT)
+
+int	pget(pid_t pid, int flags, struct proc **pp);
+
 void	ast(struct trapframe *framep);
 struct	thread *choosethread(void);
 int	cr_cansignal(struct ucred *cred, struct proc *proc, int signum);