[1/1] stdlib/tst-secure-getenv: handle >64 groups

  This test would fail unnecessarily if the user running it had more than
64 groups since getgroups returns EINVAL if the size provided is less
than the number of supplementary group IDs. Instead dynamically
determine the number of supplementary groups the user has.
---
 stdlib/tst-secure-getenv.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)
  

* Mike Gerow:

> This test would fail unnecessarily if the user running it had more than
> 64 groups since getgroups returns EINVAL if the size provided is less
> than the number of supplementary group IDs. Instead dynamically
> determine the number of supplementary groups the user has.

Yes, that's indeed a test bug.

> ---
>  stdlib/tst-secure-getenv.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/stdlib/tst-secure-getenv.c b/stdlib/tst-secure-getenv.c
> index 74580b889a..178dfa0439 100644
> --- a/stdlib/tst-secure-getenv.c
> +++ b/stdlib/tst-secure-getenv.c
> @@ -41,8 +41,14 @@ static char MAGIC_ARGUMENT[] = "run-actual-test";
>  static gid_t
>  choose_gid (void)
>  {
> -  const int count = 64;
> -  gid_t groups[count];
> +  int count = getgroups (0, NULL);
> +  if (count < 0)
> +    {
> +      printf ("getgroups: %m\n");
> +      exit (1);
> +    }
> +  gid_t *groups;
> +  groups = malloc (count * sizeof (*groups));

This should be xcalloc (or perhaps xreallocarray, if we had support for
that).

>    int ret = getgroups (count, groups);

Technically, you would also need to add a loop to retry because the
probing for size could have returned an outdated value.

Thanks,
Florian
  

On Fri, Apr 05, 2019 at 11:56:28AM +0200, Florian Weimer wrote:
> * Mike Gerow:
> > ---
> >  stdlib/tst-secure-getenv.c | 19 +++++++++++++++----
> >  1 file changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/stdlib/tst-secure-getenv.c b/stdlib/tst-secure-getenv.c
> > index 74580b889a..178dfa0439 100644
> > --- a/stdlib/tst-secure-getenv.c
> > +++ b/stdlib/tst-secure-getenv.c
> > @@ -41,8 +41,14 @@ static char MAGIC_ARGUMENT[] = "run-actual-test";
> >  static gid_t
> >  choose_gid (void)
> >  {
> > -  const int count = 64;
> > -  gid_t groups[count];
> > +  int count = getgroups (0, NULL);
> > +  if (count < 0)
> > +    {
> > +      printf ("getgroups: %m\n");
> > +      exit (1);
> > +    }
> > +  gid_t *groups;
> > +  groups = malloc (count * sizeof (*groups));
> 
> This should be xcalloc (or perhaps xreallocarray, if we had support for
> that).
Sounds good, I'll change it to xaclloc.
> 
> >    int ret = getgroups (count, groups);
> 
> Technically, you would also need to add a loop to retry because the
> probing for size could have returned an outdated value.
Considering (at least on linux) the groups are a property of the process
it seems pretty unlikely that this is something we need to worry about,
at least in the context of a test. If you insist, though, I'm happy to
handle this case.
  

* Mike Gerow:

>> Technically, you would also need to add a loop to retry because the
>> probing for size could have returned an outdated value.

> Considering (at least on linux) the groups are a property of the process
> it seems pretty unlikely that this is something we need to worry about,
> at least in the context of a test. If you insist, though, I'm happy to
> handle this case.

You are right.  For a single-threaded test, this is completely
unnecessary.  I confused it with the NSS APIs.

Thanks,
Florian