Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZhdR6Cl386bK454k@voyager>
Date: Thu, 11 Apr 2024 04:58:48 +0200
From: Markus Wichmann <nullplan@....net>
To: musl@...ts.openwall.com
Cc: Kate Deplaix <kit-ty-kate@...look.com>
Subject: Re: [PATCH] Increase NGROUPS_MAX from 32 to 1024

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

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.