Date: Thu, 31 Jul 2025 14:00:37 -0500 From: Kyle Evans <kevans@FreeBSD.org> To: Mark Johnston <markj@freebsd.org>, olce@freebsd.org Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: be1f7435ef21 - main - kern: start tracking cr_gid outside of cr_groups[] Message-ID: <4184e804-b731-4b4e-9399-d27f776f575d@FreeBSD.org> In-Reply-To: <aIu5-DG8loJrDi26@nuc> References: <202507310444.56V4icA3054832@gitrepo.freebsd.org> <aIu5-DG8loJrDi26@nuc>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <kevans@FreeBSD.org> >> AuthorDate: 2025-07-31 04:44:11 +0000 >> Commit: Kyle Evans <kevans@FreeBSD.org> >> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4184e804-b731-4b4e-9399-d27f776f575d>