svn commit: r332816 - head/sys/kern

Andriy Gapon avg at FreeBSD.org
Fri Apr 20 13:08:06 UTC 2018


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 <sys/lock.h>
 #include <sys/mutex.h>
 #include <sys/racct.h>
+#include <sys/rctl.h>
 #include <sys/refcount.h>
 #include <sys/sx.h>
 #include <sys/sysent.h>
@@ -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 <sys/proc.h>
 #include <sys/queue.h>
 #include <sys/racct.h>
+#include <sys/rctl.h>
 #include <sys/refcount.h>
 #include <sys/rwlock.h>
 #include <sys/sysproto.h>
@@ -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 <sys/jail.h>
 #include <sys/pioctl.h>
 #include <sys/racct.h>
+#include <sys/rctl.h>
 #include <sys/resourcevar.h>
 #include <sys/socket.h>
 #include <sys/socketvar.h>
@@ -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:


More information about the svn-src-head mailing list