From nobody Fri Dec 19 09:19:22 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4dXhn72Kx6z6LBF3 for ; Fri, 19 Dec 2025 09:19:23 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R12" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4dXhn66fXmz44yK for ; Fri, 19 Dec 2025 09:19:22 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1766135962; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=aJkMJyC1zy3W7Vi6pEd7z4eYc6b7JdnfNyq1WdK6pp8=; b=NK4HKEIpKOl8xmTaT05pP675YkmIOJmOHyzJgIWEUwwq7qh2GvGOP4EcVWh66yv+5qP2db O7Vd9SJdmg6pktj8d+8gKqxHowoUElXB/fahcO5eDdU/xpGKK2Xz/W/Vy578oQqmnoonC1 nUWtsKr98Z8FyYnWgTmFtmiWEzyXxBo0C04C2exLF4vPj/7AGyy3dNmoRhtrZ/SJjWm8Mu 2sKSCI7Wx+dWZKIwfwrm3jgsuNcEs6HsNN3Ir3Twl4rsyfXQWgs27QcUaBb2mvzWpvDxC/ yheKNUc7uFooJ9wyd6XTPPsYlQ7lJCvuQ2lwh/JbyMGl6YUnvMsqWMwMXuEoUQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1766135962; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=aJkMJyC1zy3W7Vi6pEd7z4eYc6b7JdnfNyq1WdK6pp8=; b=bvADmq714LEcQZyAAXHo6UroCmMgf6Z+z8AiIyQWJ9BZv6ofcdX3zglq0tmv0HvNC5ajDg oI3HdHWu8+b0JMIu2dleJLZ5mC6wXKBPtIsK2+ABSlGWnLjJKXVNpL3POrskn5+mom956f chz6XwkgZhy1m+dMdzDWRiho/5kUn9hPenCmOK3HWK1BT3w6G6Io5t0UCXfgtM/UBUGd0u oCn1Ga/i7eaggTPC+xFHMd//iLAM4EsAmAn89guwXECNHZejBElM52B+OolSFHghCmfKoj yjoIbX26NfFZB6u/lMzursGMi7YR6tApDaUoEjygNXPLfUb7iS1vTNVj032irQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1766135962; a=rsa-sha256; cv=none; b=KToiqEf+tgZoKZJEMcYWeqd373/4GuuEWGUExRWSGJekvvnZwB4LOLLa959d4lqR+vtBP3 PV6kPG5s+O3Vwm3A2rCAMQ+VeeLC9mugNGVA9lGxUkYNyQ07JRHR1EKYFKKziXQHpkqITz ctXZVX9epBmU5HOB0Wi60QRe4Mr8AeaVfYekj9S/uFenfFwHFqbrtSj8JlpLin13FfFRw0 QHYnOD3RqIQOgXmN2qxoRxvw5/by78El+QS31TThS5B0rw1GKft1SCGmNVhCf8zmG16cdc Nk0jQkdQhxH0LpDdB+UPrEHHS5IQCHkPhziEG+GAtoVYDyO6rG+z0lqlkEBVTg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4dXhn65xlJz1Gwh for ; Fri, 19 Dec 2025 09:19:22 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 3f237 by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Fri, 19 Dec 2025 09:19:22 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Olivier Certner Subject: git: f9f1c9d73259 - stable/14 - kern: RACCT: Keep process credentials alive via references List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: olce X-Git-Repository: src X-Git-Refname: refs/heads/stable/14 X-Git-Reftype: branch X-Git-Commit: f9f1c9d732596fecc088af3ddf20bbfc5cb18094 Auto-Submitted: auto-generated Date: Fri, 19 Dec 2025 09:19:22 +0000 Message-Id: <6945189a.3f237.3e825fc6@gitrepo.freebsd.org> The branch stable/14 has been updated by olce: URL: https://cgit.FreeBSD.org/src/commit/?id=f9f1c9d732596fecc088af3ddf20bbfc5cb18094 commit f9f1c9d732596fecc088af3ddf20bbfc5cb18094 Author: Olivier Certner AuthorDate: 2025-11-03 18:21:08 +0000 Commit: Olivier Certner CommitDate: 2025-12-19 09:16:48 +0000 kern: RACCT: Keep process credentials alive via references In system calls changing process credentials, on RACCT, calls to racct_proc_ucred_changed() must be issued on the new credentials. Currently, this is done after the new credentials have been installed on the process via proc_set_cred() or proc_set_cred_enforce_proc_lim(), which modifies 'p_ucred'. Only the process lock guarantees that the new credentials pointed to by 'p_ucred' cannot themselves be concurrently modified, which would cause their 'struct ucred' to potentially lose its last reference from the process before the call to racct_proc_ucred_changed(), which needs one. For better code understandability and to avoid errors in future modifications, stop relying on proc_set_cred*() storing the passed 'struct ucred' in the process 'p_ucred' and on the process lock to avoid the reference taken by proc_set_cred*() to vanish. Instead, ensure that a reference is held when racct_proc_ucred_changed() is called. As racct_proc_ucred_changed() is actually passed explicit pointers to the old and new credentials, there is in fact no need to call it after proc_set_cred(). Instead, call it before proc_set_cred() and its taking over the reference. Since setcred() uses proc_set_cred_enforce_proc_lim(), which can fail, instead of proc_set_cred(), we instead take an additional reference with crhold(). Indeed, racct_proc_ucred_changed() should update resource accounting only if proc_set_cred_enforce_proc_lim() succeeds (an alternative would be to call it in advance and then in case of failure of the latter to call it again in order to backpedal the updated accounting, but we don't see a compelling reason to do that instead of taking an additional reference). While here, add to the documentation of proc_set_cred_enforce_proc_lim() that it does not take over the credentials reference in case of failure. While here, in racct_proc_ucred_changed()'s herald comment, add the precise condition in which this function must be called. No functional change intended. Reviewed by: kib MFC after: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D53563 (cherry picked from commit c3d2b68c6933d0610bc3e09e9b94f963b4dc85aa) --- 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, 56 insertions(+), 20 deletions(-) diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c index 801424794eac..ddfe742178c0 100644 --- a/sys/kern/kern_jail.c +++ b/sys/kern/kern_jail.c @@ -2701,14 +2701,19 @@ 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 f6070cae24d5..48abb5fd0731 100644 --- a/sys/kern/kern_loginclass.c +++ b/sys/kern/kern_loginclass.c @@ -223,13 +223,18 @@ 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 5139cbb1e102..c8fea2672565 100644 --- a/sys/kern/kern_prot.c +++ b/sys/kern/kern_prot.c @@ -814,22 +814,31 @@ kern_setcred(struct thread *const td, const u_int flags, if (error != 0) goto unlock_finish; +#ifdef RACCT /* - * Set the new credentials, noting that they have changed. + * Hold a reference to 'new_cred', as we need to call some functions on + * it after proc_set_cred_enforce_proc_lim(). */ + 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 -#ifdef RCTL - crhold(new_cred); -#endif + to_free_cred = old_cred; MPASS(error == 0); - } else + } else { +#ifdef RACCT + /* Matches the crhold() just before the containing 'if'. */ + crfree(new_cred); +#endif error = EAGAIN; + } unlock_finish: PROC_UNLOCK(p); @@ -839,10 +848,12 @@ unlock_finish: * finishing operations. */ -#ifdef RCTL +#ifdef RACCT if (cred_set) { +#ifdef RCTL rctl_proc_ucred_changed(p, new_cred); - /* Paired with the crhold() just above. */ +#endif + /* Paired with the crhold() above. */ crfree(new_cred); } #endif @@ -973,16 +984,19 @@ 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); @@ -1356,13 +1370,18 @@ 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); @@ -1504,13 +1523,18 @@ 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); @@ -2669,7 +2693,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. + * that. In this case, the reference to 'newcred' is not taken over. */ 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 0e359dfe85cf..acc365d7b921 100644 --- a/sys/kern/kern_racct.c +++ b/sys/kern/kern_racct.c @@ -1022,8 +1022,10 @@ racct_proc_exit(struct proc *p) } /* - * Called after credentials change, to move resource utilisation - * between raccts. + * 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. */ void racct_proc_ucred_changed(struct proc *p, struct ucred *oldcred,