From nobody Thu Nov 27 10:06:04 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 4dHBs92fncz6J1K6 for ; Thu, 27 Nov 2025 10:06:05 +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 4dHBs91G1Jz41nt for ; Thu, 27 Nov 2025 10:06:05 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1764237965; 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=2LGVWUuZsRAtFMxLLzYbUQ1hZUqdUs0rg2QGz71ciIU=; b=Mzt9X4ASRRJiB89XJfrxBBZhvMa1DCdxtbHM1x2OKt0PIAyX0hnx3Ggq07Zu9sxTdhAiRm 8UEOJAVYn4l16VwCZ0nlYZdWzPGHbcGDSx3N6EKMY1aPL4HsoNQQgADJDo47t+/6dvJRh9 D0A2QBZmh/6R5JnpmfO5JV+4X1ugJUDADpd5cC4L92VXkmwIBzAWpTswanQV0ln9n7Oyp2 lGkLik7HQKX2+Ki87pn6x3+cRdna0uUHruTXytjddxt5xVquBzEOgIiSONX/eV1LMWst9A F7ojOnERAHFIMJ8iux8+gqtwYo5CtgtLrC9qxuSUIL0OSr64xEGnlpfnMNonpw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1764237965; 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=2LGVWUuZsRAtFMxLLzYbUQ1hZUqdUs0rg2QGz71ciIU=; b=j1y08vqK8GvfNpcjBaKjDQSXGMN2ae0u0v8ywsOOCryQR8zJMuHwY87HaB8ykBMGMWfA0M D6mdmBYtApijYp9F0wwbcl85KGgFTxI4GTVag5RsPl5HFso76UNlNQUXHq9jWN9vWOnj3i HDVFZTNMi7KfG/AUd3mr8Zol8Dt5KWzHwhfO9vTYFkQW4LWzEq+fTpZ//i3wOal3kMaUfn fl63eOC9COmFgn/f9jFbcmXWnUyQxMRI++U3i8uPzwleiS11VP4NJEczVnUNxyAuoFhX9e DhdLsirzPxZOcLL2xe1K26hkpbuxe249Am7n3lbRAnrb1uDT9trQbQfYOd1ZHQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1764237965; a=rsa-sha256; cv=none; b=ckpKV5DD5Cs3m1BCfDlgdhd1Ateg5d1vWUqO+Z6RpCwOInG8FAHZe3SRqVE9+jyyqUhZOd ThjlEt8hX89ChxX4UtztgmTKlKfNnBGWY/UYfKwvG3kemUJJAQ3lEAO4C7EP6jiD9mOIJj ABGGKpRwbXGH/IPUFDPEJBxQjE/wJvnCcFwA/tkuHrh0tUo7OwV4xg7RW8PHAKSr3S5vLc kAU5OuQ5zcKS15jqRAj0ZkLk72KN8WX9i887CY0wqb/1XgI0BTFyfUczWfTEgp3UqCfEN3 24OdJfx7rM/Oj+fON7B0xflOByezodYzsazEGQuQ5sqGBTbG7PcJ/A7KLcRNCA== 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 4dHBs90GlRz1QKw for ; Thu, 27 Nov 2025 10:06:05 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 325cb by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Thu, 27 Nov 2025 10:06:04 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Olivier Certner Subject: git: 000d5b52c19f - main - setcred(2): Fix a panic on too many groups from latest commit 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/main X-Git-Reftype: branch X-Git-Commit: 000d5b52c19ff3858a6f0cbb405d47713c4267a4 Auto-Submitted: auto-generated Date: Thu, 27 Nov 2025 10:06:04 +0000 Message-Id: <6928228c.325cb.6de6fb72@gitrepo.freebsd.org> The branch main has been updated by olce: URL: https://cgit.FreeBSD.org/src/commit/?id=000d5b52c19ff3858a6f0cbb405d47713c4267a4 commit 000d5b52c19ff3858a6f0cbb405d47713c4267a4 Author: Olivier Certner AuthorDate: 2025-11-27 09:04:50 +0000 Commit: Olivier Certner CommitDate: 2025-11-27 10:05:50 +0000 setcred(2): Fix a panic on too many groups from latest commit kern_setcred_copyin_supp_groups() is documented to always set 'sc_supp_groups', but did not do it if there are more supplementary groups than 'ngroups_max'. Also, that case was omitted from the herald comment. Add it there, also including it as a case where 'sc_supp_groups_nb' is reset to 0 as a security measure. Initially, kern_setcred_copyin_supp_groups() had the usual property that nothing had to be freed on it returning an error, but was then converted to relying on the caller to free() even on error, and this part was missed during the conversion. The benefits of this unusual convention are that we can zero or NULLify groups-related attributes in advance, preventing inadvertent use of stale data (defensive security measure), and we can avoid some small code duplication (no need to have two same calls to free()). This makes sense as kern_setcred_copyin_supp_groups() is meant to be a private sub-routine of user_setcred() only. While here, rename kern_setcred_copyin_supp_groups() => user_setcred_copyin_supp_groups(). Reported by: pho Fixes: 4cd93df95e69 ("setcred(): Remove an optimization for when cr_groups[0] was the egid") Sponsored by: The FreeBSD Foundation --- sys/kern/kern_prot.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c index 34d68927be71..b1e4b731145e 100644 --- a/sys/kern/kern_prot.c +++ b/sys/kern/kern_prot.c @@ -529,44 +529,54 @@ gidp_cmp(const void *p1, const void *p2) * 'smallgroups' must be an (uninitialized) array of length CRED_SMALLGROUPS_NB. * Always sets 'sc_supp_groups', either to a valid kernel-space groups array * (which may or may not be 'smallgroups'), or NULL if SETCREDF_SUPP_GROUPS was - * not specified, or a buffer containing garbage on copyin() failure. In the - * last two cases, 'sc_supp_groups_nb' is additionally set to 0 as a security - * measure. 'sc_supp_groups' must be freed (M_TEMP) if not equal to - * 'smallgroups' even on failure. + * not specified or there are too many groups, or a buffer containing garbage on + * copyin() failure. In the last two cases, 'sc_supp_groups_nb' is additionally + * set to 0 as a security measure. 'sc_supp_groups' must be freed (M_TEMP) if + * not equal to 'smallgroups' even on failure. */ static int -kern_setcred_copyin_supp_groups(struct setcred *const wcred, +user_setcred_copyin_supp_groups(struct setcred *const wcred, const u_int flags, gid_t *const smallgroups) { gid_t *groups; int error; if ((flags & SETCREDF_SUPP_GROUPS) == 0) { - wcred->sc_supp_groups_nb = 0; - wcred->sc_supp_groups = NULL; - return (0); + error = 0; + goto reset_groups_exit; } /* * Check the number of groups' limit right now in order to limit the * amount of bytes to copy. */ - if (wcred->sc_supp_groups_nb > ngroups_max) - return (EINVAL); + if (wcred->sc_supp_groups_nb > ngroups_max) { + error = EINVAL; + goto reset_groups_exit; + } groups = wcred->sc_supp_groups_nb <= CRED_SMALLGROUPS_NB ? smallgroups : malloc(wcred->sc_supp_groups_nb * sizeof(gid_t), M_TEMP, M_WAITOK); - error = copyin(wcred->sc_supp_groups, groups, wcred->sc_supp_groups_nb * sizeof(gid_t)); wcred->sc_supp_groups = groups; + if (error != 0) { wcred->sc_supp_groups_nb = 0; + /* + * 'sc_supp_groups' must be freed by caller if not + * 'smallgroups'. + */ return (error); } return (0); + +reset_groups_exit: + wcred->sc_supp_groups_nb = 0; + wcred->sc_supp_groups = NULL; + return (error); } int @@ -601,7 +611,7 @@ user_setcred(struct thread *td, const u_int flags, struct setcred *const wcred) * alternative for 32-bit compatibility as 'gid_t' has the same size * everywhere. */ - error = kern_setcred_copyin_supp_groups(wcred, flags, smallgroups); + error = user_setcred_copyin_supp_groups(wcred, flags, smallgroups); if (error != 0) goto free_groups;