fork: Suspend other threads if both RFPROC and RFMEM are not set

Otherwise, a multithreaded parent process may trigger races in
vm_forkproc() if one thread calls rfork() with RFMEM set and another
calls rfork() without RFMEM.

Also simplify vm_forkproc() a bit, vmspace_unshare() already checks to
see if the address space is shared.

Reported by:	syzbot+0aa7c2bec74c4066c36f@syzkaller.appspotmail.com
Reported by:	syzbot+ea84cb06937afeae609d@syzkaller.appspotmail.com
Reviewed by:	kib
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D30220
This commit is contained in:
Mark Johnston 2021-05-13 08:33:23 -04:00
parent b23362afa9
commit 9246b3090c
3 changed files with 16 additions and 9 deletions

View File

@ -313,8 +313,13 @@ fork_norfproc(struct thread *td, int flags)
("fork_norfproc called with RFPROC set"));
p1 = td->td_proc;
if (((p1->p_flag & (P_HADTHREADS|P_SYSTEM)) == P_HADTHREADS) &&
(flags & (RFCFDG | RFFDG))) {
/*
* Quiesce other threads if necessary. If RFMEM is not specified we
* must ensure that other threads do not concurrently create a second
* process sharing the vmspace, see vmspace_unshare().
*/
if ((p1->p_flag & (P_HADTHREADS | P_SYSTEM)) == P_HADTHREADS &&
((flags & (RFCFDG | RFFDG)) != 0 || (flags & RFMEM) == 0)) {
PROC_LOCK(p1);
if (thread_single(p1, SINGLE_BOUNDARY)) {
PROC_UNLOCK(p1);
@ -350,8 +355,8 @@ fork_norfproc(struct thread *td, int flags)
}
fail:
if (((p1->p_flag & (P_HADTHREADS|P_SYSTEM)) == P_HADTHREADS) &&
(flags & (RFCFDG | RFFDG))) {
if ((p1->p_flag & (P_HADTHREADS | P_SYSTEM)) == P_HADTHREADS &&
((flags & (RFCFDG | RFFDG)) != 0 || (flags & RFMEM) == 0)) {
PROC_LOCK(p1);
thread_single_end(p1, SINGLE_BOUNDARY);
PROC_UNLOCK(p1);

View File

@ -553,11 +553,9 @@ vm_forkproc(struct thread *td, struct proc *p2, struct thread *td2,
* COW locally.
*/
if ((flags & RFMEM) == 0) {
if (refcount_load(&p1->p_vmspace->vm_refcnt) > 1) {
error = vmspace_unshare(p1);
if (error)
return (error);
}
error = vmspace_unshare(p1);
if (error)
return (error);
}
cpu_fork(td, p2, td2, flags);
return (0);

View File

@ -4867,6 +4867,10 @@ vmspace_unshare(struct proc *p)
struct vmspace *newvmspace;
vm_ooffset_t fork_charge;
/*
* The caller is responsible for ensuring that the reference count
* cannot concurrently transition 1 -> 2.
*/
if (refcount_load(&oldvmspace->vm_refcnt) == 1)
return (0);
fork_charge = 0;