Re: [musl] [PATCH] Increase NGROUPS_MAX from 32 to 1024

[Markus Wichmann <nullplan@gmx.net>]

Am Wed, Apr 10, 2024 at 09:07:38PM -0400 schrieb Rich Felker:
> As for the macro, I think it's actually valid to define it as 65536,
> since even if we're running on an old kernel, there is no conformance
> distinction. I'm not sure if this is the nicest thing to do though.
> Apps may want to start with a buffer of size NGROUPS_MAX and increase
> it up to the sysconf value rather than allocating a giant amount of
> memory that will never in practice be used. This should be further
> discussed, particularly what impact it might have on application
> behavior and memory usage.
>

I had a look at Debian Codesearch for NGROUPS_MAX, to see what
applications are actually doing with the macro. And I found no instance
of anyone using it as an array size. That's what had me most worried,
because obviously increasing an array size by a few orders of magnitude
can cause a stack overrun.

A lot of applications use it or the sysconf() equivalent as upper bounds
for allocations, or even for setgroups(). So they should be fine with an
increase.

> It does have a TOCTOU race if the groups db changes
> between the first call and the retry.

Well, a lot of the login process has races if the user db changes during
the process. I think that is reasonable. As long as the race is resolved
in a safe way (as in, setting either the complete old list or the
complete new list), I think this is sensible. Although, now that I think
about it, the worst that could happen is someone being added to a group
and getting a truncated group list. And then they just have to re-login.
Which they already have to do anyway after being added to a group; they
were just too fast.

Ciao,
Markus