cred: proc_set_cred(), proc_unset_cred(): Update user's process count

As a process really changes credentials at the moment proc_set_cred() or
proc_unset_cred() is called, these functions are the proper locations to
perform the update of the new and old real users' process count (using
chgproccnt()).

Before this change, change_ruid() instead would perform that update,
although it operates only on a passed credential which is a priori not
tied to the calling process (or not to any process at all).  This was
arguably a flaw of commit b1fc0ec1a7, r77183, based on its commit
message, and in particular the portion "(...) In each case, the call now
acts on a credential not a process (...)".

Fixing this makes using change_ruid() more natural when building
candidate credentials that in the end are not applied to a process,
e.g., because of some intervening privilege check.  Also, it removes
a hack around this unwanted process count change in unionfs.

We also introduce the new proc_set_cred_enforce_proc_lim() so that
callers can respect the per-user process limit, and will use it for the
upcoming setcred().  We plan to change all callers of proc_set_cred() to
call this new function instead at some point.  In the meantime, both
proc_set_cred() and the new function will coexist.

As detailed in some proc_set_cred_enforce_proc_lim()'s comment, checking
against the process limit is currently flawed as the kernel doesn't
really maintain the number of processes per UID (besides RLIMIT_NPROC,
this in fact also applies to RLIMIT_KQUEUES, RLIMIT_NPTS, RLIMIT_SBSIZE
and RLIMIT_SWAP).  The applied limit is currently that of the old real
UID.  Root (or a process granted with PRIV_PROC_LIMIT) is not subject to
this limit.

Approved by:    markj (mentor)
Fixes:          b1fc0ec1a7
MFC after:      2 weeks
Sponsored by:   The FreeBSD Foundation
Differential Revision:  https://reviews.freebsd.org/D46923

sys/fs/unionfs/union_subr.c

@@ -920,11 +920,6 @@ unionfs_mkshadowdir(struct vnode *dvp, struct vnode *vp,
 	/* Authority change to root */
 	rootinfo = uifind((uid_t)0);
 	cred = crdup(cnp->cn_cred);
-	/*
-	 * The calls to chgproccnt() are needed to compensate for change_ruid()
-	 * calling chgproccnt().
-	 */
-	chgproccnt(cred->cr_ruidinfo, 1, 0);
 	change_euid(cred, rootinfo);
 	change_ruid(cred, rootinfo);
 	change_svuid(cred, (uid_t)0);
@@ -1046,7 +1041,6 @@ unionfs_mkshadowdir_relock:
 unionfs_mkshadowdir_finish:
 	unionfs_clear_in_progress_flag(vp, UNIONFS_COPY_IN_PROGRESS);
 	cnp->cn_cred = credbk;
-	chgproccnt(cred->cr_ruidinfo, -1, 0);
 	crfree(cred);
 
 	return (error);

sys/kern/kern_exit.c

@@ -999,11 +999,6 @@ proc_reap(struct thread *td, struct proc *p, int *status, int options)
 	ruadd(&q->p_stats->p_cru, &q->p_crux, &p->p_ru, &p->p_rux);
 	PROC_UNLOCK(q);
 
-	/*
-	 * Decrement the count of procs running with this uid.
-	 */
-	(void)chgproccnt(p->p_ucred->cr_ruidinfo, -1, 0);
-
 	/*
 	 * Destroy resource accounting information associated with the process.
 	 */
@@ -1017,9 +1012,10 @@ proc_reap(struct thread *td, struct proc *p, int *status, int options)
 	racct_proc_exit(p);
 
 	/*
-	 * Free credentials, arguments, and sigacts.
+	 * Free credentials, arguments, and sigacts, and decrement the count of
+	 * processes running with this uid.
 	 */
-	proc_unset_cred(p);
+	proc_unset_cred(p, true);
 	pargs_drop(p->p_args);
 	p->p_args = NULL;
 	sigacts_free(p->p_sigacts);

sys/kern/kern_fork.c

@@ -1086,7 +1086,7 @@ fail0:
 #endif
 	racct_proc_exit(newproc);
 fail1:
-	proc_unset_cred(newproc);
+	proc_unset_cred(newproc, false);
 fail2:
 	if (vm2 != NULL)
 		vmspace_free(vm2);

sys/kern/kern_prot.c

@@ -568,7 +568,7 @@ sys_setuid(struct thread *td, struct setuid_args *uap)
 #endif
 	{
 		/*
-		 * Set the real uid and transfer proc count to new user.
+		 * Set the real uid.
 		 */
 		if (uid != oldcred->cr_ruid) {
 			change_ruid(newcred, uip);
@@ -594,6 +594,9 @@ sys_setuid(struct thread *td, struct setuid_args *uap)
 		change_euid(newcred, uip);
 		setsugid(p);
 	}
+	/*
+	 * This also transfers the proc count to the new user.
+	 */
 	proc_set_cred(p, newcred);
 #ifdef RACCT
 	racct_proc_ucred_changed(p, oldcred, newcred);
@@ -2279,31 +2282,76 @@ cru2xt(struct thread *td, struct xucred *xcr)
 
 /*
  * Change process credentials.
+ *
  * Callers are responsible for providing the reference for passed credentials
- * and for freeing old ones.
+ * and for freeing old ones.  Calls chgproccnt() to correctly account the
+ * current process to the proper real UID, if the latter has changed.  Returns
+ * whether the operation was successful.  Failure can happen only on
+ * 'enforce_proc_lim' being true and if no new process can be accounted to the
+ * new real UID because of the current limit (see the inner comment for more
+ * details) and the caller does not have privilege (PRIV_PROC_LIMIT) to override
+ * that.
  */
-void
-proc_set_cred(struct proc *p, struct ucred *newcred)
+static bool
+_proc_set_cred(struct proc *p, struct ucred *newcred, bool enforce_proc_lim)
 {
-	struct ucred *cr;
+	struct ucred *const oldcred = p->p_ucred;
 
-	cr = p->p_ucred;
-	MPASS(cr != NULL);
+	MPASS(oldcred != NULL);
 	PROC_LOCK_ASSERT(p, MA_OWNED);
 	KASSERT(newcred->cr_users == 0, ("%s: users %d not 0 on cred %p",
 	    __func__, newcred->cr_users, newcred));
-	mtx_lock(&cr->cr_mtx);
-	KASSERT(cr->cr_users > 0, ("%s: users %d not > 0 on cred %p",
-	    __func__, cr->cr_users, cr));
-	cr->cr_users--;
-	mtx_unlock(&cr->cr_mtx);
+	KASSERT(newcred->cr_ref == 1, ("%s: ref %ld not 1 on cred %p",
+	    __func__, newcred->cr_ref, newcred));
+
+	if (newcred->cr_ruidinfo != oldcred->cr_ruidinfo) {
+		/*
+		 * XXXOC: This check is flawed but nonetheless the best we can
+		 * currently do as we don't really track limits per UID contrary
+		 * to what we pretend in setrlimit(2).  Until this is reworked,
+		 * we just check here that the number of processes for our new
+		 * real UID doesn't exceed this process' process number limit
+		 * (which is meant to be associated with the current real UID).
+		 */
+		const int proccnt_changed = chgproccnt(newcred->cr_ruidinfo, 1,
+		    enforce_proc_lim ? lim_cur_proc(p, RLIMIT_NPROC) : 0);
+
+		if (!proccnt_changed) {
+			if (priv_check_cred(oldcred, PRIV_PROC_LIMIT) != 0)
+				return (false);
+			(void)chgproccnt(newcred->cr_ruidinfo, 1, 0);
+		}
+	}
+
+	mtx_lock(&oldcred->cr_mtx);
+	KASSERT(oldcred->cr_users > 0, ("%s: users %d not > 0 on cred %p",
+	    __func__, oldcred->cr_users, oldcred));
+	oldcred->cr_users--;
+	mtx_unlock(&oldcred->cr_mtx);
 	p->p_ucred = newcred;
 	newcred->cr_users = 1;
 	PROC_UPDATE_COW(p);
+	if (newcred->cr_ruidinfo != oldcred->cr_ruidinfo)
+		(void)chgproccnt(oldcred->cr_ruidinfo, -1, 0);
+	return (true);
 }
 
 void
-proc_unset_cred(struct proc *p)
+proc_set_cred(struct proc *p, struct ucred *newcred)
+{
+	bool success = _proc_set_cred(p, newcred, false);
+
+	MPASS(success);
+}
+
+bool
+proc_set_cred_enforce_proc_lim(struct proc *p, struct ucred *newcred)
+{
+	return (_proc_set_cred(p, newcred, true));
+}
+
+void
+proc_unset_cred(struct proc *p, bool decrement_proc_count)
 {
 	struct ucred *cr;
 
@@ -2318,6 +2366,8 @@ proc_unset_cred(struct proc *p)
 		KASSERT(cr->cr_ref > 0, ("%s: ref %ld not > 0 on cred %p",
 		    __func__, cr->cr_ref, cr));
 	mtx_unlock(&cr->cr_mtx);
+	if (decrement_proc_count)
+		(void)chgproccnt(cr->cr_ruidinfo, -1, 0);
 	crfree(cr);
 }
 
@@ -2602,8 +2652,7 @@ change_egid(struct ucred *newcred, gid_t egid)
 /*-
  * Change a process's real uid.
  * Side effects: newcred->cr_ruid will be updated, newcred->cr_ruidinfo
- *               will be updated, and the old and new cr_ruidinfo proc
- *               counts will be updated.
+ *               will be updated.
  * References: newcred must be an exclusive credential reference for the
  *             duration of the call.
  */
@@ -2611,12 +2660,10 @@ void
 change_ruid(struct ucred *newcred, struct uidinfo *ruip)
 {
 
-	(void)chgproccnt(newcred->cr_ruidinfo, -1, 0);
 	newcred->cr_ruid = ruip->ui_uid;
 	uihold(ruip);
 	uifree(newcred->cr_ruidinfo);
 	newcred->cr_ruidinfo = ruip;
-	(void)chgproccnt(newcred->cr_ruidinfo, 1, 0);
 }
 
 /*-

sys/sys/ucred.h

@@ -156,8 +156,9 @@ void	crcopy(struct ucred *dest, struct ucred *src);
 struct ucred	*crcopysafe(struct proc *p, struct ucred *cr);
 struct ucred	*crdup(struct ucred *cr);
 void	crextend(struct ucred *cr, int n);
-void	proc_set_cred(struct proc *p, struct ucred *cr);
-void	proc_unset_cred(struct proc *p);
+void	proc_set_cred(struct proc *p, struct ucred *newcred);
+bool	proc_set_cred_enforce_proc_lim(struct proc *p, struct ucred *newcred);
+void	proc_unset_cred(struct proc *p, bool decrement_proc_count);
 void	crfree(struct ucred *cr);
 struct ucred	*crcowsync(void);
 struct ucred	*crget(void);