From owner-svn-src-all@freebsd.org Sat Aug 17 18:19:50 2019 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 1917ACD7F7; Sat, 17 Aug 2019 18:19:50 +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.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 469pPd6zbBz4bXT; Sat, 17 Aug 2019 18:19:49 +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 D173E1D9B9; Sat, 17 Aug 2019 18:19:49 +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 x7HIJnbZ089692; Sat, 17 Aug 2019 18:19:49 GMT (envelope-from mjg@FreeBSD.org) Received: (from mjg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x7HIJnAF089691; Sat, 17 Aug 2019 18:19:49 GMT (envelope-from mjg@FreeBSD.org) Message-Id: <201908171819.x7HIJnAF089691@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mjg set sender to mjg@FreeBSD.org using -f From: Mateusz Guzik Date: Sat, 17 Aug 2019 18:19:49 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r351175 - head/sys/kern X-SVN-Group: head X-SVN-Commit-Author: mjg X-SVN-Commit-Paths: head/sys/kern X-SVN-Commit-Revision: 351175 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 17 Aug 2019 18:19:50 -0000 Author: mjg Date: Sat Aug 17 18:19:49 2019 New Revision: 351175 URL: https://svnweb.freebsd.org/changeset/base/351175 Log: fork: rework locking around do_fork - move allproc lock into the func, it is of no use prior to it - the code would lock p1 and p2 while holding allproc to partially construct it after it gets added to the list. instead we can do the work prior to adding anything. - protect lastpid with procid_lock As a side effect we do less work with allproc held. Sponsored by: The FreeBSD Foundation Modified: head/sys/kern/kern_fork.c Modified: head/sys/kern/kern_fork.c ============================================================================== --- head/sys/kern/kern_fork.c Sat Aug 17 17:56:43 2019 (r351174) +++ head/sys/kern/kern_fork.c Sat Aug 17 18:19:49 2019 (r351175) @@ -246,17 +246,24 @@ static int fork_findpid(int flags) { pid_t result; - int trypid; + int trypid, random; + /* + * Avoid calling arc4random with procid_lock held. + */ + random = 0; + if (__predict_false(randompid)) + random = arc4random() % randompid; + + mtx_lock(&procid_lock); + trypid = lastpid + 1; if (flags & RFHIGHPID) { if (trypid < 10) trypid = 10; } else { - if (randompid) - trypid += arc4random() % randompid; + trypid += random; } - mtx_lock(&procid_lock); retry: if (trypid >= pid_max) trypid = 2; @@ -341,33 +348,16 @@ do_fork(struct thread *td, struct fork_req *fr, struct struct vmspace *vm2, struct file *fp_procdesc) { struct proc *p1, *pptr; - int trypid; struct filedesc *fd; struct filedesc_to_leader *fdtol; struct sigacts *newsigacts; - sx_assert(&allproc_lock, SX_XLOCKED); - p1 = td->td_proc; - trypid = fork_findpid(fr->fr_flags); - p2->p_state = PRS_NEW; /* protect against others */ - p2->p_pid = trypid; - AUDIT_ARG_PID(p2->p_pid); - LIST_INSERT_HEAD(&allproc, p2, p_list); - allproc_gen++; - sx_xlock(PIDHASHLOCK(p2->p_pid)); - LIST_INSERT_HEAD(PIDHASH(p2->p_pid), p2, p_hash); - sx_xunlock(PIDHASHLOCK(p2->p_pid)); - PROC_LOCK(p2); PROC_LOCK(p1); - - sx_xunlock(&allproc_lock); - bcopy(&p1->p_startcopy, &p2->p_startcopy, __rangeof(struct proc, p_startcopy, p_endcopy)); pargs_hold(p2->p_args); - PROC_UNLOCK(p1); bzero(&p2->p_startzero, @@ -376,8 +366,19 @@ do_fork(struct thread *td, struct fork_req *fr, struct /* Tell the prison that we exist. */ prison_proc_hold(p2->p_ucred->cr_prison); - PROC_UNLOCK(p2); + p2->p_state = PRS_NEW; /* protect against others */ + p2->p_pid = fork_findpid(fr->fr_flags); + AUDIT_ARG_PID(p2->p_pid); + sx_xlock(&allproc_lock); + LIST_INSERT_HEAD(&allproc, p2, p_list); + allproc_gen++; + sx_xunlock(&allproc_lock); + + sx_xlock(PIDHASHLOCK(p2->p_pid)); + LIST_INSERT_HEAD(PIDHASH(p2->p_pid), p2, p_hash); + sx_xunlock(PIDHASHLOCK(p2->p_pid)); + tidhash_add(td2); /* @@ -969,8 +970,6 @@ fork1(struct thread *td, struct fork_req *fr) newproc->p_klist = knlist_alloc(&newproc->p_mtx); STAILQ_INIT(&newproc->p_ktr); - sx_xlock(&allproc_lock); - /* * Increment the count of procs running with this uid. Don't allow * a nonprivileged user to exceed their current limit. @@ -986,7 +985,6 @@ fork1(struct thread *td, struct fork_req *fr) return (0); fail0: error = EAGAIN; - sx_xunlock(&allproc_lock); #ifdef MAC mac_proc_destroy(newproc); #endif