[v2,21/23] nss: Convert group database to new NSS framework

  Group merging is reimplemented and includes deduplication, which is
why nss/tst-nss-test4 needs to be adjusted.
---
 include/set-freeres.h     |   2 -
 nss/Makefile              |   1 +
 nss/getgrgid.c            |  14 ++-
 nss/getgrgid_r.c          |  36 +++++---
 nss/getgrnam.c            |  14 ++-
 nss/getgrnam_r.c          |  40 ++++++---
 nss/nss_generic_storage.h |   1 +
 nss/nss_getXinfo.c        |  10 +++
 nss/nss_getgrXinfo.c      | 176 ++++++++++++++++++++++++++++++++++++++
 nss/tst-nss-test4.c       |  12 +--
 10 files changed, 260 insertions(+), 46 deletions(-)
 create mode 100644 nss/nss_getgrXinfo.c
  

On 3/20/26 4:43 PM, Florian Weimer wrote:
> Group merging is reimplemented and includes deduplication, which is
> why nss/tst-nss-test4 needs to be adjusted.

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>   include/set-freeres.h     |   2 -
>   nss/Makefile              |   1 +
>   nss/getgrgid.c            |  14 ++-
>   nss/getgrgid_r.c          |  36 +++++---
>   nss/getgrnam.c            |  14 ++-
>   nss/getgrnam_r.c          |  40 ++++++---
>   nss/nss_generic_storage.h |   1 +
>   nss/nss_getXinfo.c        |  10 +++
>   nss/nss_getgrXinfo.c      | 176 ++++++++++++++++++++++++++++++++++++++
>   nss/tst-nss-test4.c       |  12 +--
>   10 files changed, 260 insertions(+), 46 deletions(-)
>   create mode 100644 nss/nss_getgrXinfo.c
> 
> diff --git a/include/set-freeres.h b/include/set-freeres.h
> index c9d2c8ae55..7806266077 100644
> --- a/include/set-freeres.h
> +++ b/include/set-freeres.h
> @@ -102,8 +102,6 @@ extern printf_arginfo_size_function ** __libc_reg_printf_freemem_ptr
>   extern printf_va_arg_function ** __libc_reg_type_freemem_ptr
>       attribute_hidden;
>   /* From nss/getXXbyYY.c  */
> -extern char * __libc_getgrgid_freemem_ptr attribute_hidden;
> -extern char * __libc_getgrnam_freemem_ptr attribute_hidden;
>   extern char * __libc_getspnam_freemem_ptr attribute_hidden;
>   extern char * __libc_getaliasbyname_freemem_ptr attribute_hidden;
>   extern char * __libc_gethostbyaddr_freemem_ptr attribute_hidden;
> diff --git a/nss/Makefile b/nss/Makefile
> index a13f6fa10c..504f573478 100644
> --- a/nss/Makefile
> +++ b/nss/Makefile
> @@ -55,6 +55,7 @@ routines = \
>     nss_getX \
>     nss_getX_r \
>     nss_getXinfo \
> +  nss_getgrXinfo \
>     nss_hash \
>     nss_module \
>     nss_parse_line_result \
> diff --git a/nss/getgrgid.c b/nss/getgrgid.c
> index 3053e22517..9a15e3c505 100644
> --- a/nss/getgrgid.c
> +++ b/nss/getgrgid.c
> @@ -17,12 +17,10 @@
>   
>   #include <grp.h>
>   
> +#include <nss_generic.h>
>   
> -#define LOOKUP_TYPE	struct group
> -#define FUNCTION_NAME	getgrgid
> -#define DATABASE_NAME	group
> -#define ADD_PARAMS	gid_t gid
> -#define ADD_VARIABLES	gid
> -#define BUFLEN		NSS_BUFLEN_GROUP
> -
> -#include "../nss/getXXbyYY.c"
> +struct group *
> +getgrgid (gid_t gid)
> +{
> +  return __nss_getX (nss_lookup_getgrgid, &gid);
> +}
> diff --git a/nss/getgrgid_r.c b/nss/getgrgid_r.c
> index af15accd10..7d4e693ccc 100644
> --- a/nss/getgrgid_r.c
> +++ b/nss/getgrgid_r.c
> @@ -17,15 +17,31 @@
>   
>   #include <grp.h>
>   
> -#include <grp-merge.h>
> +#include <nss_generic.h>
> +#include <shlib-compat.h>
>   
> -#define LOOKUP_TYPE	struct group
> -#define FUNCTION_NAME	getgrgid
> -#define DATABASE_NAME	group
> -#define ADD_PARAMS	gid_t gid
> -#define ADD_VARIABLES	gid
> -#define BUFLEN		NSS_BUFLEN_GROUP
> -#define DEEPCOPY_FN	__copy_grp
> -#define MERGE_FN	__merge_grp
> +int
> +___getgrgid_r (gid_t gid, struct group *grp,
> +               char *buffer, size_t length, struct group **result)
> +{
> +  void *ptr = grp;
> +  int ret = __nss_getX_r (nss_lookup_getgrgid, &gid, &ptr, buffer, length);
> +  *result = ptr;
> +  return ret;
> +}
> +strong_alias (___getgrgid_r, __getgrgid_r)
> +versioned_symbol (libc, ___getgrgid_r, getgrgid_r, GLIBC_2_1_2);
>   
> -#include <nss/getXXbyYY_r.c>
> +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1_2)
> +int
> +attribute_compat_text_section
> +__old_getgrgid_r (gid_t gid, struct group *grp,
> +                   char *buffer, size_t length, struct group **result)
> +{
> +  int ret = __getgrgid_r (gid, grp, buffer, length, result);
> +  if (ret != 0 || result == NULL)
> +    ret = -1;
> +  return ret;
> +}
> +compat_symbol (libc, __old_getgrgid_r, getgrgid_r, GLIBC_2_0);
> +#endif
> diff --git a/nss/getgrnam.c b/nss/getgrnam.c
> index f10e79d50d..19e990a7cd 100644
> --- a/nss/getgrnam.c
> +++ b/nss/getgrnam.c
> @@ -17,12 +17,10 @@
>   
>   #include <grp.h>
>   
> +#include <nss_generic.h>
>   
> -#define LOOKUP_TYPE	struct group
> -#define FUNCTION_NAME	getgrnam
> -#define DATABASE_NAME	group
> -#define ADD_PARAMS	const char *name
> -#define ADD_VARIABLES	name
> -#define BUFLEN		NSS_BUFLEN_GROUP
> -
> -#include "../nss/getXXbyYY.c"
> +struct group *
> +getgrnam (const char *name)
> +{
> +  return __nss_getX (nss_lookup_getgrnam, name);
> +}
> diff --git a/nss/getgrnam_r.c b/nss/getgrnam_r.c
> index 44c7b87a34..358046c5db 100644
> --- a/nss/getgrnam_r.c
> +++ b/nss/getgrnam_r.c
> @@ -17,15 +17,31 @@
>   
>   #include <grp.h>
>   
> -#include <grp-merge.h>
> -
> -#define LOOKUP_TYPE	struct group
> -#define FUNCTION_NAME	getgrnam
> -#define DATABASE_NAME	group
> -#define ADD_PARAMS	const char *name
> -#define ADD_VARIABLES	name
> -
> -#define DEEPCOPY_FN	__copy_grp
> -#define MERGE_FN	__merge_grp
> -
> -#include <nss/getXXbyYY_r.c>
> +#include <nss_generic.h>
> +#include <shlib-compat.h>
> +
> +int
> +___getgrnam_r (const char *name, struct group *grp,
> +	      char *buffer, size_t length, struct group **result)
> +{
> +  void *ptr = grp;
> +  int ret = __nss_getX_r (nss_lookup_getgrnam, name, &ptr, buffer, length);
> +  *result = ptr;
> +  return ret;
> +}
> +strong_alias (___getgrnam_r, __getgrnam_r)
> +versioned_symbol (libc, ___getgrnam_r, getgrnam_r, GLIBC_2_1_2);
> +
> +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1_2)
> +int
> +attribute_compat_text_section
> +__old_getgrnam_r (const char *name, struct group *grp,
> +		   char *buffer, size_t length, struct group **result)
> +{
> +  int ret = __getgrnam_r (name, grp, buffer, length, result);
> +  if (ret != 0 || result == NULL)
> +    ret = -1;
> +  return ret;
> +}
> +compat_symbol (libc, __old_getgrnam_r, getgrnam_r, GLIBC_2_0);
> +#endif
> diff --git a/nss/nss_generic_storage.h b/nss/nss_generic_storage.h
> index 334f51be70..2d46136c9a 100644
> --- a/nss/nss_generic_storage.h
> +++ b/nss/nss_generic_storage.h
> @@ -24,6 +24,7 @@
>   
>   union nss_generic_storage
>   {
> +  struct group grp;
>     struct passwd pwd;
>   };
>   
> diff --git a/nss/nss_getXinfo.c b/nss/nss_getXinfo.c
> index 0b2588c9fd..96836dc4c2 100644
> --- a/nss/nss_getXinfo.c
> +++ b/nss/nss_getXinfo.c
> @@ -26,6 +26,16 @@ __nss_getXinfo (enum nss_lookup_type lt, nss_lookup_key key, void **result)
>       return 0;
>   #endif
>   
> +  switch (lt)
> +    {
> +    case nss_lookup_getgrgid:
> +    case nss_lookup_getgrnam:
> +      /* Group lookups are handled separately to implement merging.  */
> +      return __nss_getgrXinfo (lt, key, result);
> +    default:
> +      break;
> +    }
> +
>     nss_action_list nip = NULL;
>     void *fct;
>     int no_more = __nss_generic_next (lt, &nip, &fct, 0, 0);
> diff --git a/nss/nss_getgrXinfo.c b/nss/nss_getgrXinfo.c
> new file mode 100644
> index 0000000000..795d12d63e
> --- /dev/null
> +++ b/nss/nss_getgrXinfo.c
> @@ -0,0 +1,176 @@
> +/* Implementation of the getgrXinfo family of functions.
> +   Copyright (C) 1996-2026 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <assert.h>
> +#include <errno.h>
> +#include <grp.h>
> +#include <nss_generic.h>
> +#include <nss.h>
> +#include <nss_group_members.h>
> +#include <nsswitch.h>
> +#include <stdbool.h>
> +#include <stdlib.h>
> +#include <stringtable.h>
> +
> +static inline bool
> +__nss_status_has_result (enum nss_status status)
> +{
> +  return status == NSS_STATUS_SUCCESS || status == NSS_STATUS_NOTFOUND;
> +}
> +
> +static int
> +__nss_status_to_posix_result (enum nss_status status)
> +{
> +  /* Treat NSS_STATUS_UNAVAIL like NSS_STATUS_NOTFOUND.  Some NSS
> +     modules return NSS_STATUS_UNAVAIL if they have disabled
> +     themselves.  */
> +  if (status == NSS_STATUS_SUCCESS || status == NSS_STATUS_NOTFOUND
> +      || status == NSS_STATUS_UNAVAIL)
> +    return 0;
> +  else
> +    {
> +      /* Paranoia, to avoid endless loops.   */
> +      if (errno == ERANGE)
> +        __set_errno (EINVAL);
> +      return -1;
> +    }
> +}
> +
> +int
> +__nss_getgrXinfo (enum nss_lookup_type lt,
> +                  nss_lookup_key key,
> +                  void **result)
> +{
> +  /* Default if no service modules are available.  */
> +  enum nss_status status = NSS_STATUS_UNAVAIL;
> +  *result = NULL;
> +
> +  /* For merging.  If table is not empty, then it supersedes the group
> +     members in first_result.  */
> +  struct group *first_result = NULL;
> +  struct nss_group_members table = { };
> +  __nss_group_members_init (&table);
> +
> +  /* With better error reporting (especially from dlopen), we could
> +     check for NSS initialization errors here and report them.  */
> +  nss_action_list nip;
> +  void *fct = __nss_generic_lookup (lt, &nip);
> +
> +  int no_more = fct == NULL;
> +  bool do_merge = false;
> +  while (no_more == 0)
> +    {
> +      void *ptr;
> +      status = __nss_generic_get (lt, key, fct, &ptr);
> +      struct group *grp = ptr;
> +
> +      if (status == NSS_STATUS_SUCCESS)
> +        {
> +          assert (grp != NULL);
> +          /* We have result data.  It may need to be merged.  We only
> +             support merging members of groups with identical names
> +             and GID values.  If we hit this, the grp result overrides
> +             the first result.  */
> +          if (do_merge && first_result != NULL
> +              && grp->gr_gid == first_result->gr_gid
> +              && strcmp (grp->gr_name, first_result->gr_name) == 0)
> +            {
> +              /* Perform the merge.  If no members have been merged yet,
> +                 process first_result as well.  */
> +              if ((table.T.count == 0
> +                   && !__nss_group_members_add (&table, first_result))
> +                  || !__nss_group_members_add (&table, grp))
> +                {
> +                  __nss_group_members_free (&table);
> +                  free (grp);
> +                  free (first_result);
> +                  return -1;
> +                }
> +              free (grp);
> +            }
> +          else
> +            {
> +              /* No merge or different data.  New result replaces
> +                 previous result.  */
> +              free (first_result);
> +              first_result = grp;
> +              __nss_group_members_free (&table);
> +            }
> +        }
> +      else if (status == NSS_STATUS_TRYAGAIN)
> +        {
> +          free (first_result);
> +          __nss_group_members_free (&table);
> +          return -1;
> +        }
> +      else if (first_result != NULL)
> +        {
> +          /* If the result wasn't SUCCESS, use the stored data in
> +             first_result/table and set the status back to
> +             NSS_STATUS_SUCCESS to match the previous pass through
> +             the loop.
> +
> +             * If the next action is CONTINUE, it will overwrite the value
> +               currently in the buffer and return the new value.
> +             * If the next action is RETURN, we'll return the previously-
> +               acquired values.
> +             * If the next action is MERGE, then it will be added to the
> +               buffer saved from the previous source.  */
> +          status = NSS_STATUS_SUCCESS;
> +        }
> +
> +      do_merge = (nss_next_action (nip, status) == NSS_ACTION_MERGE
> +                  && status == NSS_STATUS_SUCCESS);
> +      no_more = __nss_generic_next (lt, &nip, &fct, status, 0);
> +    }
> +
> +
> +  if (table.T.count > 0)
> +    {
> +      /* We have something to merge.  */
> +      struct group *merged_allocated;
> +      {
> +        struct group merged = *first_result;
> +        merged.gr_mem = __nss_group_members (&table);
> +        if (merged.gr_mem == NULL)
> +          {
> +            __nss_group_members_free (&table);
> +            free (first_result);
> +            return -1;
> +          }
> +
> +        /* Make a consolidated copy of the entire group information.  */
> +        merged_allocated = __nss_generic_dup (lt, &merged);

OK.

> +
> +        free (merged.gr_mem);
> +      }
> +
> +      __nss_group_members_free (&table);
> +      free (first_result);
> +      first_result = merged_allocated;
> +
> +      if (first_result == NULL)
> +        return -1;
> +    }
> +  else
> +    /* No merging necessary.  We can use first_result.  */
> +    __nss_group_members_free (&table);
> +
> +  *result = first_result;
> +  return __nss_status_to_posix_result (status);
> +}
> diff --git a/nss/tst-nss-test4.c b/nss/tst-nss-test4.c
> index 72e33f3d6d..332ec2e217 100644
> --- a/nss/tst-nss-test4.c
> +++ b/nss/tst-nss-test4.c
> @@ -100,9 +100,7 @@ do_test (void)
>     /* At least 3 service modules are needed to reproduce BZ#33361. */
>     __nss_configure_lookup ("group", "test1 [SUCCESS=merge] test2 files");
>   
> -  /* Test increasing sizes of group_2 to see if we fail, starting with
> -     member_cnt == 1 to ensure we always check for no de-duplication
> -     e.g. { "foo", NULL } */
> +  /* Test increasing sizes of group_2 to see if we fail.   */
>     for (member_cnt = 1; member_cnt < array_length (group_2); member_cnt++)
>       {
>         verbose_printf ("Outer loop - member_cnt is %d\n", member_cnt);
> @@ -127,15 +125,17 @@ do_test (void)
>   	  verbose_printf ("MERGED LIST of [%d] is %s\n", i, merge_1[i]);
>   	}
>   
> -      /* Add group_2 to the merge list */
> -      int group2_index = 0;
> +      /* Add group_2 to the merge list.  Skip the duplicate "foo"
> +	 group at the start.  */
> +      int group2_index = 1;
>         for (i = array_length (group_1) - 1;
>   	   i < array_length (group_1) - 1 + member_cnt; i++)
>   	{
>   	  merge_1[i] = xasprintf ("%s", group_2[group2_index++]);
>   	  verbose_printf ("MERGED LIST of [%d] is %s\n", i, merge_1[i]);
>   	}
> -      merge_1[array_length(group_1) - 1 + member_cnt]= NULL;
> +      /* Skipping the "foo" group reduced the member count by 1.  */
> +      merge_1[array_length(group_1) - 1 + member_cnt - 1]= NULL;

OK.

>   
>         align_mask = __alignof__ (struct group) - 1;
>         align_mem_mask = __alignof__ (char *) - 1;