From nobody Fri Jan 17 12:27:20 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 4YZJs52FGRz5kvJJ; Fri, 17 Jan 2025 12:27:21 +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 4YZJs45QyCz3Tfv; Fri, 17 Jan 2025 12:27:20 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737116840; 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=e3YvK8nBllJRUhU3PJ53XsZBWmn7wcsZ8pSi+FMCV3g=; b=BUW0BLE8xAdGtyuM+suvitJzTTCDPbQOBAc+x6furYFDgTm+/YhnMXkpA50PPfjRCf96+v XqCPzurubvrzLETl4taZ03e+0fwH4nPTt5YB/G7i/JdQN3OzmXJHQz0QgrDJbnRCVIr02h egnMTr+CH/kH/lr+iNskP8TeXxuOJkBeHREr6JojnWxEx2zhE63AYIGYSwMKBC8pIcvZYZ wdx4zMrBOFTaxbOkb1vSOXRoSuzLc4j/2t5fO5W8cMZLmt/JCyMzbW+0xCsaaGi+7Lrxg3 4MUqXLjuvREyYmY7kOAmzjb62ojdoMo26MsuqLd8vdqhat37PhXzmOIM5eYy6A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737116840; 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=e3YvK8nBllJRUhU3PJ53XsZBWmn7wcsZ8pSi+FMCV3g=; b=Cd1zh7/S+QMNFhYt6nT4/ma8kSSOigmJ258xS+rcC8eHyg3/ZVwo7nyZrf/LCSJCCCBDYH 8DA4xkVHk44FfAZ8/wIxLcMgAI20F8S8S+14SJ7Ag6BYe9w3o0+GazwEFDgUSuPnB3kPaA Px6/N3lx7hrMlmEWYKZXpilTcwNPtorbnRdOWTEdmPDYI66V8glDdupOCpsdt/kAiM2V2N x/OWHiV/eB4V2Utl6S8thAB4+wmgAmC02h84yCYHvaLOLaQseJnVBR/MvFqkpzU0zQ2k7D 5llmmw+94/2gXJieZQhQlwu/+1tS9MRj5JyBIymu03wHCa/zN6DeUpOzckwtow== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1737116840; a=rsa-sha256; cv=none; b=o7OVPw/f+KUEAK20BaUKc3jaGf2Ag0rr8flPcpSxmYdesmv2a6JhYLz7cczEGzVxjjIuVS lLLwJmFWJ/RMvRxitR2sVHn5i3MxJJfZ+m/ASjYehI2GfjzB8c6RB6AgzW0Qk1j6wEyAYe zjpfLhJAykeBerHoBWQq2jld2k52skYBLaJSm7F4uQ99V/Tdzvck3dqNWnKxKiuhVywO8Q bK1zB7m4Y6HvtofK2ICQdTx8iTgmCBtgcRBV1dZ+RBzMYa39gPEQTD4F9PYIJbk2e69rRz a/VvbYQqT4vUrctrMjpTPWPKu9rjO8L/dnGvE5G19ho9Q92XFTIOVA144QUBXg== 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 4YZJs44kQxz2rc; Fri, 17 Jan 2025 12:27:20 +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 50HCRKHc052788; Fri, 17 Jan 2025 12:27:20 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 50HCRKVg052785; Fri, 17 Jan 2025 12:27:20 GMT (envelope-from git) Date: Fri, 17 Jan 2025 12:27:20 GMT Message-Id: <202501171227.50HCRKVg052785@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: f872814e2d7a - stable/13 - 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/13 X-Git-Reftype: branch X-Git-Commit: f872814e2d7a8841411569fc707b028463c7656b Auto-Submitted: auto-generated The branch stable/13 has been updated by olce: URL: https://cgit.FreeBSD.org/src/commit/?id=f872814e2d7a8841411569fc707b028463c7656b commit f872814e2d7a8841411569fc707b028463c7656b Author: Olivier Certner AuthorDate: 2024-08-02 15:57:51 +0000 Commit: Olivier Certner CommitDate: 2025-01-17 12:24:53 +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) Additional changes for this MFC: 1. was added as an include in , as some of its types are necessary whether the header is included by the kernel or userland. Some later -CURRENT commits added it, but are not planned to be MFCed (mac_do(4) series, which doesn't exist in stable/13). 2. A number of files in 'lib/libprocstat' that include (indirectly) with _KERNEL defined were patched to include beforehand, so that 'bool', which is part of the new signature for proc_set_cred*(), is defined when is processed ( does not define it when _KERNEL is defined). --- lib/libprocstat/msdosfs.c | 1 + lib/libprocstat/smbfs.c | 2 ++ lib/libprocstat/udf.c | 2 ++ lib/libprocstat/zfs.c | 1 + 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 | 6 ++-- 9 files changed, 78 insertions(+), 33 deletions(-) diff --git a/lib/libprocstat/msdosfs.c b/lib/libprocstat/msdosfs.c index 2af34f856e50..7e73360757bc 100644 --- a/lib/libprocstat/msdosfs.c +++ b/lib/libprocstat/msdosfs.c @@ -35,6 +35,7 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ +#include #include #include diff --git a/lib/libprocstat/smbfs.c b/lib/libprocstat/smbfs.c index f705ccd207dd..854fd3eb986c 100644 --- a/lib/libprocstat/smbfs.c +++ b/lib/libprocstat/smbfs.c @@ -25,6 +25,8 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ +#include + #include #include #include diff --git a/lib/libprocstat/udf.c b/lib/libprocstat/udf.c index 43e79c39a62d..dc6cadeeea6d 100644 --- a/lib/libprocstat/udf.c +++ b/lib/libprocstat/udf.c @@ -25,6 +25,8 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ +#include + #include #include #include diff --git a/lib/libprocstat/zfs.c b/lib/libprocstat/zfs.c index 9ca43d6f6331..a84083a054db 100644 --- a/lib/libprocstat/zfs.c +++ b/lib/libprocstat/zfs.c @@ -25,6 +25,7 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ +#include #include #define _KERNEL diff --git a/sys/fs/unionfs/union_subr.c b/sys/fs/unionfs/union_subr.c index 22c8ffe88bde..56c16fc9ed6e 100644 --- a/sys/fs/unionfs/union_subr.c +++ b/sys/fs/unionfs/union_subr.c @@ -775,11 +775,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); @@ -831,7 +826,6 @@ unionfs_mkshadowdir_free_out: 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 bd820aae8197..64fef905daa4 100644 --- a/sys/kern/kern_exit.c +++ b/sys/kern/kern_exit.c @@ -976,11 +976,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. */ @@ -994,9 +989,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 413453f9bf8f..29b3e5d8d682 100644 --- a/sys/kern/kern_fork.c +++ b/sys/kern/kern_fork.c @@ -1087,7 +1087,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 755bdca4fd73..655ee776b6a7 100644 --- a/sys/kern/kern_prot.c +++ b/sys/kern/kern_prot.c @@ -564,7 +564,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); @@ -590,6 +590,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); @@ -2276,31 +2279,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; @@ -2315,6 +2363,8 @@ proc_unset_cred(struct proc *p) KASSERT(cr->cr_ref > 0, ("%s: ref %d 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); } @@ -2599,8 +2649,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. */ @@ -2608,12 +2657,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 f5e2757f7417..d8729b49c2dd 100644 --- a/sys/sys/ucred.h +++ b/sys/sys/ucred.h @@ -34,6 +34,7 @@ #ifndef _SYS_UCRED_H_ #define _SYS_UCRED_H_ +#include #if defined(_KERNEL) || defined(_WANT_UCRED) #include #include @@ -158,8 +159,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);