git: d7a138207fa4 - main - Revert "kern: RACCT: Keep process credentials alive via references"

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Thu, 06 Nov 2025 14:55:20 UTC
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=d7a138207fa4a2ff077d5d276f6413f1d8130032

commit d7a138207fa4a2ff077d5d276f6413f1d8130032
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2025-11-06 14:48:57 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2025-11-06 14:48:57 +0000

    Revert "kern: RACCT: Keep process credentials alive via references"
    
    The change causes a panic on boot with INVARIANTS kernels.  Revert for
    now.
    
    This reverts commit a5d1a0c9bfcca38528b861c5afb51ea9b1696b65.
    
    Reported by:    syzbot+74624c6fcbb384ea0113@syzkaller.appspotmail.com
---
 sys/kern/kern_jail.c       |  9 ++------
 sys/kern/kern_loginclass.c |  7 +-----
 sys/kern/kern_prot.c       | 54 +++++++++++++---------------------------------
 sys/kern/kern_racct.c      |  6 ++----
 4 files changed, 20 insertions(+), 56 deletions(-)

diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c
index 1b9bd4cf62d5..523b7e314a10 100644
--- a/sys/kern/kern_jail.c
+++ b/sys/kern/kern_jail.c
@@ -3043,19 +3043,14 @@ do_jail_attach(struct thread *td, struct prison *pr, int drflags)
 	PROC_LOCK(p);
 	oldcred = crcopysafe(p, newcred);
 	newcred->cr_prison = pr;
+	proc_set_cred(p, newcred);
+	setsugid(p);
 #ifdef RACCT
 	racct_proc_ucred_changed(p, oldcred, newcred);
 #endif
 #ifdef RCTL
 	crhold(newcred);
 #endif
-	/*
-	 * Takes over 'newcred''s reference, so 'newcred' must not be used
-	 * besides this point except on RCTL where we took an additional
-	 * reference above.
-	 */
-	proc_set_cred(p, newcred);
-	setsugid(p);
 	PROC_UNLOCK(p);
 #ifdef RCTL
 	rctl_proc_ucred_changed(p, newcred);
diff --git a/sys/kern/kern_loginclass.c b/sys/kern/kern_loginclass.c
index 07d388f18f8d..0c111c4f78d8 100644
--- a/sys/kern/kern_loginclass.c
+++ b/sys/kern/kern_loginclass.c
@@ -222,18 +222,13 @@ sys_setloginclass(struct thread *td, struct setloginclass_args *uap)
 	PROC_LOCK(p);
 	oldcred = crcopysafe(p, newcred);
 	newcred->cr_loginclass = newlc;
+	proc_set_cred(p, newcred);
 #ifdef RACCT
 	racct_proc_ucred_changed(p, oldcred, newcred);
 #endif
 #ifdef RCTL
 	crhold(newcred);
 #endif
-	/*
-	 * Takes over 'newcred''s reference, so 'newcred' must not be used
-	 * besides this point except on RCTL where we took an additional
-	 * reference above.
-	 */
-	proc_set_cred(p, newcred);
 	PROC_UNLOCK(p);
 #ifdef RCTL
 	rctl_proc_ucred_changed(p, newcred);
diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
index f93cee6d7698..3c145851b683 100644
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -832,31 +832,22 @@ kern_setcred(struct thread *const td, const u_int flags,
 	if (error != 0)
 		goto unlock_finish;
 
-#ifdef RACCT
 	/*
-	 * Hold a reference to 'new_cred', as we need to call some functions on
-	 * it after proc_set_cred_enforce_proc_lim().
+	 * Set the new credentials, noting that they have changed.
 	 */
-	crhold(new_cred);
-#endif
-
-	/* Set the new credentials. */
 	cred_set = proc_set_cred_enforce_proc_lim(p, new_cred);
 	if (cred_set) {
 		setsugid(p);
+		to_free_cred = old_cred;
 #ifdef RACCT
-		/* Adjust RACCT counters. */
 		racct_proc_ucred_changed(p, old_cred, new_cred);
 #endif
-		to_free_cred = old_cred;
-		MPASS(error == 0);
-	} else {
-#ifdef RACCT
-		/* Matches the crhold() just before the containing 'if'. */
-		crfree(new_cred);
+#ifdef RCTL
+		crhold(new_cred);
 #endif
+		MPASS(error == 0);
+	} else
 		error = EAGAIN;
-	}
 
 unlock_finish:
 	PROC_UNLOCK(p);
@@ -866,12 +857,10 @@ unlock_finish:
 	 * finishing operations.
 	 */
 
-#ifdef RACCT
-	if (cred_set) {
 #ifdef RCTL
+	if (cred_set) {
 		rctl_proc_ucred_changed(p, new_cred);
-#endif
-		/* Paired with the crhold() above. */
+		/* Paired with the crhold() just above. */
 		crfree(new_cred);
 	}
 #endif
@@ -1002,19 +991,16 @@ 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);
 #endif
 #ifdef RCTL
 	crhold(newcred);
 #endif
-	/*
-	 * Takes over 'newcred''s reference, so 'newcred' must not be used
-	 * besides this point except on RCTL where we took an additional
-	 * reference above.
-	 */
-	proc_set_cred(p, newcred);
 	PROC_UNLOCK(p);
 #ifdef RCTL
 	rctl_proc_ucred_changed(p, newcred);
@@ -1418,18 +1404,13 @@ sys_setreuid(struct thread *td, struct setreuid_args *uap)
 		change_svuid(newcred, newcred->cr_uid);
 		setsugid(p);
 	}
+	proc_set_cred(p, newcred);
 #ifdef RACCT
 	racct_proc_ucred_changed(p, oldcred, newcred);
 #endif
 #ifdef RCTL
 	crhold(newcred);
 #endif
-	/*
-	 * Takes over 'newcred''s reference, so 'newcred' must not be used
-	 * besides this point except on RCTL where we took an additional
-	 * reference above.
-	 */
-	proc_set_cred(p, newcred);
 	PROC_UNLOCK(p);
 #ifdef RCTL
 	rctl_proc_ucred_changed(p, newcred);
@@ -1571,18 +1552,13 @@ sys_setresuid(struct thread *td, struct setresuid_args *uap)
 		change_svuid(newcred, suid);
 		setsugid(p);
 	}
+	proc_set_cred(p, newcred);
 #ifdef RACCT
 	racct_proc_ucred_changed(p, oldcred, newcred);
 #endif
 #ifdef RCTL
 	crhold(newcred);
 #endif
-	/*
-	 * Takes over 'newcred''s reference, so 'newcred' must not be used
-	 * besides this point except on RCTL where we took an additional
-	 * reference above.
-	 */
-	proc_set_cred(p, newcred);
 	PROC_UNLOCK(p);
 #ifdef RCTL
 	rctl_proc_ucred_changed(p, newcred);
@@ -2807,7 +2783,7 @@ cru2xt(struct thread *td, struct xucred *xcr)
  * '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.  In this case, the reference to 'newcred' is not taken over.
+ * that.
  */
 static bool
 _proc_set_cred(struct proc *p, struct ucred *newcred, bool enforce_proc_lim)
diff --git a/sys/kern/kern_racct.c b/sys/kern/kern_racct.c
index d1324935bdc3..17b64ad00bb5 100644
--- a/sys/kern/kern_racct.c
+++ b/sys/kern/kern_racct.c
@@ -949,10 +949,8 @@ racct_proc_exit(struct proc *p)
 }
 
 /*
- * Called to signal credentials change, to move resource utilisation
- * between raccts.  Must be called with the proc lock held, in the same span as
- * the credentials change itself (i.e., without the proc lock being unlocked
- * between the two), but the order does not matter.
+ * Called after credentials change, to move resource utilisation
+ * between raccts.
  */
 void
 racct_proc_ucred_changed(struct proc *p, struct ucred *oldcred,