1) pfs_mode should be mode_t, not u_short

2) Fix procfs security hole.  This is basically the NetBSD fix
   (which is based in part on Sean Eric Fagan's FreeBSD fix)
   with a few minor changes.  The change creates as function,
   procfs_checkioperm(), that has check numbers 3 and 4 from sys_ptrace().
3) Make procfs_control() use procfs_checkioperm() as well (it
   already had the same basic checks but some of them were incorrect).
4) Minor stylistic cleanups, including the use of the SET/CLR/ISSET
   macros to aid general grokability.

7 files changed, 81 insertions, 47 deletions

diff --git a/sys/miscfs/procfs/procfs.h b/sys/miscfs/procfs/procfs.h
index 3a69cf3fd25..a312f7abd47 100644
--- a/sys/miscfs/procfs/procfs.h
+++ b/sys/miscfs/procfs/procfs.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: procfs.h,v 1.3 1996/06/21 12:49:54 mickey Exp $	*/
+/*	$OpenBSD: procfs.h,v 1.4 1997/08/16 02:00:47 millert Exp $	*/
 /*	$NetBSD: procfs.h,v 1.17 1996/02/12 15:01:41 christos Exp $	*/
 
 /*
@@ -65,7 +65,7 @@ struct pfsnode {
 	struct vnode	*pfs_vnode;	/* vnode associated with this pfsnode */
 	pfstype		pfs_type;	/* type of procfs node */
 	pid_t		pfs_pid;	/* associated process */
-	u_short		pfs_mode;	/* mode bits for stat() */
+	mode_t		pfs_mode;	/* mode bits for stat() */
 	u_long		pfs_flags;	/* open flags */
 	u_long		pfs_fileno;	/* unique file id */
 };
@@ -104,15 +104,16 @@ int vfs_getuserstr __P((struct uio *, char *, int *));
 vfs_namemap_t *vfs_findname __P((vfs_namemap_t *, char *, int));
 
 #define PFIND(pid) ((pid) ? pfind(pid) : &proc0)
-int procfs_freevp __P((struct vnode *));
 int procfs_allocvp __P((struct mount *, struct vnode **, long, pfstype));
-struct vnode *procfs_findtextvp __P((struct proc *));
-int procfs_donote __P((struct proc *, struct proc *, struct pfsnode *pfsp, struct uio *uio));
-int procfs_doregs __P((struct proc *, struct proc *, struct pfsnode *pfsp, struct uio *uio));
+int procfs_checkioperm __P((struct proc *t, struct proc *p));
+int procfs_doctl __P((struct proc *, struct proc *, struct pfsnode *pfsp, struct uio *uio));
 int procfs_dofpregs __P((struct proc *, struct proc *, struct pfsnode *pfsp, struct uio *uio));
 int procfs_domem __P((struct proc *, struct proc *, struct pfsnode *pfsp, struct uio *uio));
-int procfs_doctl __P((struct proc *, struct proc *, struct pfsnode *pfsp, struct uio *uio));
+int procfs_donote __P((struct proc *, struct proc *, struct pfsnode *pfsp, struct uio *uio));
+int procfs_doregs __P((struct proc *, struct proc *, struct pfsnode *pfsp, struct uio *uio));
 int procfs_dostatus __P((struct proc *, struct proc *, struct pfsnode *pfsp, struct uio *uio));
+struct vnode *procfs_findtextvp __P((struct proc *));
+int procfs_freevp __P((struct vnode *));
 
 /* functions to check whether or not files should be displayed */
 int procfs_validfile __P((struct proc *));
diff --git a/sys/miscfs/procfs/procfs_ctl.c b/sys/miscfs/procfs/procfs_ctl.c
index 346b2edc704..559ccd1e17d 100644
--- a/sys/miscfs/procfs/procfs_ctl.c
+++ b/sys/miscfs/procfs/procfs_ctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: procfs_ctl.c,v 1.5 1996/10/14 09:27:54 deraadt Exp $	*/
+/*	$OpenBSD: procfs_ctl.c,v 1.6 1997/08/16 02:00:48 millert Exp $	*/
 /*	$NetBSD: procfs_ctl.c,v 1.14 1996/02/09 22:40:48 christos Exp $	*/
 
 /*
@@ -61,7 +61,7 @@
 #define TRACE_WAIT_P(curp, p) \
 	((p)->p_stat == SSTOP && \
 	 (p)->p_pptr == (curp) && \
-	 ((p)->p_flag & P_TRACED))
+	 ISSET((p)->p_flag, P_TRACED))
 
 #define PROCFS_CTL_ATTACH	1
 #define PROCFS_CTL_DETACH	2
@@ -120,32 +120,18 @@ procfs_control(curp, p, op)
 	 * by the calling process.
 	 */
 	if (op == PROCFS_CTL_ATTACH) {
-		/* can't trace yourself! */
+		/* Can't trace yourself! */
 		if (p->p_pid == curp->p_pid)
 			return (EINVAL);
 
-		/* check whether already being traced */
-		if (p->p_flag & P_TRACED)
+		/* Check whether already being traced. */
+		if (ISSET(p->p_flag, P_TRACED))
 			return (EBUSY);
 
-		/*
-		 * it's not owned by you, or the last exec gave us
-		 * setuid/setgid privs (unless you're root),
-		 */
-		if ((p->p_cred->p_ruid != curp->p_cred->p_ruid ||
-			ISSET(p->p_flag, P_SUGID)) &&
-		    (error = suser(p->p_ucred, &p->p_acflag)) != 0)
+		if ((error = procfs_checkioperm(curp, p)) != 0)
 			return (error);
 
 		/*
-		 * ...it's init, which controls the security level
-		 * -1 -- permanently insecure
-		 *  0 -- insecure/single-user
-		 */
-		if ((p->p_pid) == 1 && (securelevel > -1))
-			return (EPERM);
-
-		/*
 		 * Go ahead and set the trace flag.
 		 * Save the old parent (it's reset in
 		 *   _DETACH, and also in kern_exit.c:wait4()
@@ -198,11 +184,11 @@ procfs_control(curp, p, op)
 	 */
 	case PROCFS_CTL_DETACH:
 		/* if not being traced, then this is a painless no-op */
-		if ((p->p_flag & P_TRACED) == 0)
+		if (!ISSET(p->p_flag, P_TRACED))
 			return (0);
 
 		/* not being traced any more */
-		p->p_flag &= ~P_TRACED;
+		CLR(p->p_flag, P_TRACED);
 
 		/* give process back to original parent */
 		if (p->p_oppid != p->p_pptr->p_pid) {
@@ -214,7 +200,7 @@ procfs_control(curp, p, op)
 		}
 
 		p->p_oppid = 0;
-		p->p_flag &= ~P_WAITED;	/* XXX ? */
+		CLR(p->p_flag, P_WAITED); /* XXX ? */
 		wakeup((caddr_t) curp);	/* XXX for CTL_WAIT below ? */
 
 		break;
@@ -244,10 +230,10 @@ procfs_control(curp, p, op)
 	 */
 	case PROCFS_CTL_WAIT:
 		error = 0;
-		if (p->p_flag & P_TRACED) {
+		if (ISSET(p->p_flag, P_TRACED)) {
 			while (error == 0 &&
 					(p->p_stat != SSTOP) &&
-					(p->p_flag & P_TRACED) &&
+					ISSET(p->p_flag, P_TRACED) &&
 					(p->p_pptr == curp)) {
 				error = tsleep((caddr_t) p,
 						PWAIT|PCATCH, "procfsx", 0);
diff --git a/sys/miscfs/procfs/procfs_fpregs.c b/sys/miscfs/procfs/procfs_fpregs.c
index 18fcef4885c..d6c3c84c5db 100644
--- a/sys/miscfs/procfs/procfs_fpregs.c
+++ b/sys/miscfs/procfs/procfs_fpregs.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: procfs_fpregs.c,v 1.2 1996/06/21 12:49:54 mickey Exp $	*/
+/*	$OpenBSD: procfs_fpregs.c,v 1.3 1997/08/16 02:00:48 millert Exp $	*/
 /*	$NetBSD: procfs_fpregs.c,v 1.4 1995/08/13 09:06:05 mycroft Exp $	*/
 
 /*
@@ -63,6 +63,9 @@ procfs_dofpregs(curp, p, pfs, uio)
 	char *kv;
 	int kl;
 
+	if ((error = procfs_checkioperm(curp, p)) != 0)
+		return (error);
+
 	kl = sizeof(r);
 	kv = (char *) &r;
 
diff --git a/sys/miscfs/procfs/procfs_mem.c b/sys/miscfs/procfs/procfs_mem.c
index 17cefd69958..00755c7f46c 100644
--- a/sys/miscfs/procfs/procfs_mem.c
+++ b/sys/miscfs/procfs/procfs_mem.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: procfs_mem.c,v 1.4 1997/01/22 04:15:17 tholo Exp $	*/
+/*	$OpenBSD: procfs_mem.c,v 1.5 1997/08/16 02:00:49 millert Exp $	*/
 /*	$NetBSD: procfs_mem.c,v 1.8 1996/02/09 22:40:50 christos Exp $	*/
 
 /*
@@ -57,6 +57,8 @@
 #include <vm/vm_kern.h>
 #include <vm/vm_page.h>
 
+#define	ISSET(t, f)	((t) & (f))
+
 static int procfs_rwmem __P((struct proc *, struct uio *));
 
 static int
@@ -109,7 +111,7 @@ procfs_rwmem(p, uio)
 		 * The map we want...
 		 */
 		map = &p->p_vmspace->vm_map;
-  
+
 		/*
 		 * Check the permissions for the area we're interested
 		 * in.
@@ -211,10 +213,14 @@ procfs_domem(curp, p, pfs, uio)
 	struct pfsnode *pfs;
 	struct uio *uio;
 {
+	int error;
 
 	if (uio->uio_resid == 0)
 		return (0);
 
+	if ((error = procfs_checkioperm(curp, p)) != 0)
+		return (error);
+
 	return (procfs_rwmem(p, uio));
 }
 
@@ -237,6 +243,34 @@ procfs_findtextvp(p)
 	return (p->p_textvp);
 }
 
+/*
+ * You cannot attach to a process's mem/regs if:
+ *
+ *	(1) it's not owned by you, or the last exec
+ *	    gave us setuid/setgid privs (unless
+ *	    you're root), or...
+ *
+ *	(2) ...it's init, which controls the security level
+ *	    of the entire system, and the system was not
+ *	    compiled with permanently insecure mode turned
+ *	    on.
+ */
+int
+procfs_checkioperm(t, p)
+	struct proc *t, *p;
+{
+	int error;
+
+	if ((t->p_cred->p_ruid != p->p_cred->p_ruid ||
+	    ISSET(t->p_flag, P_SUGID)) &&
+	    (error = suser(p->p_ucred, &p->p_acflag)) != 0)
+		return (error);
+
+	if ((t->p_pid == 1) && (securelevel > -1))
+		return (EPERM);
+
+	return (0);
+}
 
 #ifdef probably_never
 /*
@@ -288,7 +322,8 @@ procfs_findtextvp(p)
 			pager = object->pager;
 			printf("procfs: pager = %x\n", pager);
 			if (pager)
-				printf("procfs: found pager, type = %d\n", pager->pg_type);
+				printf("procfs: found pager, type = %d\n",
+				    pager->pg_type);
 			if (pager && pager->pg_type == PG_VNODE) {
 				struct vnode *vp;
 
diff --git a/sys/miscfs/procfs/procfs_regs.c b/sys/miscfs/procfs/procfs_regs.c
index fcf8a90ed48..faf294d67f4 100644
--- a/sys/miscfs/procfs/procfs_regs.c
+++ b/sys/miscfs/procfs/procfs_regs.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: procfs_regs.c,v 1.2 1996/06/21 12:49:55 mickey Exp $	*/
+/*	$OpenBSD: procfs_regs.c,v 1.3 1997/08/16 02:00:49 millert Exp $	*/
 /*	$NetBSD: procfs_regs.c,v 1.9 1995/08/13 09:06:07 mycroft Exp $	*/
 
 /*
@@ -63,6 +63,9 @@ procfs_doregs(curp, p, pfs, uio)
 	char *kv;
 	int kl;
 
+	if ((error = procfs_checkioperm(curp, p)) != 0)
+		return (error);
+
 	kl = sizeof(r);
 	kv = (char *) &r;
 
diff --git a/sys/miscfs/procfs/procfs_subr.c b/sys/miscfs/procfs/procfs_subr.c
index 42f527a97d3..2466afc1e8e 100644
--- a/sys/miscfs/procfs/procfs_subr.c
+++ b/sys/miscfs/procfs/procfs_subr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: procfs_subr.c,v 1.7 1997/08/01 05:58:56 millert Exp $	*/
+/*	$OpenBSD: procfs_subr.c,v 1.8 1997/08/16 02:00:49 millert Exp $	*/
 /*	$NetBSD: procfs_subr.c,v 1.15 1996/02/12 15:01:42 christos Exp $	*/
 
 /*
@@ -113,7 +113,7 @@ loop:
 	/*
 	 * otherwise lock the vp list while we call getnewvnode
 	 * since that can block.
-	 */ 
+	 */
 	if (pfsvplock & PROCFS_LOCKED) {
 		pfsvplock |= PROCFS_WANT;
 		sleep((caddr_t) &pfsvplock, PINOD);
diff --git a/sys/miscfs/procfs/procfs_vnops.c b/sys/miscfs/procfs/procfs_vnops.c
index 2d9d840aebe..d125626df43 100644
--- a/sys/miscfs/procfs/procfs_vnops.c
+++ b/sys/miscfs/procfs/procfs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: procfs_vnops.c,v 1.4 1997/08/01 05:58:57 millert Exp $	*/
+/*	$OpenBSD: procfs_vnops.c,v 1.5 1997/08/16 02:00:50 millert Exp $	*/
 /*	$NetBSD: procfs_vnops.c,v 1.40 1996/03/16 23:52:55 christos Exp $	*/
 
 /*
@@ -217,16 +217,22 @@ procfs_open(v)
 		struct proc *a_p;
 	} */ *ap = v;
 	struct pfsnode *pfs = VTOPFS(ap->a_vp);
+	struct proc *p1 = ap->a_p;
+	struct proc *p2;
+	int error;
+
+	if ((p2 = PFIND(pfs->pfs_pid)) == 0)
+		return (ENOENT);	/* was ESRCH, jsp */
 
 	switch (pfs->pfs_type) {
 	case Pmem:
-		if (PFIND(pfs->pfs_pid) == 0)
-			return (ENOENT);	/* was ESRCH, jsp */
-
 		if (((pfs->pfs_flags & FWRITE) && (ap->a_mode & O_EXCL)) ||
 		    ((pfs->pfs_flags & O_EXCL) && (ap->a_mode & FWRITE)))
 			return (EBUSY);
 
+		if ((error = procfs_checkioperm(p1, p2)) != 0)
+			return (error);
+
 		if (ap->a_mode & FWRITE)
 			pfs->pfs_flags = ap->a_mode & (FWRITE|O_EXCL);
 
@@ -427,15 +433,15 @@ procfs_print(v)
 }
 
 int
-procfs_link(v) 
+procfs_link(v)
 	void *v;
 {
 	struct vop_link_args /* {
 		struct vnode *a_dvp;
-		struct vnode *a_vp;  
+		struct vnode *a_vp;
 		struct componentname *a_cnp;
 	} */ *ap = v;
- 
+
 	VOP_ABORTOP(ap->a_dvp, ap->a_cnp);
 	vput(ap->a_dvp);
 	return (EROFS);
@@ -452,7 +458,7 @@ procfs_symlink(v)
 		struct vattr *a_vap;
 		char *a_target;
 	} */ *ap = v;
-  
+
 	VOP_ABORTOP(ap->a_dvp, ap->a_cnp);
 	vput(ap->a_dvp);
 	return (EROFS);