Date: Thu, 20 May 2021 14:10:41 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: cf60931c32d7 - stable/13 - fork: Suspend other threads if both RFPROC and RFMEM are not set Message-ID: <202105201410.14KEAfYF039029@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=cf60931c32d7c5401c69ff0081e049a100673444 commit cf60931c32d7c5401c69ff0081e049a100673444 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2021-05-13 12:33:23 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2021-05-20 13:16:47 +0000 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 Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D30220 (cherry picked from commit 9246b3090cbc82c54350391601b9acef2aa9a625) --- sys/kern/kern_fork.c | 13 +++++++++---- sys/vm/vm_glue.c | 8 +++----- sys/vm/vm_map.c | 4 ++++ 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c index 2a092b192878..0d0659b432fe 100644 --- a/sys/kern/kern_fork.c +++ b/sys/kern/kern_fork.c @@ -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); diff --git a/sys/vm/vm_glue.c b/sys/vm/vm_glue.c index a2500828eae4..be741fd40199 100644 --- a/sys/vm/vm_glue.c +++ b/sys/vm/vm_glue.c @@ -550,11 +550,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); diff --git a/sys/vm/vm_map.c b/sys/vm/vm_map.c index b3288fce5114..f17342b2a545 100644 --- a/sys/vm/vm_map.c +++ b/sys/vm/vm_map.c @@ -4896,6 +4896,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;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202105201410.14KEAfYF039029>