Module Name: src
Committed By: kamil
Date: Thu Jun 13 20:20:18 UTC 2019
Modified Files:
src/sys/kern: kern_exec.c kern_exit.c kern_fork.c
src/sys/sys: lwp.h
Log Message:
Correct use-after-free issue in vfork(2)
In the previous behavior vforking parent was keeping pointer to a child
and checking whether it clears a PL_PPWAIT in its bitfield p_lflag. However
a child can go invalid between exec/exit event from child and waking up
vforked parent and this can cause invalid pointer read and in the worst
scenario kernel crash.
In the new behavior vforked child keeps a reference to vforked parent LWP
and sets a value l_vforkwaiting to false. This means that vforked child
can finish its work, exec/exit and be terminated and once parent will be
woken up it will read its own field whether its child is still blocking.
Add new field in struct lwp: l_vforkwaiting protected by proc_lock.
In future it should be refactored and all PL_PPWAIT users transformed to
l_vforkwaiting and next l_vforkwaiting probably transformed into a bit
field.
This is another attempt of fixing this bug after <rmind> from 2012 in
commit:
Author: rmind <[email protected]>
Date: Sun Jul 22 22:40:18 2012 +0000
fork1: fix use-after-free problems. Addresses PR/46128 from Andrew Doran.
Note: PL_PPWAIT should be fully replaced and modificaiton of l_pflag by
other LWP is undesirable, but this is enough for netbsd-6.
The new version no longer performs unsafe access in l_lflag changing the
LP_VFORKWAIT bit.
Verified with ATF t_vfork and t_ptrace* tests and they are no longer
causing any issues in my local setup.
Fixes PR/46128 by Andrew Doran
To generate a diff of this commit:
cvs rdiff -u -r1.466 -r1.467 src/sys/kern/kern_exec.c
cvs rdiff -u -r1.275 -r1.276 src/sys/kern/kern_exit.c
cvs rdiff -u -r1.212 -r1.213 src/sys/kern/kern_fork.c
cvs rdiff -u -r1.183 -r1.184 src/sys/sys/lwp.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/kern/kern_exec.c
diff -u src/sys/kern/kern_exec.c:1.466 src/sys/kern/kern_exec.c:1.467
--- src/sys/kern/kern_exec.c:1.466 Tue Jun 11 23:18:55 2019
+++ src/sys/kern/kern_exec.c Thu Jun 13 20:20:18 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: kern_exec.c,v 1.466 2019/06/11 23:18:55 kamil Exp $ */
+/* $NetBSD: kern_exec.c,v 1.467 2019/06/13 20:20:18 kamil Exp $ */
/*-
* Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -59,7 +59,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_exec.c,v 1.466 2019/06/11 23:18:55 kamil Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_exec.c,v 1.467 2019/06/13 20:20:18 kamil Exp $");
#include "opt_exec.h"
#include "opt_execfmt.h"
@@ -1207,10 +1207,17 @@ execve_runproc(struct lwp *l, struct exe
* exited and exec()/exit() are the only places it will be cleared.
*/
if ((p->p_lflag & PL_PPWAIT) != 0) {
+ lwp_t *lp;
+
mutex_enter(proc_lock);
+ lp = p->p_vforklwp;
+ p->p_vforklwp = NULL;
+
l->l_lwpctl = NULL; /* was on loan from blocked parent */
p->p_lflag &= ~PL_PPWAIT;
- cv_broadcast(&p->p_pptr->p_waitcv);
+ lp->l_vforkwaiting = false;
+
+ cv_broadcast(&lp->l_waitcv);
mutex_exit(proc_lock);
}
Index: src/sys/kern/kern_exit.c
diff -u src/sys/kern/kern_exit.c:1.275 src/sys/kern/kern_exit.c:1.276
--- src/sys/kern/kern_exit.c:1.275 Fri May 17 03:34:26 2019
+++ src/sys/kern/kern_exit.c Thu Jun 13 20:20:18 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: kern_exit.c,v 1.275 2019/05/17 03:34:26 ozaki-r Exp $ */
+/* $NetBSD: kern_exit.c,v 1.276 2019/06/13 20:20:18 kamil Exp $ */
/*-
* Copyright (c) 1998, 1999, 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -67,7 +67,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_exit.c,v 1.275 2019/05/17 03:34:26 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_exit.c,v 1.276 2019/06/13 20:20:18 kamil Exp $");
#include "opt_ktrace.h"
#include "opt_dtrace.h"
@@ -342,9 +342,15 @@ exit1(struct lwp *l, int exitcode, int s
*/
mutex_enter(proc_lock);
if (p->p_lflag & PL_PPWAIT) {
+ lwp_t *lp;
+
l->l_lwpctl = NULL; /* was on loan from blocked parent */
p->p_lflag &= ~PL_PPWAIT;
- cv_broadcast(&p->p_pptr->p_waitcv);
+
+ lp = p->p_vforklwp;
+ p->p_vforklwp = NULL;
+ lp->l_vforkwaiting = false;
+ cv_broadcast(&lp->l_waitcv);
}
if (SESS_LEADER(p)) {
Index: src/sys/kern/kern_fork.c
diff -u src/sys/kern/kern_fork.c:1.212 src/sys/kern/kern_fork.c:1.213
--- src/sys/kern/kern_fork.c:1.212 Fri May 3 22:34:21 2019
+++ src/sys/kern/kern_fork.c Thu Jun 13 20:20:18 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: kern_fork.c,v 1.212 2019/05/03 22:34:21 kamil Exp $ */
+/* $NetBSD: kern_fork.c,v 1.213 2019/06/13 20:20:18 kamil Exp $ */
/*-
* Copyright (c) 1999, 2001, 2004, 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -67,7 +67,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_fork.c,v 1.212 2019/05/03 22:34:21 kamil Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_fork.c,v 1.213 2019/06/13 20:20:18 kamil Exp $");
#include "opt_ktrace.h"
#include "opt_dtrace.h"
@@ -413,11 +413,12 @@ fork1(struct lwp *l1, int flags, int exi
if (flags & FORK_PPWAIT) {
/* Mark ourselves as waiting for a child. */
- l1->l_pflag |= LP_VFORKWAIT;
p2->p_lflag = PL_PPWAIT;
+ l1->l_vforkwaiting = true;
p2->p_vforklwp = l1;
} else {
p2->p_lflag = 0;
+ l1->l_vforkwaiting = false;
}
p2->p_sflag = 0;
p2->p_slflag = 0;
@@ -610,10 +611,10 @@ fork1(struct lwp *l1, int flags, int exi
/*
* Preserve synchronization semantics of vfork. If waiting for
- * child to exec or exit, sleep until it clears LP_VFORKWAIT.
+ * child to exec or exit, sleep until it clears p_vforkwaiting.
*/
- while (p2->p_lflag & PL_PPWAIT) // XXX: p2 can go invalid
- cv_wait(&p1->p_waitcv, proc_lock);
+ while (l1->l_vforkwaiting)
+ cv_wait(&l1->l_waitcv, proc_lock);
/*
* Let the parent know that we are tracing its child.
Index: src/sys/sys/lwp.h
diff -u src/sys/sys/lwp.h:1.183 src/sys/sys/lwp.h:1.184
--- src/sys/sys/lwp.h:1.183 Fri May 17 03:34:26 2019
+++ src/sys/sys/lwp.h Thu Jun 13 20:20:18 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: lwp.h,v 1.183 2019/05/17 03:34:26 ozaki-r Exp $ */
+/* $NetBSD: lwp.h,v 1.184 2019/06/13 20:20:18 kamil Exp $ */
/*
* Copyright (c) 2001, 2006, 2007, 2008, 2009, 2010
@@ -132,6 +132,7 @@ struct lwp {
callout_t l_timeout_ch; /* !: callout for tsleep */
u_int l_emap_gen; /* !: emap generation number */
kcondvar_t l_waitcv; /* a: vfork() wait */
+ bool l_vforkwaiting; /* a: vfork() waiting */
#if PCU_UNIT_COUNT > 0
struct cpu_info * volatile l_pcu_cpu[PCU_UNIT_COUNT];
@@ -256,7 +257,6 @@ extern int maxlwp __read_mostly; /* max
#define LP_INTR 0x00000040 /* Soft interrupt handler */
#define LP_SYSCTLWRITE 0x00000080 /* sysctl write lock held */
#define LP_MUSTJOIN 0x00000100 /* Must join kthread on exit */
-#define LP_VFORKWAIT 0x00000200 /* Waiting at vfork() for a child */
#define LP_SINGLESTEP 0x00000400 /* Single step thread in ptrace(2) */
#define LP_TIMEINTR 0x00010000 /* Time this soft interrupt */
#define LP_PREEMPTING 0x00020000 /* mi_switch called involuntarily */
