Date: Thu, 6 Nov 2025 14:55:20 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: d7a138207fa4 - main - Revert "kern: RACCT: Keep process credentials alive via references" Message-ID: <202511061455.5A6EtKuM067522@gitrepo.freebsd.org>
index | next in thread | raw e-mail
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,help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202511061455.5A6EtKuM067522>