From owner-svn-src-head@freebsd.org Thu Nov 22 21:08:39 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 5FB581107E9E; Thu, 22 Nov 2018 21:08:39 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id EC74769BD1; Thu, 22 Nov 2018 21:08:38 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id B14771526C; Thu, 22 Nov 2018 21:08:38 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id wAML8cSS024703; Thu, 22 Nov 2018 21:08:38 GMT (envelope-from mjg@FreeBSD.org) Received: (from mjg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id wAML8cnG024699; Thu, 22 Nov 2018 21:08:38 GMT (envelope-from mjg@FreeBSD.org) Message-Id: <201811222108.wAML8cnG024699@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mjg set sender to mjg@FreeBSD.org using -f From: Mateusz Guzik Date: Thu, 22 Nov 2018 21:08:38 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r340784 - head/sys/kern X-SVN-Group: head X-SVN-Commit-Author: mjg X-SVN-Commit-Paths: head/sys/kern X-SVN-Commit-Revision: 340784 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: EC74769BD1 X-Spamd-Result: default: False [1.49 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_SPAM_SHORT(0.33)[0.332,0]; NEURAL_SPAM_MEDIUM(0.74)[0.738,0]; NEURAL_SPAM_LONG(0.42)[0.420,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US] X-Rspamd-Server: mx1.freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Nov 2018 21:08:39 -0000 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) {