Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 22 Nov 2018 21:08:38 +0000 (UTC)
From:      Mateusz Guzik <mjg@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r340784 - head/sys/kern
Message-ID:  <201811222108.wAML8cnG024699@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: mjg
Date: Thu Nov 22 21:08:37 2018
New Revision: 340784
URL: https://svnweb.freebsd.org/changeset/base/340784

Log:
  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

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	Thu Nov 22 20:49:41 2018	(r340783)
+++ head/sys/kern/kern_exit.c	Thu Nov 22 21:08:37 2018	(r340784)
@@ -285,6 +285,15 @@ 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.
 	 */
@@ -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;

Modified: head/sys/kern/kern_fork.c
==============================================================================
--- head/sys/kern/kern_fork.c	Thu Nov 22 20:49:41 2018	(r340783)
+++ head/sys/kern/kern_fork.c	Thu Nov 22 21:08:37 2018	(r340784)
@@ -725,6 +725,7 @@ do_fork(struct thread *td, struct fork_req *fr, struct
 	 */
 	_PHOLD(p2);
 	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	Thu Nov 22 20:49:41 2018	(r340783)
+++ head/sys/kern/subr_syscall.c	Thu Nov 22 21:08:37 2018	(r340784)
@@ -257,6 +257,7 @@ again:
 			}
 			cv_timedwait(&p2->p_pwait, &p2->p_mtx, hz);
 		}
+		_PRELE(p2);
 		PROC_UNLOCK(p2);
 
 		if (td->td_dbgflags & TDB_VFORK) {

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201811222108.wAML8cnG024699>