From owner-svn-src-all@freebsd.org Fri Apr 20 13:08:05 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id B99D4FA1D66; Fri, 20 Apr 2018 13:08:05 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 6B5EB6D230; Fri, 20 Apr 2018 13:08:05 +0000 (UTC) (envelope-from avg@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 5E71A18B27; Fri, 20 Apr 2018 13:08:05 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w3KD852X015966; Fri, 20 Apr 2018 13:08:05 GMT (envelope-from avg@FreeBSD.org) Received: (from avg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w3KD849v015961; Fri, 20 Apr 2018 13:08:04 GMT (envelope-from avg@FreeBSD.org) Message-Id: <201804201308.w3KD849v015961@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: avg set sender to avg@FreeBSD.org using -f From: Andriy Gapon Date: Fri, 20 Apr 2018 13:08:04 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r332816 - head/sys/kern X-SVN-Group: head X-SVN-Commit-Author: avg X-SVN-Commit-Paths: head/sys/kern X-SVN-Commit-Revision: 332816 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.25 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: Fri, 20 Apr 2018 13:08:06 -0000 Author: avg Date: Fri Apr 20 13:08:04 2018 New Revision: 332816 URL: https://svnweb.freebsd.org/changeset/base/332816 Log: call racct_proc_ucred_changed() under the proc lock The lock is required to ensure that the switch to the new credentials and the transfer of the process's accounting data from the old credentials to the new ones is done atomically. Otherwise, some updates may be applied to the new credentials and then additionally transferred from the old credentials if the updates happen after proc_set_cred() and before racct_proc_ucred_changed(). The problem is especially pronounced for RACCT_RSS because - there is a strict accounting for this resource (it's reclaimable) - it's updated asynchronously by the vm daemon - it's updated by setting an absolute value instead of applying a delta I had to remove a call to rctl_proc_ucred_changed() from racct_proc_ucred_changed() and make all callers of latter call the former as well. The reason is that rctl_proc_ucred_changed, as it is implemented now, cannot be called while holding the proc lock, so the lock is dropped after calling racct_proc_ucred_changed. Additionally, I've added calls to crhold / crfree around the rctl call, because without the proc lock there is no gurantee that the new credentials, owned by the process, will stay stable. That does not eliminate a possibility that the credentials passed to the rctl will get stale. Ideally, rctl_proc_ucred_changed should be able to work under the proc lock. Many thanks to kib for pointing out the above problems. PR: 222027 Discussed with: kib No comment: trasz MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D15048 Modified: head/sys/kern/kern_jail.c head/sys/kern/kern_loginclass.c head/sys/kern/kern_prot.c head/sys/kern/kern_racct.c head/sys/kern/kern_rctl.c Modified: head/sys/kern/kern_jail.c ============================================================================== --- head/sys/kern/kern_jail.c Fri Apr 20 12:40:05 2018 (r332815) +++ head/sys/kern/kern_jail.c Fri Apr 20 13:08:04 2018 (r332816) @@ -51,6 +51,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -2401,10 +2402,15 @@ do_jail_attach(struct thread *td, struct prison *pr) newcred->cr_prison = pr; proc_set_cred(p, newcred); setsugid(p); - PROC_UNLOCK(p); #ifdef RACCT racct_proc_ucred_changed(p, oldcred, newcred); + crhold(newcred); #endif + PROC_UNLOCK(p); +#ifdef RCTL + rctl_proc_ucred_changed(p, newcred); + crfree(newcred); +#endif prison_deref(oldcred->cr_prison, PD_DEREF | PD_DEUREF); crfree(oldcred); return (0); @@ -3960,6 +3966,7 @@ prison_racct_modify(struct prison *pr) */ racct_move(pr->pr_prison_racct->prr_racct, oldprr->prr_racct); +#ifdef RCTL /* * Force rctl to reattach rules to processes. */ @@ -3967,9 +3974,10 @@ prison_racct_modify(struct prison *pr) PROC_LOCK(p); cred = crhold(p->p_ucred); PROC_UNLOCK(p); - racct_proc_ucred_changed(p, cred, cred); + rctl_proc_ucred_changed(p, cred); crfree(cred); } +#endif sx_sunlock(&allproc_lock); prison_racct_free_locked(oldprr); Modified: head/sys/kern/kern_loginclass.c ============================================================================== --- head/sys/kern/kern_loginclass.c Fri Apr 20 12:40:05 2018 (r332815) +++ head/sys/kern/kern_loginclass.c Fri Apr 20 13:08:04 2018 (r332816) @@ -58,6 +58,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -230,9 +231,14 @@ sys_setloginclass(struct thread *td, struct setlogincl oldcred = crcopysafe(p, newcred); newcred->cr_loginclass = newlc; proc_set_cred(p, newcred); - PROC_UNLOCK(p); #ifdef RACCT racct_proc_ucred_changed(p, oldcred, newcred); + crhold(newcred); +#endif + PROC_UNLOCK(p); +#ifdef RCTL + rctl_proc_ucred_changed(p, newcred); + crfree(newcred); #endif loginclass_free(oldcred->cr_loginclass); crfree(oldcred); Modified: head/sys/kern/kern_prot.c ============================================================================== --- head/sys/kern/kern_prot.c Fri Apr 20 12:40:05 2018 (r332815) +++ head/sys/kern/kern_prot.c Fri Apr 20 13:08:04 2018 (r332816) @@ -66,6 +66,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -575,10 +576,15 @@ sys_setuid(struct thread *td, struct setuid_args *uap) setsugid(p); } proc_set_cred(p, newcred); - PROC_UNLOCK(p); #ifdef RACCT racct_proc_ucred_changed(p, oldcred, newcred); + crhold(newcred); #endif + PROC_UNLOCK(p); +#ifdef RCTL + rctl_proc_ucred_changed(p, newcred); + crfree(newcred); +#endif uifree(uip); crfree(oldcred); return (0); @@ -923,10 +929,15 @@ sys_setreuid(struct thread *td, struct setreuid_args * setsugid(p); } proc_set_cred(p, newcred); - PROC_UNLOCK(p); #ifdef RACCT racct_proc_ucred_changed(p, oldcred, newcred); + crhold(newcred); #endif + PROC_UNLOCK(p); +#ifdef RCTL + rctl_proc_ucred_changed(p, newcred); + crfree(newcred); +#endif uifree(ruip); uifree(euip); crfree(oldcred); @@ -1064,9 +1075,14 @@ sys_setresuid(struct thread *td, struct setresuid_args setsugid(p); } proc_set_cred(p, newcred); - PROC_UNLOCK(p); #ifdef RACCT racct_proc_ucred_changed(p, oldcred, newcred); + crhold(newcred); +#endif + PROC_UNLOCK(p); +#ifdef RCTL + rctl_proc_ucred_changed(p, newcred); + crfree(newcred); #endif uifree(ruip); uifree(euip); Modified: head/sys/kern/kern_racct.c ============================================================================== --- head/sys/kern/kern_racct.c Fri Apr 20 12:40:05 2018 (r332815) +++ head/sys/kern/kern_racct.c Fri Apr 20 13:08:04 2018 (r332816) @@ -1046,7 +1046,7 @@ racct_proc_ucred_changed(struct proc *p, struct ucred if (!racct_enable) return; - PROC_LOCK_ASSERT(p, MA_NOTOWNED); + PROC_LOCK_ASSERT(p, MA_OWNED); newuip = newcred->cr_ruidinfo; olduip = oldcred->cr_ruidinfo; @@ -1073,10 +1073,6 @@ racct_proc_ucred_changed(struct proc *p, struct ucred p->p_racct); } RACCT_UNLOCK(); - -#ifdef RCTL - rctl_proc_ucred_changed(p, newcred); -#endif } void Modified: head/sys/kern/kern_rctl.c ============================================================================== --- head/sys/kern/kern_rctl.c Fri Apr 20 12:40:05 2018 (r332815) +++ head/sys/kern/kern_rctl.c Fri Apr 20 13:08:04 2018 (r332816) @@ -1956,12 +1956,15 @@ rctl_proc_ucred_changed(struct proc *p, struct ucred * struct prison_racct *newprr; int rulecnt, i; - ASSERT_RACCT_ENABLED(); + if (!racct_enable) + return; + PROC_LOCK_ASSERT(p, MA_NOTOWNED); + newuip = newcred->cr_ruidinfo; newlc = newcred->cr_loginclass; newprr = newcred->cr_prison->pr_prison_racct; - + LIST_INIT(&newrules); again: