- We shouldn't allocate MAXPATHLEN from the stack.

 - Don't copy the pathbuf into kernel space unless we're systracing.
deraadt@ ok

2 files changed, 41 insertions, 28 deletions

diff --git a/sys/kern/exec_script.c b/sys/kern/exec_script.c
index ba6b99f0575..929982fa393 100644
--- a/sys/kern/exec_script.c
+++ b/sys/kern/exec_script.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: exec_script.c,v 1.21 2004/07/07 07:31:40 marius Exp $	*/
+/*	$OpenBSD: exec_script.c,v 1.22 2005/08/01 07:02:39 art Exp $	*/
 /*	$NetBSD: exec_script.c,v 1.13 1996/02/04 02:15:06 christos Exp $	*/
 
 /*
@@ -221,20 +221,21 @@ check_shell:
 #endif
 		/* normally can't fail, but check for it if diagnostic */
 #if NSYSTRACE > 0
-		error = 1;
 		if (ISSET(p->p_flag, P_SYSTRACE)) {
 			error = systrace_scriptname(p, *tmpsap);
 			if (error == 0)
 				tmpsap++;
-		}
-		if (error != 0)
-			/*
-			 * Since systrace_scriptname() provides a
-			 * convenience, not a security issue, we are
-			 * safe to do this.
-			 */
-			error = copystr(epp->ep_name, *tmpsap++,
-			    MAXPATHLEN, NULL);
+			else
+				/*
+				 * Since systrace_scriptname() provides a
+				 * convenience, not a security issue, we are
+				 * safe to do this.
+				 */
+				error = copystr(epp->ep_name, *tmpsap++,
+				    MAXPATHLEN, NULL);
+		} else
+			error = copyinstr(epp->ep_name, *tmpsap++, MAXPATHLEN,
+			    NULL);
 #else
 		error = copyinstr(epp->ep_name, *tmpsap++, MAXPATHLEN,
 		    (size_t *)0);
diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index 5d2126c2004..c5fc952c164 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_exec.c,v 1.93 2005/07/07 23:43:04 deraadt Exp $	*/
+/*	$OpenBSD: kern_exec.c,v 1.94 2005/08/01 07:02:39 art Exp $	*/
 /*	$NetBSD: kern_exec.c,v 1.75 1996/02/09 18:59:28 christos Exp $	*/
 
 /*-
@@ -264,9 +264,9 @@ sys_execve(p, v, retval)
 #if NSYSTRACE > 0
 	int wassugid =
 	    ISSET(p->p_flag, P_SUGID) || ISSET(p->p_flag, P_SUGIDEXEC);
-	char pathbuf[MAXPATHLEN];
 	size_t pathbuflen;
 #endif
+	char *pathbuf = NULL;
 
 	/*
 	 * Cheap solution to complicated problems.
@@ -275,27 +275,29 @@ sys_execve(p, v, retval)
 	p->p_flag |= P_INEXEC;
 
 #if NSYSTRACE > 0
-	if (ISSET(p->p_flag, P_SYSTRACE))
+	if (ISSET(p->p_flag, P_SYSTRACE)) {
 		systrace_execve0(p);
-
-	error = copyinstr(SCARG(uap, path), pathbuf, MAXPATHLEN, &pathbuflen);
-	if (error != 0)
-		goto clrflag;
-
-	NDINIT(&nid, LOOKUP, NOFOLLOW, UIO_SYSSPACE, pathbuf, p);
-#else
-	/* init the namei data to point the file user's program name */
-	NDINIT(&nid, LOOKUP, NOFOLLOW, UIO_USERSPACE, SCARG(uap, path), p);
+		pathbuf = pool_get(&namei_pool, PR_WAITOK);
+		error = copyinstr(SCARG(uap, path), pathbuf, MAXPATHLEN,
+		    &pathbuflen);
+		if (error != 0)
+			goto clrflag;
+	}
 #endif
+	if (pathbuf != NULL) {
+		NDINIT(&nid, LOOKUP, NOFOLLOW, UIO_SYSSPACE, pathbuf, p);
+	} else {
+		NDINIT(&nid, LOOKUP, NOFOLLOW, UIO_USERSPACE,
+		    SCARG(uap, path), p);
+	}
 
 	/*
 	 * initialize the fields of the exec package.
 	 */
-#if NSYSTRACE > 0
-	pack.ep_name = pathbuf;
-#else
-	pack.ep_name = (char *)SCARG(uap, path);
-#endif
+	if (pathbuf != NULL)
+		pack.ep_name = pathbuf;
+	else
+		pack.ep_name = (char *)SCARG(uap, path);
 	pack.ep_hdr = malloc(exec_maxhdrsz, M_EXEC, M_WAITOK);
 	pack.ep_hdrlen = exec_maxhdrsz;
 	pack.ep_hdrvalid = 0;
@@ -660,6 +662,9 @@ sys_execve(p, v, retval)
 		systrace_execve1(pathbuf, p);
 #endif
 
+	if (pathbuf != NULL)
+		pool_put(&namei_pool, pathbuf);
+
 	return (0);
 
 bad:
@@ -685,6 +690,10 @@ bad:
  clrflag:
 #endif
 	p->p_flag &= ~P_INEXEC;
+
+	if (pathbuf != NULL)
+		pool_put(&namei_pool, pathbuf);
+
 	return (error);
 
 exec_abort:
@@ -709,6 +718,9 @@ free_pack_abort:
 
 	/* NOTREACHED */
 	p->p_flag &= ~P_INEXEC;
+	if (pathbuf != NULL)
+		pool_put(&namei_pool, pathbuf);
+
 	return (0);
 }