From b00b27e925f0f0c21802159dadf41af02e09ae24 Mon Sep 17 00:00:00 2001
From: Mateusz Guzik <mjg@FreeBSD.org>
Date: Thu, 22 Nov 2018 21:08:37 +0000
Subject: [PATCH] fork: fix use-after-free with vfork

The pointer to the child is stored without any reference held. Then it is
blindly used to wait until P_PPWAIT is cleared. However, if the child is
autoreaped it could have exited and get freed before the parent started
waiting.

Use the existing hold mechanism to mitigate the problem. Most common case
of doing exec remains unchanged. The corner case of doing exit performs
wake up before waiting for holds to clear.

Reviewed by:	kib
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D18295
---
 sys/kern/kern_exit.c    | 16 ++++++++++------
 sys/kern/kern_fork.c    |  1 +
 sys/kern/subr_syscall.c |  1 +
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c
index f88c7cf28f59..a4c53e9d1a41 100644
--- a/sys/kern/kern_exit.c
+++ b/sys/kern/kern_exit.c
@@ -284,6 +284,15 @@ exit1(struct thread *td, int rval, int signo)
 	p->p_flag |= P_WEXIT;
 	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.
@@ -329,13 +338,9 @@ 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_PPWAIT | P_PPTRACE);
+	p->p_flag &= ~(P_TRACED | P_PPTRACE);
 	p->p_ptevents = 0;
 
 	/*
@@ -636,7 +641,6 @@ 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;
diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index a155246e6cc0..2d64e422ba8b 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -725,6 +725,7 @@ do_fork(struct thread *td, struct fork_req *fr, struct proc *p2, struct thread *
 	 */
 	_PHOLD(p2);
 	if (fr->fr_flags & RFPPWAIT) {
+		_PHOLD(p2);
 		td->td_pflags |= TDP_RFPPWAIT;
 		td->td_rfppwait_p = p2;
 		td->td_dbgflags |= TDB_VFORK;
diff --git a/sys/kern/subr_syscall.c b/sys/kern/subr_syscall.c
index 6c70ddc9f789..db0efb080b03 100644
--- a/sys/kern/subr_syscall.c
+++ b/sys/kern/subr_syscall.c
@@ -257,6 +257,7 @@ syscallret(struct thread *td, int error)
 			}
 			cv_timedwait(&p2->p_pwait, &p2->p_mtx, hz);
 		}
+		_PRELE(p2);
 		PROC_UNLOCK(p2);
 
 		if (td->td_dbgflags & TDB_VFORK) {