[v1] nss: fix __get_default_domain logic

  Florian provided the fix, I did the testcase.  This bug presents
itself by affecting various ldap-related queries behind nss_compat,
which makes it very difficult to reproduce in our testsuite, so this
test is limited to testing the broken function itself.  Since that
function isn't exposed directly, I chose to just link that object to
the test case and test it directly.

    nss: fix __get_default_domain logic
    
    Fix logic bug in __nss_get_default_domain that prevents
    proper initialization.
    
    Because this function is not exposed, the test case must link
    against the object directly.
    
    Co-authored-by: Florian Weimer <fweimer@redhat.com>
  

On Fri, Apr 3, 2026 at 1:50 AM DJ Delorie <dj@redhat.com> wrote:

>
> Florian provided the fix, I did the testcase.  This bug presents
> itself by affecting various ldap-related queries behind nss_compat,
> which makes it very difficult to reproduce in our testsuite, so this
> test is limited to testing the broken function itself.  Since that
> function isn't exposed directly, I chose to just link that object to
> the test case and test it directly.
>
>     nss: fix __get_default_domain logic
>
>     Fix logic bug in __nss_get_default_domain that prevents
>     proper initialization.
>
>     Because this function is not exposed, the test case must link
>     against the object directly.
>
>     Co-authored-by: Florian Weimer <fweimer@redhat.com>
>
> diff --git a/nss/Makefile b/nss/Makefile
> index 1c48bd0876..54389ced0e 100644
> --- a/nss/Makefile
> +++ b/nss/Makefile
> @@ -317,6 +317,7 @@ tests := \
>    test-netdb \
>    test-rpcent \
>    testgrp \
> +  tst-default-domain \
>    tst-fgetsgent_r \
>    tst-getaddrinfo \
>    tst-getaddrinfo2 \
> @@ -515,6 +516,8 @@ endif
>  $(objpfx)tst-nss-files-alias-leak.out: $(objpfx)libnss_files.so
>  $(objpfx)tst-nss-files-alias-truncated.out: $(objpfx)libnss_files.so
>
> +$(objpfx)tst-default-domain: $(objpfx)nisdomain.os
> +
>  tst-nss-gai-hv2-canonname-ENV = \
>                 MALLOC_TRACE=$(objpfx)tst-nss-gai-hv2-canonname.mtrace \
>                 LD_PRELOAD=$(common-objpfx)/malloc/libc_malloc_debug.so
> diff --git a/nss/nss_compat/nisdomain.c b/nss/nss_compat/nisdomain.c
> index e7a776b539..e98ad69f44 100644
> --- a/nss/nss_compat/nisdomain.c
> +++ b/nss/nss_compat/nisdomain.c
> @@ -36,7 +36,7 @@ __nss_get_default_domain (char **outdomain)
>
>    __libc_lock_lock (domainname_lock);
>
> -  if (domainname[0] != '\0')
> +  if (domainname[0] == '\0')
>

I looked back at the commit (64d1e08ea822bf47cb2796ad0f727136227f983c) that
introduced this function and I also confirm that this makes more sense this
way.
The `__nss_get_default_domain` function replaced uses of
`yp_get_default_domain` which has similar implementation and the
appropriate `domainname[0] == '\0'` check.

In principle the test looks good to me too.


>      {
>        if (getdomainname (domainname, MAXDOMAINNAMELEN) < 0)
>         result = errno;
> diff --git a/nss/tst-default-domain.c b/nss/tst-default-domain.c
> new file mode 100644
> index 0000000000..2fa9153d88
> --- /dev/null
> +++ b/nss/tst-default-domain.c
> @@ -0,0 +1,123 @@
> +/* Basic test of __nss_get_default_domain
> +   Copyright (C) 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 <unistd.h>
> +#include <string.h>
> +#include <dlfcn.h>
> +
> +#include "nss_compat/nisdomain.h"
> +
> +#include <support/test-driver.h>
> +#include <support/support.h>
> +#include <support/namespace.h>
> +#include <support/check.h>
> +
> +char unset_domain[] = "unset_domain";
> +char new_domain[] = "new_domain";
> +
> +/* This function checks the __nss_get_default_domain() function in
> +   nss_compat/nssdomain.c.  Because this is an internal function to
> +   libnss_compat.so, the Makefile will link that object to this test
> +   case directly.  */
> +
> +static int
> +do_test (void)
> +{
> +  char *domain_name;
> +  char buf[1024];
> +
> +  /* We need to be in a network namespace so we can change the domain
> +     name without interfering with the host system.  */
> +  support_become_root ();
> +  support_enter_network_namespace ();
> +  if (!support_in_uts_namespace ())
> +    return EXIT_UNSUPPORTED;
> +
> +  /* First pass: set an empty domain and make sure it's returned
> +     correctly.  This should not be cached.  */
> +
> +  /* Set the domain name to a known value.  */
> +  TEST_VERIFY (setdomainname ("", 0) == 0);
> +
> +  /* Make sure it got set.  */
> +  TEST_VERIFY (getdomainname (buf, sizeof(buf)) == 0);
> +  TEST_COMPARE_STRING (buf, "");
> +
> +  /* Set this to a known "unknown" value so we can detect if it's not
> +     changed.  */
> +  domain_name = unset_domain;
> +
> +  /* This is the function we're testing.  */
> +  TEST_VERIFY (__nss_get_default_domain (& domain_name) == 0);
> +
> +  /* Make sure the correct domain name is returned.  */
> +  TEST_VERIFY (domain_name != NULL);
> +  TEST_COMPARE_STRING (domain_name, "");
> +
> +  /* Second pass: set a non-empty domain and make sure it's returned
> +     correctly.  This works because the empty domain is not
> +     cached.  */
> +
> +  /* Set the domain name to a known value.  */
> +  TEST_VERIFY (setdomainname (new_domain, strlen (new_domain)) == 0);
> +
> +  /* Make sure it got set.  */
> +  TEST_VERIFY (getdomainname (buf, sizeof(buf)) == 0);
> +  TEST_COMPARE_STRING (buf, new_domain);
> +
> +  /* Set this to a known "unknown" value so we can detect if it's not
> +     changed.  */
> +  domain_name = unset_domain;
> +
> +  /* This is the function we're testing.  */
> +  TEST_VERIFY (__nss_get_default_domain (& domain_name) == 0);
> +
> +  /* Make sure the correct domain name is returned.  */
> +  TEST_VERIFY (domain_name != NULL);
> +  TEST_COMPARE_STRING (domain_name, new_domain);
> +
> +  /* The function caches the name, so check it twice.  */
> +  TEST_VERIFY (__nss_get_default_domain (& domain_name) == 0);
> +
> +  TEST_VERIFY (domain_name != NULL);
> +  TEST_COMPARE_STRING (domain_name, new_domain);
> +
> +  /* Third pass: set an empty domain again but expect the cached
> +     value.  */
> +
> +  /* Set the domain name to a known value.  */
> +  TEST_VERIFY (setdomainname ("", 0) == 0);
> +
> +  /* Make sure it got set.  */
> +  TEST_VERIFY (getdomainname (buf, sizeof(buf)) == 0);
> +  TEST_COMPARE_STRING (buf, "");
> +
> +  /* Set this to a known "unknown" value so we can detect if it's not
> +     changed.  */
> +  domain_name = unset_domain;
> +
> +  /* This is the function we're testing.  */
> +  TEST_VERIFY (__nss_get_default_domain (& domain_name) == 0);
> +
> +  TEST_VERIFY (domain_name != NULL);
> +  TEST_COMPARE_STRING (domain_name, new_domain);
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
>
>
Reviewed-by: Frédéric Bérat <fberat@redhat.com>
  

* Frederic Berat:

> I looked back at the commit (64d1e08ea822bf47cb2796ad0f727136227f983c)
> that introduced this function and I also confirm that this makes more
> sense this way.  The `__nss_get_default_domain` function replaced uses
> of `yp_get_default_domain` which has similar implementation and the
> appropriate `domainname[0] == '\0'` check.

DJ, please mention that commit in the commit message.

Thanks,
Florian
  

Frederic Berat <fberat@redhat.com> writes:
> Reviewed-by: Frédéric Bérat <fberat@redhat.com> 

Thanks!

> DJ, please mention that commit in the commit message.

Done.

Pushed.