Add a P_INEXEC flag that indicates that the process has called execve() and

it has not yet returned.  Use this flag to deny debugging requests while
the process is execve()ing, and close once and for all any race conditions
that might occur between execve() and various debugging interfaces.

Reviewed by:	jhb, rwatson

3 files changed, 30 insertions, 15 deletions

diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index 5f87bd7..3766cb6 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -118,6 +118,20 @@ execve(td, uap)
 
 	imgp = &image_params;
 
+	/*
+	 * Lock the process and set the P_INEXEC flag to indicate that
+	 * it should be left alone until we're done here.  This is
+	 * necessary to avoid race conditions - e.g. in ptrace() -
+	 * that might allow a local user to illicitly obtain elevated
+	 * privileges.
+	 */
+	mtx_lock(&Giant);
+	PROC_LOCK(p);
+	KASSERT((p->p_flag & P_INEXEC) == 0,
+	    (__FUNCTION__ "(): process already has P_INEXEC flag"));
+	p->p_flag |= P_INEXEC;
+	PROC_UNLOCK(p);
+	
 /* XXXKSE */
 /* !!!!!!!! we need abort all the other threads of this process before we */
 /* proceed beyond his point! */
@@ -140,8 +154,6 @@ execve(td, uap)
 	imgp->ps_strings = 0;
 	imgp->auxarg_size = 0;
 
-	mtx_lock(&Giant);
-
 	/*
 	 * Allocate temporary demand zeroed space for argument and
 	 *	environment strings
@@ -307,15 +319,6 @@ interpret:
 	}
 
 	/*
-	 * XXX: Note, the whole execve() is incredibly racey right now
-	 * with regards to debugging and privilege/credential management.
-	 * In particular, it's possible to race during exec() to attach
-	 * debugging to a process that will gain privilege.
-	 *
-	 * This MUST be fixed prior to any release.
-	 */
-
-	/*
 	 * Implement image setuid/setgid.
 	 *
 	 * Don't honor setuid/setgid if the filesystem prohibits it or if
@@ -399,14 +402,16 @@ interpret:
 	p->p_textvp = ndp->ni_vp;
 
 	/*
-	 * notify others that we exec'd
+	 * Notify others that we exec'd, and clear the P_INEXEC flag
+	 * as we're now a bona fide freshly-execed process.
 	 */
 	PROC_LOCK(p);
 	KNOTE(&p->p_klist, NOTE_EXEC);
+	p->p_flag &= ~P_INEXEC;
 
 	/*
 	 * If tracing the process, trap to debugger so breakpoints
-	 * 	can be set before the program executes.
+	 * can be set before the program executes.
 	 */
 	_STOPEVENT(p, S_EXEC, 0);
 
@@ -461,15 +466,20 @@ exec_fail_dealloc:
 		goto done2;
 
 exec_fail:
+	/* we're done here, clear P_INEXEC */
+	PROC_LOCK(p);
+	p->p_flag &= ~P_INEXEC;
+	PROC_UNLOCK(p);
+	
 	if (imgp->vmspace_destroyed) {
 		/* sorry, no more process anymore. exit gracefully */
 		exit1(td, W_EXITCODE(0, SIGABRT));
 		/* NOT REACHED */
 		error = 0;
-	} 
+	}
 done2:
 	mtx_unlock(&Giant);
-	return(error);
+	return (error);
 }
 
 int
diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
index 894efac..11917b4 100644
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -1569,6 +1569,10 @@ p_candebug(struct proc *p1, struct proc *p2)
 			return (error);
 	}
 
+	/* can't trace a process that's currently exec'ing */
+	if ((p2->p_flag & P_INEXEC) != 0)
+		return (EAGAIN);
+	
 	return (0);
 }
 
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 4008848..19e6dbd 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -470,6 +470,7 @@ struct proc {
 #define	P_JAILED	0x1000000 /* Process is in jail. */
 #define	P_OLDMASK	0x2000000 /* Need to restore mask after suspend. */
 #define	P_ALTSTACK	0x4000000 /* Have alternate signal stack. */
+#define	P_INEXEC	0x8000000 /* Process is in execve(). */
 
 /* These flags are kept in p_sflag and are protected with sched_lock. */
 #define	PS_INMEM	0x00001	/* Loaded into memory. */