Date: Sun, 5 Oct 2014 12:29:12 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: freebsd-hackers@freebsd.org Cc: kib@freebsd.org Subject: fork: hold newly created processes Message-ID: <20141005102912.GB9262@dft-labs.eu>
next in thread | raw e-mail | index | archive | help
fork: hold newly created processes
Consumers of fork1 -> do_fork receive new proc pointer, but nothing
guarnatees its stability at that time.
New process could already exit and be waited for, in which case we get a
use after free.
This is a temporary fix.
diff --git a/sys/compat/linux/linux_fork.c b/sys/compat/linux/linux_fork.c
index 316cf2a..8ad33f8 100644
--- a/sys/compat/linux/linux_fork.c
+++ b/sys/compat/linux/linux_fork.c
@@ -95,6 +95,8 @@ linux_fork(struct thread *td, struct linux_fork_args *args)
sched_add(td2, SRQ_BORING);
thread_unlock(td2);
+ PRELE(p2);
+
return (0);
}
@@ -139,6 +141,7 @@ linux_vfork(struct thread *td, struct linux_vfork_args *args)
PROC_LOCK(p2);
while (p2->p_flag & P_PPWAIT)
cv_wait(&p2->p_pwait, &p2->p_mtx);
+ _PRELE(p2);
PROC_UNLOCK(p2);
return (0);
@@ -287,13 +290,14 @@ linux_clone(struct thread *td, struct linux_clone_args *args)
td->td_retval[0] = p2->p_pid;
td->td_retval[1] = 0;
+ PROC_LOCK(p2);
if (args->flags & LINUX_CLONE_VFORK) {
/* wait for the children to exit, ie. emulate vfork */
- PROC_LOCK(p2);
while (p2->p_flag & P_PPWAIT)
cv_wait(&p2->p_pwait, &p2->p_mtx);
- PROC_UNLOCK(p2);
}
+ _PRELE(p2);
+ PROC_UNLOCK(p2);
return (0);
}
diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c
index 141d438..af905a5 100644
--- a/sys/kern/init_main.c
+++ b/sys/kern/init_main.c
@@ -832,6 +832,7 @@ create_init(const void *udata __unused)
audit_cred_proc1(newcred);
#endif
initproc->p_ucred = newcred;
+ _PRELE(initproc);
PROC_UNLOCK(initproc);
crfree(oldcred);
cred_update_thread(FIRST_THREAD_IN_PROC(initproc));
diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index 8135afb..596a28c 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -107,6 +107,7 @@ sys_fork(struct thread *td, struct fork_args *uap)
if (error == 0) {
td->td_retval[0] = p2->p_pid;
td->td_retval[1] = 0;
+ PRELE(p2);
}
return (error);
}
@@ -130,6 +131,7 @@ sys_pdfork(td, uap)
if (error == 0) {
td->td_retval[0] = p2->p_pid;
td->td_retval[1] = 0;
+ PRELE(p2);
error = copyout(&fd, uap->fdp, sizeof(fd));
}
return (error);
@@ -147,6 +149,7 @@ sys_vfork(struct thread *td, struct vfork_args *uap)
if (error == 0) {
td->td_retval[0] = p2->p_pid;
td->td_retval[1] = 0;
+ PRELE(p2);
}
return (error);
}
@@ -166,6 +169,8 @@ sys_rfork(struct thread *td, struct rfork_args *uap)
if (error == 0) {
td->td_retval[0] = p2 ? p2->p_pid : 0;
td->td_retval[1] = 0;
+ if (p2 != NULL)
+ PRELE(p2);
}
return (error);
}
@@ -359,7 +364,7 @@ do_fork(struct thread *td, int flags, struct proc *p2, struct thread *td2,
struct vmspace *vm2, int pdflags)
{
struct proc *p1, *pptr;
- int p2_held, trypid;
+ int trypid;
struct filedesc *fd;
struct filedesc_to_leader *fdtol;
struct sigacts *newsigacts;
@@ -367,7 +372,6 @@ do_fork(struct thread *td, int flags, struct proc *p2, struct thread *td2,
sx_assert(&proctree_lock, SX_SLOCKED);
sx_assert(&allproc_lock, SX_XLOCKED);
- p2_held = 0;
p1 = td->td_proc;
/*
@@ -674,6 +678,12 @@ do_fork(struct thread *td, int flags, struct proc *p2, struct thread *td2,
p2->p_state = PRS_NORMAL;
PROC_SUNLOCK(p2);
+ /*
+ * We return p2 pointer to the caller. Hold it so that it is
+ * guaranteed to remain valid.
+ */
+ _PHOLD(p2);
+
#ifdef KDTRACE_HOOKS
/*
* Tell the DTrace fasttrap provider about the new process so that any
@@ -696,8 +706,6 @@ do_fork(struct thread *td, int flags, struct proc *p2, struct thread *td2,
td->td_dbgflags |= TDB_FORK;
td->td_dbg_forked = p2->p_pid;
td2->td_dbgflags |= TDB_STOPATFORK;
- _PHOLD(p2);
- p2_held = 1;
}
if (flags & RFPPWAIT) {
td->td_pflags |= TDP_RFPPWAIT;
@@ -733,8 +741,6 @@ do_fork(struct thread *td, int flags, struct proc *p2, struct thread *td2,
PROC_LOCK(p2);
while ((td2->td_dbgflags & TDB_STOPATFORK) != 0)
cv_wait(&p2->p_dbgwait, &p2->p_mtx);
- if (p2_held)
- _PRELE(p2);
PROC_UNLOCK(p2);
}
diff --git a/sys/kern/kern_kthread.c b/sys/kern/kern_kthread.c
index 969c513..bb0e232 100644
--- a/sys/kern/kern_kthread.c
+++ b/sys/kern/kern_kthread.c
@@ -134,6 +134,8 @@ kproc_create(void (*func)(void *), void *arg,
sched_add(td, SRQ_BORING);
thread_unlock(td);
+ PRELE(p2);
+
return 0;
}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141005102912.GB9262>