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

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
@@ -306,15 +318,6 @@ execve(td, uap)
 		wakeup((caddr_t)p->p_pptr);
 	}
 
-	/*
-	 * 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.
 	 *
@@ -399,14 +402,16 @@ execve(td, uap)
 	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 @@ execve(td, uap)
 		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

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

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. */