[glibc] resolv: reset _u._ext.nscount in __res_iclose [BZ #34154]

Adhemerval Zanella via Glibc-cvs <[email protected]> Wed, 3 Jun 2026 14:11:42 +0000 (GMT)
Newsgroups gmane.comp.lib.glibc.cvs
Message-ID <[email protected]>
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=13dc1040af5a6dd3d582a96fe3931a2f98179a52

commit 13dc1040af5a6dd3d582a96fe3931a2f98179a52
Author: Adam Yi <[email protected]>
Date:   Mon May 18 11:21:47 2026 +0800

    resolv: reset _u._ext.nscount in __res_iclose [BZ #34154]
    
    __res_iclose, when called with FREE_ADDR=true, frees and NULLs every
    statp->_u._ext.nsaddrs[ns] but does not reset statp->_u._ext.nscount.
    This breaks the invariant relied on by __res_context_send's cache
    validation loop in resolv/res_send.c:293-312: when _u._ext.nscount
    is non-zero, every _u._ext.nsaddrs[ns] corresponding to
    statp->nsaddr_list[ns].sin_family != 0 is expected to be non-NULL.
    
    res_init unconditionally calls __res_iclose(&_res, true) before
    __res_vinit, so any __res_vinit failure (for example,
    fopen("/etc/resolv.conf") returning EMFILE under file-descriptor
    exhaustion, or any allocation failure in __resolv_conf_load,
    __resolv_conf_allocate, or __resolv_conf_attach before
    update_from_conf runs) leaves _u._ext.nscount non-zero with all
    _u._ext.nsaddrs[] NULL.  The next name lookup walks the validation
    loop and dereferences NULL in sock_eq.  A DNS lookup may fail after
    a failed res_init, but it should not segfault.
    
    Reset _u._ext.nscount = 0 alongside the existing __resolv_conf_detach
    call.  The next __res_context_send call then re-enters its init
    block (res_send.c:316-335) and repopulates _u._ext.nsaddrs[] from
    statp->nsaddr_list[], which __res_iclose leaves untouched.  This
    also lets support/resolv_test.c drop its now-redundant manual reset
    after __res_iclose(&_res, true).
    
    Add a regression test that drops RLIMIT_NOFILE so fopen of
    /etc/resolv.conf fails with EMFILE inside __res_vinit, then verifies
    the subsequent gethostbyname does not crash.
    
    Signed-off-by: Adam Yi <[email protected]>
    Reviewed-by: Adhemerval Zanella  <[email protected]>

Diff:
---
 resolv/Makefile                      |   3 +
 resolv/res-close.c                   |   9 ++-
 resolv/tst-resolv-res_init-failure.c | 109 +++++++++++++++++++++++++++++++++++
 support/resolv_test.c                |   1 -
 4 files changed, 119 insertions(+), 3 deletions(-)

diff --git a/resolv/Makefile b/resolv/Makefile
index 971608eff5..68b3a4dbf3 100644
--- a/resolv/Makefile
+++ b/resolv/Makefile
@@ -122,6 +122,7 @@ tests += \
   tst-resolv-noaaaa \
   tst-resolv-noaaaa-vc \
   tst-resolv-nondecimal \
+  tst-resolv-res_init-failure \
   tst-resolv-res_init-multi \
   tst-resolv-search \
   tst-resolv-semi-failure \
@@ -307,6 +308,8 @@ $(objpfx)tst-resolv-dns-section: $(objpfx)libresolv.so \
 $(objpfx)tst-resolv-edns: $(objpfx)libresolv.so $(shared-thread-library)
 $(objpfx)tst-resolv-network: $(objpfx)libresolv.so $(shared-thread-library)
 $(objpfx)tst-resolv-res_init: $(objpfx)libresolv.so
+$(objpfx)tst-resolv-res_init-failure: $(objpfx)libresolv.so \
+  $(shared-thread-library)
 $(objpfx)tst-resolv-res_init-multi: $(objpfx)libresolv.so \
   $(shared-thread-library)
 $(objpfx)tst-resolv-res_init-thread: $(objpfx)libresolv.so \
diff --git a/resolv/res-close.c b/resolv/res-close.c
index 374f09d2de..a6aaa22aaf 100644
--- a/resolv/res-close.c
+++ b/resolv/res-close.c
@@ -89,7 +89,9 @@
 #include <stdlib.h>
 
 /* Close all open sockets.  If FREE_ADDR is true, deallocate any
-   separately allocated name server addresses.  */
+   separately allocated name server addresses, detach the extended
+   resolver configuration, and reset the cached extended-state count
+   so subsequent name lookups reinitialize the extended state.  */
 void
 __res_iclose (res_state statp, bool free_addr)
 {
@@ -114,7 +116,10 @@ __res_iclose (res_state statp, bool free_addr)
           }
       }
   if (free_addr)
-    __resolv_conf_detach (statp);
+    {
+      __resolv_conf_detach (statp);
+      statp->_u._ext.nscount = 0;
+    }
 }
 libc_hidden_def (__res_iclose)
 
diff --git a/resolv/tst-resolv-res_init-failure.c b/resolv/tst-resolv-res_init-failure.c
new file mode 100644
index 0000000000..d20348acf2
--- /dev/null
+++ b/resolv/tst-resolv-res_init-failure.c
@@ -0,0 +1,109 @@
+/* Test that name lookups do not crash after a failing res_init call.
+   Copyright The GNU Toolchain Authors.
+   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/>.  */
+
+/* When res_init is called on a thread whose resolver state has
+   already been initialized, it unconditionally invokes
+   __res_iclose(&_res, true) before __res_vinit.  __res_iclose used
+   to NULL every _u._ext.nsaddrs[ns] without resetting
+   _u._ext.nscount, breaking the invariant that
+   __res_context_send's validation loop relies on.  If __res_vinit
+   then failed (for example, because fopen("/etc/resolv.conf")
+   returned EMFILE), the resolver state was left with
+   _u._ext.nscount > 0 and _u._ext.nsaddrs[*] == NULL.  The next
+   name lookup walked the validation loop and dereferenced NULL in
+   sock_eq.
+
+   This test reproduces the crash:
+     1. perform a name lookup so __res_context_send populates
+        _u._ext.nsaddrs[] and sets _u._ext.nscount;
+     2. drop RLIMIT_NOFILE so fopen("/etc/resolv.conf") fails with
+        EMFILE on the next call;
+     3. call res_init, which must report failure;
+     4. perform another name lookup, which must not crash.  */
+
+#include <arpa/nameser.h>
+#include <netdb.h>
+#include <resolv.h>
+#include <sys/resource.h>
+
+#include <support/check.h>
+#include <support/resolv_test.h>
+#include <support/xunistd.h>
+
+static void
+response (const struct resolv_response_context *ctx,
+	  struct resolv_response_builder *b,
+	  const char *qname, uint16_t qclass, uint16_t qtype)
+{
+  resolv_response_init (b, (struct resolv_response_flags) { 0 });
+  resolv_response_add_question (b, qname, qclass, qtype);
+  resolv_response_section (b, ns_s_an);
+  resolv_response_open_record (b, qname, qclass, qtype, 0);
+  if (qtype == T_A)
+    {
+      const char ipv4[4] = { 192, 0, 2, 1 };
+      resolv_response_add_data (b, ipv4, sizeof (ipv4));
+    }
+  resolv_response_close_record (b);
+}
+
+static int
+do_test (void)
+{
+  struct resolv_test *aux = resolv_test_start
+    ((struct resolv_redirect_config) { .response_callback = response });
+
+  /* Initial lookup.  This drives __res_context_send through its init
+     block, which allocates _u._ext.nsaddrs[] and sets
+     _u._ext.nscount to _res.nscount.  */
+  struct hostent *h = gethostbyname ("primer.example");
+  TEST_VERIFY_EXIT (h != NULL);
+
+  /* Drop RLIMIT_NOFILE so that the next fopen of /etc/resolv.conf
+     inside __resolv_conf_load fails with EMFILE.  Already-open
+     descriptors (including those held by the resolv test harness)
+     remain valid; only new descriptor allocations are blocked.  */
+  struct rlimit rl;
+  TEST_COMPARE (getrlimit (RLIMIT_NOFILE, &rl), 0);
+  rlim_t saved_cur = rl.rlim_cur;
+  rl.rlim_cur = 3;
+  TEST_COMPARE (setrlimit (RLIMIT_NOFILE, &rl), 0);
+
+  /* res_init must report failure: __res_iclose has already cleared
+     the extended state, __res_vinit then fails to reload
+     /etc/resolv.conf.  */
+  TEST_COMPARE (res_init (), -1);
+
+  /* The repro: without the fix in __res_iclose, this call would
+     enter __res_context_send's validation loop with stale
+     _u._ext.nscount and NULL _u._ext.nsaddrs[0], dereference NULL
+     in sock_eq, and SIGSEGV.  We do not care whether the lookup
+     succeeds (it will likely fail because socket() also returns
+     EMFILE), only that it returns without crashing.  */
+  (void) gethostbyname ("after.example");
+
+  /* Restore the descriptor limit before resolv_test_end so its
+     cleanup work is not constrained.  */
+  rl.rlim_cur = saved_cur;
+  TEST_COMPARE (setrlimit (RLIMIT_NOFILE, &rl), 0);
+
+  resolv_test_end (aux);
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/support/resolv_test.c b/support/resolv_test.c
index dad755ac30..ec406b281a 100644
--- a/support/resolv_test.c
+++ b/support/resolv_test.c
@@ -1183,7 +1183,6 @@ resolv_test_start (struct resolv_redirect_config config)
   /* Disable IPv6 name server addresses.  The code below only
      overrides the IPv4 addresses.  */
   __res_iclose (&_res, true);
-  _res._u._ext.nscount = 0;
 
   /* Redirect queries to the server socket.  */
   if (test_verbose)