From nobody Thu Jan 16 18:08:30 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 4YYrTC2Rhzz5kksd; Thu, 16 Jan 2025 18:08:31 +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 "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4YYrTC18nyz3RhS; Thu, 16 Jan 2025 18:08:31 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737050911; 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=BWzCOOpCKArjYDfX1tpj0fbSL38UCi0ErbCwHNTZO6U=; b=C6mF28nreHpoz05XhKgxxOV9j/HWM4UCY4OQeEtiVw+EHUVSS2ODkRsF8kO3IjZFdWl92W seNNjWDe4u5XGK1Jgg/IpYQwqYarEnjrTgqm1vMrntjyrcfFWKz16QUf66d0MIk+4k+E9F HCjAYjig48ZvHeKWc6MdfotbnB+17UakchP2SZOlSJjR5jFEIlqtnNPFl3F/6yKZmTgx/u w9xXoT8z+dsrAEu1fg5Y+oUCbSgThFtkcvuEFq1Fq5fAz7cha/AwnXXV7aPnlFPQlQirqm c4EhY0YMWhccbS+7LXOT5d1veh0yCPugRx8p5touKgAThXqHRXxZatME+V9UDQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737050911; 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=BWzCOOpCKArjYDfX1tpj0fbSL38UCi0ErbCwHNTZO6U=; b=E7tRJYiyJ6nBBpMgvQVQDLtCVtq0FClWgwqKj7UYfylRJAKrUP1ijzFYleZfx2m86wLmK/ FyJ/bwE7/RU4+uULm0Gik2FHjUjWqqepHw15r1tLTUKu1N6rt2jTmhn7NAUga8hZ0bGrqS N2c0aGd8Jr/T/r2yxDxrLDSDJjipf+8Lk/jINOdwLeRZJ6b7dHGK00d9rxe0OyGE7XCLf5 DQpezDRUE0pl+gfpNt5Kl2q9CLQhjsymlY/HufhHwvAO0YcOgWXwzkyrNcz87PY8LtgRJ6 JkiQui0YMQB8hdrCwe1aa42iyE0nQ6JBE8SQxEgxhgnQnmwIZuRicL23hdEusw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1737050911; a=rsa-sha256; cv=none; b=TbLa1u41PM8fUvb0Ialeh0nuCNSHdu63Oh3NHQpwYfqcZeH7C+EYaoz4aXlTzsRdr9wLUL mhrd7STBbCqtI+1darVjw6IRaskPaeqBSmDLyRcCM2fFuh5hh+OSQy92SdSmT3QAI4Rv7J /nI1ArgjZp/5YsaWvdaGZvuDD3vvJzpCQ52klIsA1k8ovWav2UOVcahyFHcoyfmi1pnP/U mAIThp7l1MLLBzLV51zLavD160RXtIV0/OBsMTnbkU0SqB5kXMnqsjOZMf+gyq6VoHbgFZ b29GAs15quTiI9f0yxOHSYUVaCS8+fjKHmdmt552lJzbh2dmc0/jMUU7zY4BwQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (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 did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4YYrTC0TBzzjs2; Thu, 16 Jan 2025 18:08:31 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 50GI8UrV092531; Thu, 16 Jan 2025 18:08:30 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 50GI8UE7092528; Thu, 16 Jan 2025 18:08:30 GMT (envelope-from git) Date: Thu, 16 Jan 2025 18:08:30 GMT Message-Id: <202501161808.50GI8UE7092528@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Olivier Certner Subject: git: b6c9ff04fcb4 - stable/14 - cred: proc_set_cred(), proc_unset_cred(): Update user's process count 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: b6c9ff04fcb4147f8071f421aa33099eaabe371a Auto-Submitted: auto-generated The branch stable/14 has been updated by olce: URL: https://cgit.FreeBSD.org/src/commit/?id=b6c9ff04fcb4147f8071f421aa33099eaabe371a commit b6c9ff04fcb4147f8071f421aa33099eaabe371a Author: Olivier Certner AuthorDate: 2024-08-02 15:57:51 +0000 Commit: Olivier Certner CommitDate: 2025-01-16 18:06:59 +0000 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 b1fc0ec1a7a49ded, 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: b1fc0ec1a7a49ded MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D46923 (cherry picked from commit d2be7ed63affd8af5fe6203002b7cc3cbe7f7891) --- sys/fs/unionfs/union_subr.c | 6 ---- sys/kern/kern_exit.c | 10 ++---- sys/kern/kern_fork.c | 2 +- sys/kern/kern_prot.c | 81 +++++++++++++++++++++++++++++++++++---------- sys/sys/ucred.h | 5 +-- 5 files changed, 71 insertions(+), 33 deletions(-) diff --git a/sys/fs/unionfs/union_subr.c b/sys/fs/unionfs/union_subr.c index ae2d417758f0..c294e9b70ad9 100644 --- a/sys/fs/unionfs/union_subr.c +++ b/sys/fs/unionfs/union_subr.c @@ -884,11 +884,6 @@ unionfs_mkshadowdir(struct unionfs_mount *ump, struct vnode *udvp, /* 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); @@ -958,7 +953,6 @@ unionfs_mkshadowdir(struct unionfs_mount *ump, struct vnode *udvp, unionfs_mkshadowdir_abort: cnp->cn_cred = credbk; - chgproccnt(cred->cr_ruidinfo, -1, 0); crfree(cred); return (error); diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c index f9445a481d92..788b58da450d 100644 --- a/sys/kern/kern_exit.c +++ b/sys/kern/kern_exit.c @@ -1001,11 +1001,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. */ @@ -1019,9 +1014,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); diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c index 8054f93c2c9d..8f11bc246593 100644 --- a/sys/kern/kern_fork.c +++ b/sys/kern/kern_fork.c @@ -1098,7 +1098,7 @@ fail0: #endif racct_proc_exit(newproc); fail1: - proc_unset_cred(newproc); + proc_unset_cred(newproc, false); fail2: if (vm2 != NULL) vmspace_free(vm2); diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c index bf026593a3ee..7c5ce97fef93 100644 --- a/sys/kern/kern_prot.c +++ b/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_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) +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); } /*- diff --git a/sys/sys/ucred.h b/sys/sys/ucred.h index 8724cfcdc1e6..5aceb28e5940 100644 --- a/sys/sys/ucred.h +++ b/sys/sys/ucred.h @@ -158,8 +158,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);