From nobody Sat Nov 02 20:39:30 2024 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 4XgqN269Jsz5chFp; Sat, 02 Nov 2024 20:39:30 +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 "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4XgqN24Klnz44Fw; Sat, 2 Nov 2024 20:39:30 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1730579970; 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=s08C1EMYC2omw7aGNiAozscUwotu13Kmn+yQR1zFqqY=; b=SKr3bWulw1sQJ/bZ+yqL/dp1XHrigsXRUkbr80XLdTNsC3HUrdSpaZOiuspDvIoSwRtdfd OkRVa1NSuTVjzlSb+UzVM6FTvt6JTkGmHk2eZrBGzK9zpwi5aAdJCHXRaXtE2P8ndk9Ere CdK8PNHCAGsyrNt94YRlARwjKWnmf6ybqGdNPvR5z3gOlfkVRZZMwJL7J2/90Z5m+6ckU2 f5dBZSD4SjyGHiu1H/XMT+Ipc819bmAzhHwTisQc07O/9xNzVk5Q9Hr8BhAViNCJnSG0vK qG8eR5+mMqiYk57/072sBrXVKpeQQnZgKZj3bEkKsFlr+VoM34Sj+gBey1Z9Bg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1730579970; 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=s08C1EMYC2omw7aGNiAozscUwotu13Kmn+yQR1zFqqY=; b=Q/WCoMRx5ssTUj/PGUm1mDh8cFK7EOj6OsKPoDQ0u/SPLLoJ9tkRvacen5dkHoQOzkwkYK MTG8zybGpu/+DO9MHo+tKDczsnJrftkpoBxAL/fSh/E+MYIesPJBvJabjgn5XITEjpERG+ 2vtJUAOkbijlOAGx5Gizi1YRYpQy2Cekk4P6BRP8bVPFAfcH/hQY5NWuIL54e/1U+4bAew 6Q0OSxvXhTrLyZ9XPJtsbiAdbKRG4ORj5vYWtH8kgkIdwZqZS1WF7bVugTi7PetQ5JVTSo RyQSo+I6N6WjmduiQvRn1McvmVw/LC1rV1z9jZxXKzC2dWbhe5iT0rCG/HHFuA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1730579970; a=rsa-sha256; cv=none; b=xmuST4CGZyOSsjBikeiqM+oWtXVCQJ2U1kw8agK89OsWesVEAZfj4wRy20uGg8BrDL0LEz NnSme6IBJdAvk3A3U9282B2UR0FNZ7bFkT8yFvheechjG2i44Fk73ll6VnVJVJ72KppvTf 7zpLdIa0iEHBxNO0c+RhF/B9cM9K+6gusIapEtHXjoUchjbtT0hZ5KGdLejFmR5e3soIlR TC/aOl0OjXuG2c4I8hDDftZNCK2bf2vSGYpMf0vly7R6Pp4zcKEwp3nk84wGNvFckFIbZT UAICs2ztLtbu56Grz3ezxbqBSAu0gxRs7qtmEAtiKqxHbH72akW50ZLbmJCNzw== 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 4XgqN23xx6zKFB; Sat, 2 Nov 2024 20:39:30 +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 4A2KdU6U046272; Sat, 2 Nov 2024 20:39:30 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 4A2KdUR1046269; Sat, 2 Nov 2024 20:39:30 GMT (envelope-from git) Date: Sat, 2 Nov 2024 20:39:30 GMT Message-Id: <202411022039.4A2KdUR1046269@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Olivier Certner Subject: git: abd39811cd7e - main - cred: kern_setgroups(): Internally use int as number of groups' type 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: abd39811cd7e4bb928da503f4a5c79364ac8d0f5 Auto-Submitted: auto-generated The branch main has been updated by olce: URL: https://cgit.FreeBSD.org/src/commit/?id=abd39811cd7e4bb928da503f4a5c79364ac8d0f5 commit abd39811cd7e4bb928da503f4a5c79364ac8d0f5 Author: Olivier Certner AuthorDate: 2024-10-01 16:46:46 +0000 Commit: Olivier Certner CommitDate: 2024-11-02 20:37:41 +0000 cred: kern_setgroups(): Internally use int as number of groups' type sys_setgroups() (and sys_getgroups()) was changed in commit "kern: fail getgroup and setgroup with negative int" (4bc2174a1b48) to take the number of groups as an 'int' (for sys_getgroups(), POSIX mandates this change; for sys_setgroups(), which it does not standardize, it's arguably for consistency). All our internal APIs related to groups on 'struct ucred', as well as related members on the latter, treat that number as an 'int' as well (and not a 'u_int'). Consequently, to avoid surprises, change kern_setgroups() to behave the same, and fix audit_arg_groupset() accordingly. With that change, everything is handled with signed integers internally. Update sanity checks accordingly. Reviewed by: mhorne Approved by: markj (mentor) MFC after: 3 days Differential Revision: https://reviews.freebsd.org/D46912 --- sys/kern/kern_prot.c | 16 ++++++++++++++-- sys/security/audit/audit.h | 2 +- sys/security/audit/audit_arg.c | 8 ++++---- sys/sys/syscallsubr.h | 2 +- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c index 7ca08c3cf490..67e4428b039e 100644 --- a/sys/kern/kern_prot.c +++ b/sys/kern/kern_prot.c @@ -815,6 +815,15 @@ sys_setgroups(struct thread *td, struct setgroups_args *uap) gid_t *groups; int gidsetsize, error; + /* + * Sanity check size now to avoid passing too big a value to copyin(), + * even if kern_setgroups() will do it again. + * + * Ideally, the 'gidsetsize' argument should have been a 'u_int' (and it + * was, in this implementation, for a long time), but POSIX standardized + * getgroups() to take an 'int' and it would be quite entrapping to have + * setgroups() differ. + */ gidsetsize = uap->gidsetsize; if (gidsetsize > ngroups_max + 1 || gidsetsize < 0) return (EINVAL); @@ -843,13 +852,16 @@ gidp_cmp(const void *p1, const void *p2) } int -kern_setgroups(struct thread *td, u_int ngrp, gid_t *groups) +kern_setgroups(struct thread *td, int ngrp, gid_t *groups) { struct proc *p = td->td_proc; struct ucred *newcred, *oldcred; int error; - MPASS(ngrp <= ngroups_max + 1); + /* Sanity check size. */ + if (ngrp < 0 || ngrp > ngroups_max + 1) + return (EINVAL); + AUDIT_ARG_GROUPSET(groups, ngrp); newcred = crget(); crextend(newcred, ngrp); diff --git a/sys/security/audit/audit.h b/sys/security/audit/audit.h index e7a9c83afbb3..b87dd52e0773 100644 --- a/sys/security/audit/audit.h +++ b/sys/security/audit/audit.h @@ -98,7 +98,7 @@ void audit_arg_rgid(gid_t rgid); void audit_arg_ruid(uid_t ruid); void audit_arg_sgid(gid_t sgid); void audit_arg_suid(uid_t suid); -void audit_arg_groupset(gid_t *gidset, u_int gidset_size); +void audit_arg_groupset(gid_t *gidset, int gidset_size); void audit_arg_login(char *login); void audit_arg_ctlname(int *name, int namelen); void audit_arg_mask(int mask); diff --git a/sys/security/audit/audit_arg.c b/sys/security/audit/audit_arg.c index c8ae56e87487..c667d3968817 100644 --- a/sys/security/audit/audit_arg.c +++ b/sys/security/audit/audit_arg.c @@ -263,13 +263,13 @@ audit_arg_suid(uid_t suid) } void -audit_arg_groupset(gid_t *gidset, u_int gidset_size) +audit_arg_groupset(gid_t *gidset, int gidset_size) { - u_int i; + int i; struct kaudit_record *ar; - KASSERT(gidset_size <= ngroups_max + 1, - ("audit_arg_groupset: gidset_size > (kern.ngroups + 1)")); + KASSERT(gidset_size >= 0 && gidset_size <= ngroups_max + 1, + ("audit_arg_groupset: gidset_size < 0 or > (kern.ngroups + 1)")); ar = currecord(); if (ar == NULL) diff --git a/sys/sys/syscallsubr.h b/sys/sys/syscallsubr.h index 2e0a362f90ad..6ee7c6d802c4 100644 --- a/sys/sys/syscallsubr.h +++ b/sys/sys/syscallsubr.h @@ -320,7 +320,7 @@ int kern_select(struct thread *td, int nd, fd_set *fd_in, fd_set *fd_ou, fd_set *fd_ex, struct timeval *tvp, int abi_nfdbits); int kern_sendit(struct thread *td, int s, struct msghdr *mp, int flags, struct mbuf *control, enum uio_seg segflg); -int kern_setgroups(struct thread *td, u_int ngrp, gid_t *groups); +int kern_setgroups(struct thread *td, int ngrp, gid_t *groups); int kern_setitimer(struct thread *, u_int, struct itimerval *, struct itimerval *); int kern_setpriority(struct thread *td, int which, int who, int prio);