[<prev] [next>] [day] [month] [year] [list]
Date: Fri, 02 Jul 2021 23:58:26 -0300
From: Érico Nogueira <ericonr@...root.org>
To: <musl@...ts.openwall.com>
Subject: NGROUPS_MAX definition conflicts with kernel one
Hello all.
I brought this up on IRC and we thought it best to bring it up on the ML
as well.
musl's NGROUPS_MAX is defined to 32, which is the kernel's definition
for the value for linux < 2.6.4. Meanwhile, linux >= 2.6.4 has 65536.
Linux 2.6.4 also introduced /proc/sys/kernel/ngroups_max, to allow for
this value to be changed in the kernel without libc having to keep
changing their definitions.
glibc's headers define NGROUPS_MAX to 65536, the new limit, so there is
already some discrepancy between the two libraries. Fortunately, the man
pages for getgroups(2) suggests using sysconf(_SC_NGROUPS_MAX) instead
of the NGROUPS_MAX value directly, since the sysconf function can,
theoretically, query the information from the currently running kernel.
glibc's does just that: it tries to read from /proc, and if that fails,
it will return NGROUPS_MAX as defined in <limits.h>. Unfortunately,
musl's implementation directly returns NGROUPS_MAX from its own
definition of it, which means it returns 32.
There are few scenarios where I can see this being relevant, to be
honest; hardly anyone is going to have more than 32 supplementary
groups. However, in the case they do, functions like getgroups(2) will
fail even if the caller passes a buffer with size
sysconf(_SC_NGROUPS_MAX). An application could be forgiven for not
trying to increase the buffer size past it in the case that
sysconf(_SC_NGROUPS_MAX) exists (doesn't return -1).
There are a few ways forward from here: NGROUPS_MAX can be updated to
the latest kernel maximum and/or sysconf(_SC_NGROUPS_MAX) can be fixed
to try and read values from /proc; or we do nothing.
Not doing nothing would also require rolling out further fixes:
- musl's initgroups(3) uses a static array with 32 members for the
groups, which isn't enough if we want to support cases with more
groups; that would require implementing dynamic allocation, which,
with naive logic, would allocate 256KB (65536 entries * sizeof(gid_t))
- any applications/libraries mainly used in the musl ecosystem might be
relying on NPROCS_MAX being a small value that can be used, for
example, as the size of a stack-allocated array; with a bigger size,
this would no longer be true (see [1] as an example, though I am
already looking into fixing that particular case)
[1] https://github.com/pikhq/musl-nscd/blob/9ab730cfb5fe3dc224ded1033e1b36ea60c60be6/src/socket_handle.c#L232
In conclusion, as mentioned on IRC, "it's a place where the old limit
was a feature", since it avoids big allocations when very very few use
cases need them. IMO, if we want to be more correct while keeping the
advantage of a low value, sysconf(_SC_NPROCS_MAX) could read /proc, but
NPROCS_MAX could be kept as 32. But I don't know how the initgroups(3)
case would deal with it. If we decide to do nothing, such a limitation
(even if derived from a design choice) should at least end up documented
clearly.
A suggestion on IRC was also that users requiring more than 32
supplementary groups could patch musl to work on their environment. For
that purpose, I have attached a patch to this email to make it simpler
for such patching to occur.
Cheers,
Érico
View attachment "0001-use-NGROUPS_MAX-instead-of-hardcoded-value-where-pos.patch" of type "text/plain" (1351 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
