From nobody Thu Jul 31 19:26:45 2025 X-Original-To: dev-commits-src-main@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 4btJx30wcCz62x1Z; Thu, 31 Jul 2025 19:26:47 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (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 "smtp.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4btJx2733dz3qYL; Thu, 31 Jul 2025 19:26:46 +0000 (UTC) (envelope-from kevans@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1753990007; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=NdRS69KJLXpZrTyUfv15PRQmLkmWokYhXP8JfVhxUDE=; b=GOuE3HLsCM5VHdvbaKxo5xZgTMnKnOH2X5JWciXEJ2TPRpGSjdZm7ThkX4I7qHrtIueE8G MAJ5OBLxbsme7i1FdUOv1pn5kG1oVYq+3uK4dtEGvfqi6IMRMXUzy3D9iramb/pxndL/TL d3RsOUdJ9yI9C1HRPkarHyBEoaZiELOmu2i4EfgXdR1WM+Ra38h5fB3d8ddEExxa0ACf4O dHScIW1UVRpTSrly8MguWFtd0nl/hvlMe79WNIe3ZZwYFAxB6VSxPz9dCn6glP4P/aHE2k lJl5kjkMMP8701VTB5GEECQ0JBRWhNj2pzDDeRvDCYLXcpbB6FdQ9TCf7IR7YA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1753990007; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=NdRS69KJLXpZrTyUfv15PRQmLkmWokYhXP8JfVhxUDE=; b=uf9cZyAakvrFZbGXuMTtW1XoRGZXHoxAwTSFXlVjliy2WkaEE5RHVmpHObcQRzm5RpTrAW A0UrLTq9vXdUtb+VKEMLMCWDdW3J5T4KfR+12Recas1mjN0IIoh7mEDchzxTIqtAj0T7wh zCoQEoqguugVDFC3NioJDP3+9qpEaAbIpIu8H932tJay13L1PnxnYSU/piALxuKAmWJ4lO ByyI+AsDaKlvTwMWg22VseHIcyxOLI0leW1lFB2+iQmyvaYWPu2xUIa6JEii7R2wiy6M/i jLfNOVxq7KbHL16CdYtdS5fpXtenZobbiTEm7+uKDSIzNdCzxUyAoH4qnKpikw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1753990007; a=rsa-sha256; cv=none; b=RCk3yZeN4cwAwFzSpt00F2ijJ5551zcbESG/yJ7/jqxz1lx4dwtCBEaErHdxeT+YXTPmfx vExDsZTXrPNmHndd5+CHnc+7v38k0Hf0l27TtKrXItQD/xt+tvEQdrs9bKnasaXyjQcrEC F/iuIDf63gGc9OrccOKEL2unHX9wxL9E4ExUIkkS/0eht4nOYAfeJSwwtxWTudSSQAyKSS XRbI1o5cluncMSUGxMGiFxwsibtL9MdLYX4srOkaqWL6RCtgrKM6B4TlUXdYwnjypHQeeu NO60C/GLJFS5bsfNNlZjhJqBhrOT3gm+IJ6fGxbTUpb4fFHNbI5Ttmdj8azP1Q== Received: from [10.9.4.95] (unknown [209.182.120.176]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (Client did not present a certificate) (Authenticated sender: kevans/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id 4btJx23F7CzMGJ; Thu, 31 Jul 2025 19:26:46 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Message-ID: <8876e655-5e8f-4050-84a9-ec9b1ddc2a35@FreeBSD.org> Date: Thu, 31 Jul 2025 14:26:45 -0500 List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: git: be1f7435ef21 - main - kern: start tracking cr_gid outside of cr_groups[] From: Kyle Evans To: Mark Johnston , olce@freebsd.org Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org References: <202507310444.56V4icA3054832@gitrepo.freebsd.org> <4184e804-b731-4b4e-9399-d27f776f575d@FreeBSD.org> Content-Language: en-US In-Reply-To: <4184e804-b731-4b4e-9399-d27f776f575d@FreeBSD.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 7/31/25 14:00, Kyle Evans wrote: > On 7/31/25 13:46, Mark Johnston wrote: >> On Thu, Jul 31, 2025 at 04:44:38AM +0000, Kyle Evans wrote: >>> The branch main has been updated by kevans: >>> >>> URL: https://cgit.FreeBSD.org/src/commit/? >>> id=be1f7435ef218b1df35aebf3b90dd65ffd8bbe51 >>> >>> commit be1f7435ef218b1df35aebf3b90dd65ffd8bbe51 >>> Author:     Kyle Evans >>> AuthorDate: 2025-07-31 04:44:11 +0000 >>> Commit:     Kyle Evans >>> CommitDate: 2025-07-31 04:44:11 +0000 >>> >>>      kern: start tracking cr_gid outside of cr_groups[] >>>      This is the (mostly) kernel side of de-conflating cr_gid and the >>>      supplemental groups.  The pre-existing behavior for getgroups() and >>>      setgroups() is retained to keep the user <-> kernel boundary >>>      functionally the same while we audit use of these syscalls, but >>> we can >>>      remove a lot of the internal special-casing just by reorganizing >>> ucred >>>      like this. >>>      struct xucred has been altered because the cr_gid macro becomes >>>      problematic if ucred has a real cr_gid member but xucred does >>> not.  Most >>>      notably, they both also have cr_groups[] members, so the definition >>>      means that we could easily have situations where we end up using >>> the >>>      first supplemental group as the egid in some places.  We really >>> can't >>>      change the ABI of xucred, so instead we alias the first member >>> to the >>>      `cr_gid` name and maintain the status quo. >>>      This also fixes the Linux setgroups(2)/getgroups(2) >>> implementation to >>>      more cleanly preserve the group set, now that we don't need to >>> special >>>      case cr_groups[0]. >>>      __FreeBSD_version bumped for the `struct ucred` ABI break. >>>      For relnotes: downstreams and out-of-tree modules absolutely >>> must fix >>>      any references to cr_groups[0] in their code.  These are almost >>>      exclusively incorrect in the new world, and cr_gid should be used >>>      instead.  There is a cr_gid macro available in earlier FreeBSD >>> versions >>>      that can be used to avoid having version-dependant conditionals >>> to refer >>>      to the effective group id.  Surrounding code may need adjusted >>> if it >>>      peels off the first element of cr_groups and uses the others as the >>>      supplemental groups, since the supplemental groups start at >>> cr_groups[0] >>>      now if &cr_groups[0] != &cr_gid. >>>      Relnotes:       yes (see last paragraph) >>>      Co-authored-by: olce >>>      Differential Revision:  https://reviews.freebsd.org/D51489 >> >> This syzbot report looks like it might be related to this change: >> https://syzkaller.appspot.com/bug?extid=4e68da43c26f357a2b7e >> >> No reproducer yet, but sometimes it takes a little while. > > I'll keep an eye out, thanks.  It strikes me that crsetgroups_internal > should probably grow a groups_check_max_len() call; this assertion > likely would have happened in a much more useful spot in the first place. > > Thanks, > > Kyle Evans Actually, what crcopysafe() doing here is not ideal. Note the comment from crextend(): 2812 /* 2813 * We allocate a power of 2 larger than 'nbytes', except when that 2814 * exceeds PAGE_SIZE, in which case we allocate the right multiple of 2815 * pages. We assume PAGE_SIZE is a power of 2 (the call to roundup2() 2816 * below) but do not need to for sizeof(gid_t). 2817 */ and then: 2830 cr->cr_agroups = nbytes / sizeof(gid_t); crcopysafe() then attempts to extend up to what cr_agroups can hold, but cr_agroups isn't guaranteed to be <= ngroups_max since we allocate a power of 2 larger; we'll want to cap that. I was considering whether we should do it at assignment time or in crcopysafe(). I'm kind of leaning toward the latter, and slapping a note on cr_agroups somewhere that it may exceed the limit. OTOH, we don't have a compelling need for cr_agroups to reflect the size of the allocation beyond the max number of groups today. Thanks, Kyle Evans Thanks, Kyle Evans