svn commit: r340793 - head/sys/kern

[Mateusz Guzik]

Author: mjg
Date: Fri Nov 23 04:38:50 2018
New Revision: 340793
URL: https://svnweb.freebsd.org/changeset/base/340793

Log:
  Revert "fork: fix use-after-free with vfork"
  
  This unreliably breaks libc handling of vfork where forking succeded,
  but execve did not.
  
  vfork code in libc performs waitpid with WNOHANG in case of failed exec.
  With the fix exit codepath was waking up the parent before the child
  fully transitioned to a zombie. Woken up parent would waitpid, which
  could find a not-yet-zombie child and fail to reap it due to the WNOHANG
  flag.
  
  While removing the flag fixes the problem, it is not an option due to older
  releases which would still suffer from the kernel change.
  
  Revert the fix until a solution can be worked out.
  
  Note that while use-after-free which gets back due to the revert is a real
  bug, it's side-effects are limited due to the fact that struct proc memory
  is never released by UMA.

Modified:
  head/sys/kern/kern_exit.c
  head/sys/kern/kern_fork.c
  head/sys/kern/subr_syscall.c

Modified: head/sys/kern/kern_exit.c
==============================================================================
--- head/sys/kern/kern_exit.c	Fri Nov 23 03:42:05 2018	(r340792)
+++ head/sys/kern/kern_exit.c	Fri Nov 23 04:38:50 2018	(r340793)
@@ -285,15 +285,6 @@ exit1(struct thread *td, int rval, int signo)
 	wakeup(&p->p_stype);
 
 	/*
-	 * If P_PPWAIT is set our parent holds us with p_lock and may
-	 * be waiting on p_pwait.
-	 */
-	if (p->p_flag & P_PPWAIT) {
-		p->p_flag &= ~P_PPWAIT;
-		cv_broadcast(&p->p_pwait);
-	}
-
-	/*
 	 * Wait for any processes that have a hold on our vmspace to
 	 * release their reference.
 	 */
@@ -338,9 +329,13 @@ exit1(struct thread *td, int rval, int signo)
 	 */
 	EVENTHANDLER_DIRECT_INVOKE(process_exit, p);
 
+	/*
+	 * If parent is waiting for us to exit or exec,
+	 * P_PPWAIT is set; we will wakeup the parent below.
+	 */
 	PROC_LOCK(p);
 	stopprofclock(p);
-	p->p_flag &= ~(P_TRACED | P_PPTRACE);
+	p->p_flag &= ~(P_TRACED | P_PPWAIT | P_PPTRACE);
 	p->p_ptevents = 0;
 
 	/*
@@ -641,6 +636,7 @@ exit1(struct thread *td, int rval, int signo)
 	 * proc lock.
 	 */
 	wakeup(p->p_pptr);
+	cv_broadcast(&p->p_pwait);
 	sched_exit(p->p_pptr, td);
 	PROC_SLOCK(p);
 	p->p_state = PRS_ZOMBIE;

Modified: head/sys/kern/kern_fork.c
==============================================================================
--- head/sys/kern/kern_fork.c	Fri Nov 23 03:42:05 2018	(r340792)
+++ head/sys/kern/kern_fork.c	Fri Nov 23 04:38:50 2018	(r340793)
@@ -720,7 +720,6 @@ do_fork(struct thread *td, struct fork_req *fr, struct
 		dtrace_fasttrap_fork(p1, p2);
 #endif
 	if (fr->fr_flags & RFPPWAIT) {
-		_PHOLD(p2);
 		td->td_pflags |= TDP_RFPPWAIT;
 		td->td_rfppwait_p = p2;
 		td->td_dbgflags |= TDB_VFORK;

Modified: head/sys/kern/subr_syscall.c
==============================================================================
--- head/sys/kern/subr_syscall.c	Fri Nov 23 03:42:05 2018	(r340792)
+++ head/sys/kern/subr_syscall.c	Fri Nov 23 04:38:50 2018	(r340793)
@@ -257,7 +257,6 @@ again:
 			}
 			cv_timedwait(&p2->p_pwait, &p2->p_mtx, hz);
 		}
-		_PRELE(p2);
 		PROC_UNLOCK(p2);
 
 		if (td->td_dbgflags & TDB_VFORK) {