[musl] NGROUPS_MAX definition conflicts with kernel one

[musl] NGROUPS_MAX definition conflicts with kernel one

[Érico Nogueira]

[-- Attachment #1.1: Type: text/plain, Size: 3239 bytes --]

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

[-- Attachment #2: 0001-use-NGROUPS_MAX-instead-of-hardcoded-value-where-pos.patch --]
[-- Type: text/plain, Size: 1351 bytes --]

From fb980d64744e076cfcb3e93d8f5e46b3ff9543bb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89rico=20Nogueira?= <ericonr@disroot.org>
Date: Sat, 3 Jul 2021 00:31:55 -0300
Subject: [PATCH] use NGROUPS_MAX instead of hardcoded value where possible

if someone using musl finds it necessary to patch musl to use 65536 as
NGROUPS_MAX, as defined by kernels >= 2.6.4, they can patch it in one
place, include/limits.h, instead of having to find all the places where
it's hardcoded
---
 include/sys/param.h | 2 +-
 src/conf/sysconf.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/sys/param.h b/include/sys/param.h
index ce6b8019..f4780588 100644
--- a/include/sys/param.h
+++ b/include/sys/param.h
@@ -6,7 +6,7 @@
 #define MAXNAMLEN 255
 #define MAXPATHLEN 4096
 #define NBBY 8
-#define NGROUPS 32
+#define NGROUPS NGROUPS_MAX
 #define CANBSIZ 255
 #define NOFILE 256
 #define NCARGS 131072
diff --git a/src/conf/sysconf.c b/src/conf/sysconf.c
index 3baaed32..1a5d3860 100644
--- a/src/conf/sysconf.c
+++ b/src/conf/sysconf.c
@@ -28,7 +28,7 @@ long sysconf(int name)
 		[_SC_ARG_MAX] = JT_ARG_MAX,
 		[_SC_CHILD_MAX] = RLIM(NPROC),
 		[_SC_CLK_TCK] = 100,
-		[_SC_NGROUPS_MAX] = 32,
+		[_SC_NGROUPS_MAX] = NGROUPS_MAX,
 		[_SC_OPEN_MAX] = RLIM(NOFILE),
 		[_SC_STREAM_MAX] = -1,
 		[_SC_TZNAME_MAX] = TZNAME_MAX,
-- 
2.32.0