public inbox for glibc-cvs@sourceware.org
help / color / mirror / Atom feed
* [glibc/siddhesh/gai-cleanup2] Simplify allocations and fix merge and continue actions [BZ #28931]
@ 2022-03-08 14:09 Siddhesh Poyarekar
  0 siblings, 0 replies; 3+ messages in thread
From: Siddhesh Poyarekar @ 2022-03-08 14:09 UTC (permalink / raw)
  To: glibc-cvs

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=63da75f3c90c7f9265fa297441c52d001a8a363c

commit 63da75f3c90c7f9265fa297441c52d001a8a363c
Author: Siddhesh Poyarekar <siddhesh@sourceware.org>
Date:   Mon Mar 7 22:15:18 2022 +0530

    Simplify allocations and fix merge and continue actions [BZ #28931]
    
    Allocations for address tuples is currently a bit confusing because of
    the pointer chasing through PAT, making it hard to observe the sequence
    in which allocations have been made.  Narrow scope of the pointer
    chasing through PAT so that it is only used where necessary.
    
    This also tightens actions behaviour with the hosts database in
    getaddrinfo to comply with the manual text.  The "continue" action
    discards previous results and the "merge" action results in an immedate
    lookup failure.  Consequently, chaining of allocations across modules is
    no longer necessary, thus opening up cleanup opportunities.
    
    A test has been added that checks some combinations to ensure that they
    work correctly.  The "dns [SUCCESS=continue] dns" for example results in
    a segfault without this fix.
    
    Resolves: BZ #28931
    
    Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

Diff:
---
 nss/Makefile                |   1 +
 nss/tst-nss-gai-actions.c   | 242 ++++++++++++++++++++++++++++++++++++++++++++
 sysdeps/posix/getaddrinfo.c | 142 ++++++++++++++++----------
 3 files changed, 333 insertions(+), 52 deletions(-)

diff --git a/nss/Makefile b/nss/Makefile
index 552e5d03e1..43a0b9defe 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -78,6 +78,7 @@ tests += tst-nss-files-hosts-multi
 tests += tst-nss-files-hosts-getent
 tests += tst-nss-files-alias-leak
 tests += tst-nss-files-alias-truncated
+tests += tst-nss-gai-actions
 endif
 
 # If we have a thread library then we can test cancellation against
diff --git a/nss/tst-nss-gai-actions.c b/nss/tst-nss-gai-actions.c
new file mode 100644
index 0000000000..b5ba5f0138
--- /dev/null
+++ b/nss/tst-nss-gai-actions.c
@@ -0,0 +1,242 @@
+/* Test continue and merge NSS actions for getaddrinfo.
+   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/>.  */
+
+#include <dlfcn.h>
+#include <gnu/lib-names.h>
+#include <nss.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <support/check.h>
+#include <support/format_nss.h>
+#include <support/namespace.h>
+#include <support/support.h>
+#include <support/xstdio.h>
+#include <support/xunistd.h>
+
+enum
+{
+  ACTION_MERGE = 0,
+  ACTION_CONTINUE,
+};
+
+enum
+{
+  DB_FILES = 0,
+  DB_DNS,
+};
+
+struct test_params
+{
+  int db;
+  int action;
+  int family;
+  bool canon;
+};
+
+struct support_chroot *chroot_env;
+
+static void
+prepare (int argc, char **argv)
+{
+  chroot_env = support_chroot_create
+    ((struct support_chroot_configuration)
+     {
+       .resolv_conf = "",
+       .hosts = "",
+       .host_conf = "multi on\n",
+     });
+}
+
+/* Create the /etc/hosts file from outside the chroot.  */
+static void
+write_hosts (void)
+{
+  const int count = 512;
+
+  FILE *fp = xfopen (chroot_env->path_hosts, "w");
+  fputs ("127.0.0.1   localhost localhost.localdomain\n"
+         "::1         localhost localhost.localdomain\n",
+         fp);
+  for (int i = 1; i < count; ++i)
+    fprintf (fp, "192.0.%d.%d gnu.org\n", (i / 256) & 0xff, i & 0xff);
+  xfclose (fp);
+}
+
+static const char *
+family_str (int family)
+{
+  switch (family)
+    {
+    case AF_UNSPEC:
+      return "AF_UNSPEC";
+    case AF_INET:
+      return "AF_INET";
+    default:
+      __builtin_unreachable ();
+    }
+}
+
+static const char *
+db_str (int db)
+{
+  switch (db)
+    {
+    case DB_DNS:
+      return "dns";
+    case DB_FILES:
+      return "files";
+    default:
+      __builtin_unreachable ();
+    }
+}
+
+static const char *
+action_str (int action)
+{
+  switch (action)
+    {
+    case ACTION_MERGE:
+      return "merge";
+    case ACTION_CONTINUE:
+      return "continue";
+    default:
+      __builtin_unreachable ();
+    }
+}
+
+/* getaddrinfo test.  To be run from a subprocess.  */
+static void
+test_gai (void *closure)
+{
+  struct test_params *params = closure;
+
+  struct addrinfo hints =
+    {
+      .ai_family = params->family,
+    };
+
+  struct addrinfo *ai;
+
+  if (params->canon)
+    hints.ai_flags = AI_CANONNAME;
+
+  /* Use /etc/hosts in the chroot.  */
+  if (params->db == DB_FILES)
+    xchroot (chroot_env->path_chroot);
+
+  printf ("***** Testing \"%s [SUCCESS=%s] %s\" for family %s, %s\n",
+	  db_str (params->db), action_str (params->action),
+	  db_str (params->db), family_str (params->family),
+	  params->canon ? "AI_CANONNAME" : "");
+
+  int ret = getaddrinfo ("gnu.org", "80", &hints, &ai);
+
+  switch (params->action)
+    {
+    case ACTION_MERGE:
+      if (ret == 0)
+	{
+	  char *formatted = support_format_addrinfo (ai, ret);
+
+	  printf ("merge unexpectedly succeeded:\n %s\n", formatted);
+	  support_record_failure ();
+	  free (formatted);
+	}
+      else
+	return;
+    case ACTION_CONTINUE:
+	{
+	  char *formatted = support_format_addrinfo (ai, ret);
+
+	  if (params->db == DB_FILES)
+	    {
+	      /* Verify that the result appears exactly once.  */
+	      const char *expected = "address: STREAM/TCP 192.0.0.1 80\n"
+		"address: DGRAM/UDP 192.0.0.1 80\n"
+		"address: RAW/IP 192.0.0.1 80\n";
+
+	      const char *contains = strstr (formatted, expected);
+	      const char *contains2 = NULL;
+
+	      if (contains != NULL)
+		contains2 = strstr (contains + strlen (expected), expected);
+
+	      if (contains == NULL || contains2 != NULL)
+		{
+		  printf ("continue failed:\n%s\n", formatted);
+		  support_record_failure ();
+		}
+	    }
+	  else if (ret != 0)
+	    {
+	      printf ("continue unexpectedly failed:\n%s\n", formatted);
+	      support_record_failure ();
+	    }
+
+	  free (formatted);
+	  break;
+	}
+    default:
+      __builtin_unreachable ();
+    }
+}
+
+static void
+test_in_subprocess (int db, int action)
+{
+  char buf[32];
+
+  snprintf (buf, sizeof (buf), "%s [SUCCESS=%s] %s",
+	    db_str (db), action_str (action), db_str (db));
+  __nss_configure_lookup ("hosts", buf);
+
+  struct test_params params =
+    {
+      .db = db,
+      .action = action,
+      .family = AF_UNSPEC,
+      .canon = false,
+    };
+  support_isolate_in_subprocess (test_gai, &params);
+  params.family = AF_INET;
+  support_isolate_in_subprocess (test_gai, &params);
+  params.canon = true;
+  support_isolate_in_subprocess (test_gai, &params);
+}
+
+static int
+do_test (void)
+{
+  support_become_root ();
+  if (!support_can_chroot ())
+    FAIL_UNSUPPORTED ("Cannot chroot\n");
+
+  write_hosts ();
+  test_in_subprocess (DB_FILES, ACTION_CONTINUE);
+  test_in_subprocess (DB_FILES, ACTION_MERGE);
+  test_in_subprocess (DB_DNS, ACTION_CONTINUE);
+  test_in_subprocess (DB_DNS, ACTION_MERGE);
+
+  support_chroot_free (chroot_env);
+  return 0;
+}
+
+#define PREPARE prepare
+#include <support/test-driver.c>
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 18dccd5924..454a27eb2f 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -458,11 +458,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
   if (name != NULL)
     {
-      at = alloca_account (sizeof (struct gaih_addrtuple), alloca_used);
-      at->family = AF_UNSPEC;
-      at->scopeid = 0;
-      at->next = NULL;
-
       if (req->ai_flags & AI_IDN)
 	{
 	  char *out;
@@ -473,13 +468,21 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	  malloc_name = true;
 	}
 
-      if (__inet_aton_exact (name, (struct in_addr *) at->addr) != 0)
+      uint32_t addr[4];
+      if (__inet_aton_exact (name, (struct in_addr *) addr) != 0)
 	{
+	  at = alloca_account (sizeof (struct gaih_addrtuple), alloca_used);
+	  at->scopeid = 0;
+	  at->next = NULL;
+
 	  if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
-	    at->family = AF_INET;
+	    {
+	      memcpy (at->addr, addr, sizeof (at->addr));
+	      at->family = AF_INET;
+	    }
 	  else if (req->ai_family == AF_INET6 && (req->ai_flags & AI_V4MAPPED))
 	    {
-	      at->addr[3] = at->addr[0];
+	      at->addr[3] = addr[0];
 	      at->addr[2] = htonl (0xffff);
 	      at->addr[1] = 0;
 	      at->addr[0] = 0;
@@ -493,49 +496,62 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
 	  if (req->ai_flags & AI_CANONNAME)
 	    canon = name;
+
+	  goto process_list;
 	}
-      else if (at->family == AF_UNSPEC)
+
+      char *scope_delim = strchr (name, SCOPE_DELIMITER);
+      int e;
+
+      if (scope_delim == NULL)
+	e = inet_pton (AF_INET6, name, addr);
+      else
+	e = __inet_pton_length (AF_INET6, name, scope_delim - name, addr);
+
+      if (e > 0)
 	{
-	  char *scope_delim = strchr (name, SCOPE_DELIMITER);
-	  int e;
-	  if (scope_delim == NULL)
-	    e = inet_pton (AF_INET6, name, at->addr);
+	  at = alloca_account (sizeof (struct gaih_addrtuple),
+			       alloca_used);
+	  at->scopeid = 0;
+	  at->next = NULL;
+
+	  if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET6)
+	    {
+	      memcpy (at->addr, addr, sizeof (at->addr));
+	      at->family = AF_INET6;
+	    }
+	  else if (req->ai_family == AF_INET
+		   && IN6_IS_ADDR_V4MAPPED (addr))
+	    {
+	      at->addr[0] = addr[3];
+	      at->addr[1] = addr[1];
+	      at->addr[2] = addr[2];
+	      at->addr[3] = addr[3];
+	      at->family = AF_INET;
+	    }
 	  else
-	    e = __inet_pton_length (AF_INET6, name, scope_delim - name,
-				    at->addr);
-	  if (e > 0)
 	    {
-	      if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET6)
-		at->family = AF_INET6;
-	      else if (req->ai_family == AF_INET
-		       && IN6_IS_ADDR_V4MAPPED (at->addr))
-		{
-		  at->addr[0] = at->addr[3];
-		  at->family = AF_INET;
-		}
-	      else
-		{
-		  result = -EAI_ADDRFAMILY;
-		  goto free_and_return;
-		}
-
-	      if (scope_delim != NULL
-		  && __inet6_scopeid_pton ((struct in6_addr *) at->addr,
-					   scope_delim + 1,
-					   &at->scopeid) != 0)
-		{
-		  result = -EAI_NONAME;
-		  goto free_and_return;
-		}
+	      result = -EAI_ADDRFAMILY;
+	      goto free_and_return;
+	    }
 
-	      if (req->ai_flags & AI_CANONNAME)
-		canon = name;
+	  if (scope_delim != NULL
+	      && __inet6_scopeid_pton ((struct in6_addr *) at->addr,
+				       scope_delim + 1,
+				       &at->scopeid) != 0)
+	    {
+	      result = -EAI_NONAME;
+	      goto free_and_return;
 	    }
+
+	  if (req->ai_flags & AI_CANONNAME)
+	    canon = name;
+
+	  goto process_list;
 	}
 
-      if (at->family == AF_UNSPEC && (req->ai_flags & AI_NUMERICHOST) == 0)
+      if ((req->ai_flags & AI_NUMERICHOST) == 0)
 	{
-	  struct gaih_addrtuple **pat = &at;
 	  int no_data = 0;
 	  int no_inet6_data = 0;
 	  nss_action_list nip;
@@ -543,6 +559,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	  enum nss_status status = NSS_STATUS_UNAVAIL;
 	  int no_more;
 	  struct resolv_context *res_ctx = NULL;
+	  bool do_merge = false;
 
 	  /* If we do not have to look for IPv6 addresses or the canonical
 	     name, use the simple, old functions, which do not support
@@ -579,7 +596,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 			  result = -EAI_MEMORY;
 			  goto free_and_return;
 			}
-		      *pat = addrmem;
+		      at = addrmem;
 		    }
 		  else
 		    {
@@ -632,6 +649,8 @@ gaih_inet (const char *name, const struct gaih_service *service,
 		    }
 
 		  struct gaih_addrtuple *addrfree = addrmem;
+		  struct gaih_addrtuple **pat = &at;
+
 		  for (int i = 0; i < air->naddrs; ++i)
 		    {
 		      socklen_t size = (air->family[i] == AF_INET
@@ -695,12 +714,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
 		  free (air);
 
-		  if (at->family == AF_UNSPEC)
-		    {
-		      result = -EAI_NONAME;
-		      goto free_and_return;
-		    }
-
 		  goto process_list;
 		}
 	      else if (err == 0)
@@ -732,6 +745,21 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
 	  while (!no_more)
 	    {
+	      /* Always start afresh; continue should discard previous results
+		 and the hosts database does not support merge.  */
+	      at = NULL;
+	      free (canonbuf);
+	      free (addrmem);
+	      canon = canonbuf = NULL;
+	      addrmem = NULL;
+
+	      if (do_merge)
+		{
+		  __set_h_errno (NETDB_INTERNAL);
+		  __set_errno (EBUSY);
+		  break;
+		}
+
 	      no_data = 0;
 	      nss_gethostbyname4_r *fct4 = NULL;
 
@@ -744,12 +772,14 @@ gaih_inet (const char *name, const struct gaih_service *service,
 		{
 		  while (1)
 		    {
-		      status = DL_CALL_FCT (fct4, (name, pat,
+		      status = DL_CALL_FCT (fct4, (name, &at,
 						   tmpbuf->data, tmpbuf->length,
 						   &errno, &h_errno,
 						   NULL));
 		      if (status == NSS_STATUS_SUCCESS)
 			break;
+		      /* gethostbyname4_r may write into AT, so reset it.  */
+		      at = NULL;
 		      if (status != NSS_STATUS_TRYAGAIN
 			  || errno != ERANGE || h_errno != NETDB_INTERNAL)
 			{
@@ -774,7 +804,9 @@ gaih_inet (const char *name, const struct gaih_service *service,
 		      no_data = 1;
 
 		      if ((req->ai_flags & AI_CANONNAME) != 0 && canon == NULL)
-			canon = (*pat)->name;
+			canon = at->name;
+
+		      struct gaih_addrtuple **pat = &at;
 
 		      while (*pat != NULL)
 			{
@@ -826,6 +858,8 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
 		  if (fct != NULL)
 		    {
+		      struct gaih_addrtuple **pat = &at;
+
 		      if (req->ai_family == AF_INET6
 			  || req->ai_family == AF_UNSPEC)
 			{
@@ -899,6 +933,10 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	      if (nss_next_action (nip, status) == NSS_ACTION_RETURN)
 		break;
 
+	      /* The hosts database does not support MERGE.  */
+	      if (nss_next_action (nip, status) == NSS_ACTION_MERGE)
+		do_merge = true;
+
 	      nip++;
 	      if (nip->module == NULL)
 		no_more = -1;
@@ -930,7 +968,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	}
 
     process_list:
-      if (at->family == AF_UNSPEC)
+      if (at == NULL)
 	{
 	  result = -EAI_NONAME;
 	  goto free_and_return;


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [glibc/siddhesh/gai-cleanup2] Simplify allocations and fix merge and continue actions [BZ #28931]
@ 2022-03-14 14:16 Siddhesh Poyarekar
  0 siblings, 0 replies; 3+ messages in thread
From: Siddhesh Poyarekar @ 2022-03-14 14:16 UTC (permalink / raw)
  To: glibc-cvs

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=62d440fb2fcc4a34fa500dd7e975e4a5587b7d0c

commit 62d440fb2fcc4a34fa500dd7e975e4a5587b7d0c
Author: Siddhesh Poyarekar <siddhesh@sourceware.org>
Date:   Mon Mar 7 22:15:18 2022 +0530

    Simplify allocations and fix merge and continue actions [BZ #28931]
    
    Allocations for address tuples is currently a bit confusing because of
    the pointer chasing through PAT, making it hard to observe the sequence
    in which allocations have been made.  Narrow scope of the pointer
    chasing through PAT so that it is only used where necessary.
    
    This also tightens actions behaviour with the hosts database in
    getaddrinfo to comply with the manual text.  The "continue" action
    discards previous results and the "merge" action results in an immedate
    lookup failure.  Consequently, chaining of allocations across modules is
    no longer necessary, thus opening up cleanup opportunities.
    
    A test has been added that checks some combinations to ensure that they
    work correctly.
    
    Resolves: BZ #28931
    
    Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

Diff:
---
 nss/Makefile                |   1 +
 nss/tst-nss-gai-actions.c   | 208 ++++++++++++++++++++++++++++++++++++++++++++
 sysdeps/posix/getaddrinfo.c | 142 +++++++++++++++++++-----------
 3 files changed, 299 insertions(+), 52 deletions(-)

diff --git a/nss/Makefile b/nss/Makefile
index de439d4911..8ca69ed25b 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -79,6 +79,7 @@ tests += tst-nss-files-hosts-multi
 tests += tst-nss-files-hosts-getent
 tests += tst-nss-files-alias-leak
 tests += tst-nss-files-alias-truncated
+tests += tst-nss-gai-actions
 endif
 
 # If we have a thread library then we can test cancellation against
diff --git a/nss/tst-nss-gai-actions.c b/nss/tst-nss-gai-actions.c
new file mode 100644
index 0000000000..402c999b0d
--- /dev/null
+++ b/nss/tst-nss-gai-actions.c
@@ -0,0 +1,208 @@
+/* Test continue and merge NSS actions for getaddrinfo.
+   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/>.  */
+
+#include <dlfcn.h>
+#include <gnu/lib-names.h>
+#include <nss.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <support/check.h>
+#include <support/format_nss.h>
+#include <support/namespace.h>
+#include <support/support.h>
+#include <support/xstdio.h>
+#include <support/xunistd.h>
+
+enum
+{
+  ACTION_MERGE = 0,
+  ACTION_CONTINUE,
+};
+
+struct test_params
+{
+  int action;
+  int family;
+  bool canon;
+};
+
+struct support_chroot *chroot_env;
+
+static void
+prepare (int argc, char **argv)
+{
+  chroot_env = support_chroot_create
+    ((struct support_chroot_configuration)
+     {
+       .resolv_conf = "",
+       .hosts = "",
+       .host_conf = "multi on\n",
+     });
+}
+
+/* Create the /etc/hosts file from outside the chroot.  */
+static void
+write_hosts (void)
+{
+  const int count = 512;
+
+  FILE *fp = xfopen (chroot_env->path_hosts, "w");
+  fputs ("127.0.0.1   localhost localhost.localdomain\n"
+         "::1         localhost localhost.localdomain\n",
+         fp);
+  for (int i = 1; i < count; ++i)
+    fprintf (fp, "192.0.%d.%d example.org\n", (i / 256) & 0xff, i & 0xff);
+  xfclose (fp);
+}
+
+static const char *
+family_str (int family)
+{
+  switch (family)
+    {
+    case AF_UNSPEC:
+      return "AF_UNSPEC";
+    case AF_INET:
+      return "AF_INET";
+    default:
+      __builtin_unreachable ();
+    }
+}
+
+static const char *
+action_str (int action)
+{
+  switch (action)
+    {
+    case ACTION_MERGE:
+      return "merge";
+    case ACTION_CONTINUE:
+      return "continue";
+    default:
+      __builtin_unreachable ();
+    }
+}
+
+/* getaddrinfo test.  To be run from a subprocess.  */
+static void
+test_gai (void *closure)
+{
+  struct test_params *params = closure;
+
+  struct addrinfo hints =
+    {
+      .ai_family = params->family,
+    };
+
+  struct addrinfo *ai;
+
+  if (params->canon)
+    hints.ai_flags = AI_CANONNAME;
+
+  /* Use /etc/hosts in the chroot.  */
+  xchroot (chroot_env->path_chroot);
+
+  printf ("***** Testing \"files [SUCCESS=%s] files\" for family %s, %s\n",
+	  action_str (params->action), family_str (params->family),
+	  params->canon ? "AI_CANONNAME" : "");
+
+  int ret = getaddrinfo ("example.org", "80", &hints, &ai);
+
+  switch (params->action)
+    {
+    case ACTION_MERGE:
+      if (ret == 0)
+	{
+	  char *formatted = support_format_addrinfo (ai, ret);
+
+	  printf ("merge unexpectedly succeeded:\n %s\n", formatted);
+	  support_record_failure ();
+	  free (formatted);
+	}
+      else
+	return;
+    case ACTION_CONTINUE:
+	{
+	  char *formatted = support_format_addrinfo (ai, ret);
+
+	  /* Verify that the result appears exactly once.  */
+	  const char *expected = "address: STREAM/TCP 192.0.0.1 80\n"
+	    "address: DGRAM/UDP 192.0.0.1 80\n"
+	    "address: RAW/IP 192.0.0.1 80\n";
+
+	  const char *contains = strstr (formatted, expected);
+	  const char *contains2 = NULL;
+
+	  if (contains != NULL)
+	    contains2 = strstr (contains + strlen (expected), expected);
+
+	  if (contains == NULL || contains2 != NULL)
+	    {
+	      printf ("continue failed:\n%s\n", formatted);
+	      support_record_failure ();
+	    }
+
+	  free (formatted);
+	  break;
+	}
+    default:
+      __builtin_unreachable ();
+    }
+}
+
+static void
+test_in_subprocess (int action)
+{
+  char buf[32];
+
+  snprintf (buf, sizeof (buf), "files [SUCCESS=%s] files",
+	    action_str (action));
+  __nss_configure_lookup ("hosts", buf);
+
+  struct test_params params =
+    {
+      .action = action,
+      .family = AF_UNSPEC,
+      .canon = false,
+    };
+  support_isolate_in_subprocess (test_gai, &params);
+  params.family = AF_INET;
+  support_isolate_in_subprocess (test_gai, &params);
+  params.canon = true;
+  support_isolate_in_subprocess (test_gai, &params);
+}
+
+static int
+do_test (void)
+{
+  support_become_root ();
+  if (!support_can_chroot ())
+    FAIL_UNSUPPORTED ("Cannot chroot\n");
+
+  write_hosts ();
+  test_in_subprocess (ACTION_CONTINUE);
+  test_in_subprocess (ACTION_MERGE);
+
+  support_chroot_free (chroot_env);
+  return 0;
+}
+
+#define PREPARE prepare
+#include <support/test-driver.c>
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 18dccd5924..454a27eb2f 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -458,11 +458,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
   if (name != NULL)
     {
-      at = alloca_account (sizeof (struct gaih_addrtuple), alloca_used);
-      at->family = AF_UNSPEC;
-      at->scopeid = 0;
-      at->next = NULL;
-
       if (req->ai_flags & AI_IDN)
 	{
 	  char *out;
@@ -473,13 +468,21 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	  malloc_name = true;
 	}
 
-      if (__inet_aton_exact (name, (struct in_addr *) at->addr) != 0)
+      uint32_t addr[4];
+      if (__inet_aton_exact (name, (struct in_addr *) addr) != 0)
 	{
+	  at = alloca_account (sizeof (struct gaih_addrtuple), alloca_used);
+	  at->scopeid = 0;
+	  at->next = NULL;
+
 	  if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
-	    at->family = AF_INET;
+	    {
+	      memcpy (at->addr, addr, sizeof (at->addr));
+	      at->family = AF_INET;
+	    }
 	  else if (req->ai_family == AF_INET6 && (req->ai_flags & AI_V4MAPPED))
 	    {
-	      at->addr[3] = at->addr[0];
+	      at->addr[3] = addr[0];
 	      at->addr[2] = htonl (0xffff);
 	      at->addr[1] = 0;
 	      at->addr[0] = 0;
@@ -493,49 +496,62 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
 	  if (req->ai_flags & AI_CANONNAME)
 	    canon = name;
+
+	  goto process_list;
 	}
-      else if (at->family == AF_UNSPEC)
+
+      char *scope_delim = strchr (name, SCOPE_DELIMITER);
+      int e;
+
+      if (scope_delim == NULL)
+	e = inet_pton (AF_INET6, name, addr);
+      else
+	e = __inet_pton_length (AF_INET6, name, scope_delim - name, addr);
+
+      if (e > 0)
 	{
-	  char *scope_delim = strchr (name, SCOPE_DELIMITER);
-	  int e;
-	  if (scope_delim == NULL)
-	    e = inet_pton (AF_INET6, name, at->addr);
+	  at = alloca_account (sizeof (struct gaih_addrtuple),
+			       alloca_used);
+	  at->scopeid = 0;
+	  at->next = NULL;
+
+	  if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET6)
+	    {
+	      memcpy (at->addr, addr, sizeof (at->addr));
+	      at->family = AF_INET6;
+	    }
+	  else if (req->ai_family == AF_INET
+		   && IN6_IS_ADDR_V4MAPPED (addr))
+	    {
+	      at->addr[0] = addr[3];
+	      at->addr[1] = addr[1];
+	      at->addr[2] = addr[2];
+	      at->addr[3] = addr[3];
+	      at->family = AF_INET;
+	    }
 	  else
-	    e = __inet_pton_length (AF_INET6, name, scope_delim - name,
-				    at->addr);
-	  if (e > 0)
 	    {
-	      if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET6)
-		at->family = AF_INET6;
-	      else if (req->ai_family == AF_INET
-		       && IN6_IS_ADDR_V4MAPPED (at->addr))
-		{
-		  at->addr[0] = at->addr[3];
-		  at->family = AF_INET;
-		}
-	      else
-		{
-		  result = -EAI_ADDRFAMILY;
-		  goto free_and_return;
-		}
-
-	      if (scope_delim != NULL
-		  && __inet6_scopeid_pton ((struct in6_addr *) at->addr,
-					   scope_delim + 1,
-					   &at->scopeid) != 0)
-		{
-		  result = -EAI_NONAME;
-		  goto free_and_return;
-		}
+	      result = -EAI_ADDRFAMILY;
+	      goto free_and_return;
+	    }
 
-	      if (req->ai_flags & AI_CANONNAME)
-		canon = name;
+	  if (scope_delim != NULL
+	      && __inet6_scopeid_pton ((struct in6_addr *) at->addr,
+				       scope_delim + 1,
+				       &at->scopeid) != 0)
+	    {
+	      result = -EAI_NONAME;
+	      goto free_and_return;
 	    }
+
+	  if (req->ai_flags & AI_CANONNAME)
+	    canon = name;
+
+	  goto process_list;
 	}
 
-      if (at->family == AF_UNSPEC && (req->ai_flags & AI_NUMERICHOST) == 0)
+      if ((req->ai_flags & AI_NUMERICHOST) == 0)
 	{
-	  struct gaih_addrtuple **pat = &at;
 	  int no_data = 0;
 	  int no_inet6_data = 0;
 	  nss_action_list nip;
@@ -543,6 +559,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	  enum nss_status status = NSS_STATUS_UNAVAIL;
 	  int no_more;
 	  struct resolv_context *res_ctx = NULL;
+	  bool do_merge = false;
 
 	  /* If we do not have to look for IPv6 addresses or the canonical
 	     name, use the simple, old functions, which do not support
@@ -579,7 +596,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 			  result = -EAI_MEMORY;
 			  goto free_and_return;
 			}
-		      *pat = addrmem;
+		      at = addrmem;
 		    }
 		  else
 		    {
@@ -632,6 +649,8 @@ gaih_inet (const char *name, const struct gaih_service *service,
 		    }
 
 		  struct gaih_addrtuple *addrfree = addrmem;
+		  struct gaih_addrtuple **pat = &at;
+
 		  for (int i = 0; i < air->naddrs; ++i)
 		    {
 		      socklen_t size = (air->family[i] == AF_INET
@@ -695,12 +714,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
 		  free (air);
 
-		  if (at->family == AF_UNSPEC)
-		    {
-		      result = -EAI_NONAME;
-		      goto free_and_return;
-		    }
-
 		  goto process_list;
 		}
 	      else if (err == 0)
@@ -732,6 +745,21 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
 	  while (!no_more)
 	    {
+	      /* Always start afresh; continue should discard previous results
+		 and the hosts database does not support merge.  */
+	      at = NULL;
+	      free (canonbuf);
+	      free (addrmem);
+	      canon = canonbuf = NULL;
+	      addrmem = NULL;
+
+	      if (do_merge)
+		{
+		  __set_h_errno (NETDB_INTERNAL);
+		  __set_errno (EBUSY);
+		  break;
+		}
+
 	      no_data = 0;
 	      nss_gethostbyname4_r *fct4 = NULL;
 
@@ -744,12 +772,14 @@ gaih_inet (const char *name, const struct gaih_service *service,
 		{
 		  while (1)
 		    {
-		      status = DL_CALL_FCT (fct4, (name, pat,
+		      status = DL_CALL_FCT (fct4, (name, &at,
 						   tmpbuf->data, tmpbuf->length,
 						   &errno, &h_errno,
 						   NULL));
 		      if (status == NSS_STATUS_SUCCESS)
 			break;
+		      /* gethostbyname4_r may write into AT, so reset it.  */
+		      at = NULL;
 		      if (status != NSS_STATUS_TRYAGAIN
 			  || errno != ERANGE || h_errno != NETDB_INTERNAL)
 			{
@@ -774,7 +804,9 @@ gaih_inet (const char *name, const struct gaih_service *service,
 		      no_data = 1;
 
 		      if ((req->ai_flags & AI_CANONNAME) != 0 && canon == NULL)
-			canon = (*pat)->name;
+			canon = at->name;
+
+		      struct gaih_addrtuple **pat = &at;
 
 		      while (*pat != NULL)
 			{
@@ -826,6 +858,8 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
 		  if (fct != NULL)
 		    {
+		      struct gaih_addrtuple **pat = &at;
+
 		      if (req->ai_family == AF_INET6
 			  || req->ai_family == AF_UNSPEC)
 			{
@@ -899,6 +933,10 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	      if (nss_next_action (nip, status) == NSS_ACTION_RETURN)
 		break;
 
+	      /* The hosts database does not support MERGE.  */
+	      if (nss_next_action (nip, status) == NSS_ACTION_MERGE)
+		do_merge = true;
+
 	      nip++;
 	      if (nip->module == NULL)
 		no_more = -1;
@@ -930,7 +968,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	}
 
     process_list:
-      if (at->family == AF_UNSPEC)
+      if (at == NULL)
 	{
 	  result = -EAI_NONAME;
 	  goto free_and_return;


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [glibc/siddhesh/gai-cleanup2] Simplify allocations and fix merge and continue actions [BZ #28931]
@ 2022-03-07 16:55 Siddhesh Poyarekar
  0 siblings, 0 replies; 3+ messages in thread
From: Siddhesh Poyarekar @ 2022-03-07 16:55 UTC (permalink / raw)
  To: glibc-cvs

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=b97920970eeecfcfb9b323e901ab9071fe7db119

commit b97920970eeecfcfb9b323e901ab9071fe7db119
Author: Siddhesh Poyarekar <siddhesh@sourceware.org>
Date:   Mon Mar 7 22:15:18 2022 +0530

    Simplify allocations and fix merge and continue actions [BZ #28931]
    
    Allocations for address tuples is currently a bit confusing because of
    the pointer chasing through PAT, making it hard to observe the sequence
    in which allocations have been made.  Narrow scope of the pointer
    chasing through PAT so that it is only used where necessary.
    
    This also tightens actions behaviour with the hosts database in
    getaddrinfo to comply with the manual text.  The "continue" action
    discards previous results and the "merge" action results in an immedate
    lookup failure.  Consequently, chaining of allocations across modules is
    no longer necessary, thus opening up cleanup opportunities.
    
    A test has been added that checks some combinations to ensure that they
    work correctly.  The "dns [SUCCESS=continue] dns" for example results in
    a segfault without this fix.
    
    Resolves: BZ #28931
    
    Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

Diff:
---
 nss/Makefile                |   1 +
 nss/tst-nss-gai-actions.c   | 242 ++++++++++++++++++++++++++++++++++++++++++++
 sysdeps/posix/getaddrinfo.c | 142 ++++++++++++++++----------
 3 files changed, 333 insertions(+), 52 deletions(-)

diff --git a/nss/Makefile b/nss/Makefile
index 552e5d03e1..43a0b9defe 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -78,6 +78,7 @@ tests += tst-nss-files-hosts-multi
 tests += tst-nss-files-hosts-getent
 tests += tst-nss-files-alias-leak
 tests += tst-nss-files-alias-truncated
+tests += tst-nss-gai-actions
 endif
 
 # If we have a thread library then we can test cancellation against
diff --git a/nss/tst-nss-gai-actions.c b/nss/tst-nss-gai-actions.c
new file mode 100644
index 0000000000..b5ba5f0138
--- /dev/null
+++ b/nss/tst-nss-gai-actions.c
@@ -0,0 +1,242 @@
+/* Test continue and merge NSS actions for getaddrinfo.
+   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/>.  */
+
+#include <dlfcn.h>
+#include <gnu/lib-names.h>
+#include <nss.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <support/check.h>
+#include <support/format_nss.h>
+#include <support/namespace.h>
+#include <support/support.h>
+#include <support/xstdio.h>
+#include <support/xunistd.h>
+
+enum
+{
+  ACTION_MERGE = 0,
+  ACTION_CONTINUE,
+};
+
+enum
+{
+  DB_FILES = 0,
+  DB_DNS,
+};
+
+struct test_params
+{
+  int db;
+  int action;
+  int family;
+  bool canon;
+};
+
+struct support_chroot *chroot_env;
+
+static void
+prepare (int argc, char **argv)
+{
+  chroot_env = support_chroot_create
+    ((struct support_chroot_configuration)
+     {
+       .resolv_conf = "",
+       .hosts = "",
+       .host_conf = "multi on\n",
+     });
+}
+
+/* Create the /etc/hosts file from outside the chroot.  */
+static void
+write_hosts (void)
+{
+  const int count = 512;
+
+  FILE *fp = xfopen (chroot_env->path_hosts, "w");
+  fputs ("127.0.0.1   localhost localhost.localdomain\n"
+         "::1         localhost localhost.localdomain\n",
+         fp);
+  for (int i = 1; i < count; ++i)
+    fprintf (fp, "192.0.%d.%d gnu.org\n", (i / 256) & 0xff, i & 0xff);
+  xfclose (fp);
+}
+
+static const char *
+family_str (int family)
+{
+  switch (family)
+    {
+    case AF_UNSPEC:
+      return "AF_UNSPEC";
+    case AF_INET:
+      return "AF_INET";
+    default:
+      __builtin_unreachable ();
+    }
+}
+
+static const char *
+db_str (int db)
+{
+  switch (db)
+    {
+    case DB_DNS:
+      return "dns";
+    case DB_FILES:
+      return "files";
+    default:
+      __builtin_unreachable ();
+    }
+}
+
+static const char *
+action_str (int action)
+{
+  switch (action)
+    {
+    case ACTION_MERGE:
+      return "merge";
+    case ACTION_CONTINUE:
+      return "continue";
+    default:
+      __builtin_unreachable ();
+    }
+}
+
+/* getaddrinfo test.  To be run from a subprocess.  */
+static void
+test_gai (void *closure)
+{
+  struct test_params *params = closure;
+
+  struct addrinfo hints =
+    {
+      .ai_family = params->family,
+    };
+
+  struct addrinfo *ai;
+
+  if (params->canon)
+    hints.ai_flags = AI_CANONNAME;
+
+  /* Use /etc/hosts in the chroot.  */
+  if (params->db == DB_FILES)
+    xchroot (chroot_env->path_chroot);
+
+  printf ("***** Testing \"%s [SUCCESS=%s] %s\" for family %s, %s\n",
+	  db_str (params->db), action_str (params->action),
+	  db_str (params->db), family_str (params->family),
+	  params->canon ? "AI_CANONNAME" : "");
+
+  int ret = getaddrinfo ("gnu.org", "80", &hints, &ai);
+
+  switch (params->action)
+    {
+    case ACTION_MERGE:
+      if (ret == 0)
+	{
+	  char *formatted = support_format_addrinfo (ai, ret);
+
+	  printf ("merge unexpectedly succeeded:\n %s\n", formatted);
+	  support_record_failure ();
+	  free (formatted);
+	}
+      else
+	return;
+    case ACTION_CONTINUE:
+	{
+	  char *formatted = support_format_addrinfo (ai, ret);
+
+	  if (params->db == DB_FILES)
+	    {
+	      /* Verify that the result appears exactly once.  */
+	      const char *expected = "address: STREAM/TCP 192.0.0.1 80\n"
+		"address: DGRAM/UDP 192.0.0.1 80\n"
+		"address: RAW/IP 192.0.0.1 80\n";
+
+	      const char *contains = strstr (formatted, expected);
+	      const char *contains2 = NULL;
+
+	      if (contains != NULL)
+		contains2 = strstr (contains + strlen (expected), expected);
+
+	      if (contains == NULL || contains2 != NULL)
+		{
+		  printf ("continue failed:\n%s\n", formatted);
+		  support_record_failure ();
+		}
+	    }
+	  else if (ret != 0)
+	    {
+	      printf ("continue unexpectedly failed:\n%s\n", formatted);
+	      support_record_failure ();
+	    }
+
+	  free (formatted);
+	  break;
+	}
+    default:
+      __builtin_unreachable ();
+    }
+}
+
+static void
+test_in_subprocess (int db, int action)
+{
+  char buf[32];
+
+  snprintf (buf, sizeof (buf), "%s [SUCCESS=%s] %s",
+	    db_str (db), action_str (action), db_str (db));
+  __nss_configure_lookup ("hosts", buf);
+
+  struct test_params params =
+    {
+      .db = db,
+      .action = action,
+      .family = AF_UNSPEC,
+      .canon = false,
+    };
+  support_isolate_in_subprocess (test_gai, &params);
+  params.family = AF_INET;
+  support_isolate_in_subprocess (test_gai, &params);
+  params.canon = true;
+  support_isolate_in_subprocess (test_gai, &params);
+}
+
+static int
+do_test (void)
+{
+  support_become_root ();
+  if (!support_can_chroot ())
+    FAIL_UNSUPPORTED ("Cannot chroot\n");
+
+  write_hosts ();
+  test_in_subprocess (DB_FILES, ACTION_CONTINUE);
+  test_in_subprocess (DB_FILES, ACTION_MERGE);
+  test_in_subprocess (DB_DNS, ACTION_CONTINUE);
+  test_in_subprocess (DB_DNS, ACTION_MERGE);
+
+  support_chroot_free (chroot_env);
+  return 0;
+}
+
+#define PREPARE prepare
+#include <support/test-driver.c>
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 18dccd5924..454a27eb2f 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -458,11 +458,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
   if (name != NULL)
     {
-      at = alloca_account (sizeof (struct gaih_addrtuple), alloca_used);
-      at->family = AF_UNSPEC;
-      at->scopeid = 0;
-      at->next = NULL;
-
       if (req->ai_flags & AI_IDN)
 	{
 	  char *out;
@@ -473,13 +468,21 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	  malloc_name = true;
 	}
 
-      if (__inet_aton_exact (name, (struct in_addr *) at->addr) != 0)
+      uint32_t addr[4];
+      if (__inet_aton_exact (name, (struct in_addr *) addr) != 0)
 	{
+	  at = alloca_account (sizeof (struct gaih_addrtuple), alloca_used);
+	  at->scopeid = 0;
+	  at->next = NULL;
+
 	  if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
-	    at->family = AF_INET;
+	    {
+	      memcpy (at->addr, addr, sizeof (at->addr));
+	      at->family = AF_INET;
+	    }
 	  else if (req->ai_family == AF_INET6 && (req->ai_flags & AI_V4MAPPED))
 	    {
-	      at->addr[3] = at->addr[0];
+	      at->addr[3] = addr[0];
 	      at->addr[2] = htonl (0xffff);
 	      at->addr[1] = 0;
 	      at->addr[0] = 0;
@@ -493,49 +496,62 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
 	  if (req->ai_flags & AI_CANONNAME)
 	    canon = name;
+
+	  goto process_list;
 	}
-      else if (at->family == AF_UNSPEC)
+
+      char *scope_delim = strchr (name, SCOPE_DELIMITER);
+      int e;
+
+      if (scope_delim == NULL)
+	e = inet_pton (AF_INET6, name, addr);
+      else
+	e = __inet_pton_length (AF_INET6, name, scope_delim - name, addr);
+
+      if (e > 0)
 	{
-	  char *scope_delim = strchr (name, SCOPE_DELIMITER);
-	  int e;
-	  if (scope_delim == NULL)
-	    e = inet_pton (AF_INET6, name, at->addr);
+	  at = alloca_account (sizeof (struct gaih_addrtuple),
+			       alloca_used);
+	  at->scopeid = 0;
+	  at->next = NULL;
+
+	  if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET6)
+	    {
+	      memcpy (at->addr, addr, sizeof (at->addr));
+	      at->family = AF_INET6;
+	    }
+	  else if (req->ai_family == AF_INET
+		   && IN6_IS_ADDR_V4MAPPED (addr))
+	    {
+	      at->addr[0] = addr[3];
+	      at->addr[1] = addr[1];
+	      at->addr[2] = addr[2];
+	      at->addr[3] = addr[3];
+	      at->family = AF_INET;
+	    }
 	  else
-	    e = __inet_pton_length (AF_INET6, name, scope_delim - name,
-				    at->addr);
-	  if (e > 0)
 	    {
-	      if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET6)
-		at->family = AF_INET6;
-	      else if (req->ai_family == AF_INET
-		       && IN6_IS_ADDR_V4MAPPED (at->addr))
-		{
-		  at->addr[0] = at->addr[3];
-		  at->family = AF_INET;
-		}
-	      else
-		{
-		  result = -EAI_ADDRFAMILY;
-		  goto free_and_return;
-		}
-
-	      if (scope_delim != NULL
-		  && __inet6_scopeid_pton ((struct in6_addr *) at->addr,
-					   scope_delim + 1,
-					   &at->scopeid) != 0)
-		{
-		  result = -EAI_NONAME;
-		  goto free_and_return;
-		}
+	      result = -EAI_ADDRFAMILY;
+	      goto free_and_return;
+	    }
 
-	      if (req->ai_flags & AI_CANONNAME)
-		canon = name;
+	  if (scope_delim != NULL
+	      && __inet6_scopeid_pton ((struct in6_addr *) at->addr,
+				       scope_delim + 1,
+				       &at->scopeid) != 0)
+	    {
+	      result = -EAI_NONAME;
+	      goto free_and_return;
 	    }
+
+	  if (req->ai_flags & AI_CANONNAME)
+	    canon = name;
+
+	  goto process_list;
 	}
 
-      if (at->family == AF_UNSPEC && (req->ai_flags & AI_NUMERICHOST) == 0)
+      if ((req->ai_flags & AI_NUMERICHOST) == 0)
 	{
-	  struct gaih_addrtuple **pat = &at;
 	  int no_data = 0;
 	  int no_inet6_data = 0;
 	  nss_action_list nip;
@@ -543,6 +559,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	  enum nss_status status = NSS_STATUS_UNAVAIL;
 	  int no_more;
 	  struct resolv_context *res_ctx = NULL;
+	  bool do_merge = false;
 
 	  /* If we do not have to look for IPv6 addresses or the canonical
 	     name, use the simple, old functions, which do not support
@@ -579,7 +596,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 			  result = -EAI_MEMORY;
 			  goto free_and_return;
 			}
-		      *pat = addrmem;
+		      at = addrmem;
 		    }
 		  else
 		    {
@@ -632,6 +649,8 @@ gaih_inet (const char *name, const struct gaih_service *service,
 		    }
 
 		  struct gaih_addrtuple *addrfree = addrmem;
+		  struct gaih_addrtuple **pat = &at;
+
 		  for (int i = 0; i < air->naddrs; ++i)
 		    {
 		      socklen_t size = (air->family[i] == AF_INET
@@ -695,12 +714,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
 		  free (air);
 
-		  if (at->family == AF_UNSPEC)
-		    {
-		      result = -EAI_NONAME;
-		      goto free_and_return;
-		    }
-
 		  goto process_list;
 		}
 	      else if (err == 0)
@@ -732,6 +745,21 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
 	  while (!no_more)
 	    {
+	      /* Always start afresh; continue should discard previous results
+		 and the hosts database does not support merge.  */
+	      at = NULL;
+	      free (canonbuf);
+	      free (addrmem);
+	      canon = canonbuf = NULL;
+	      addrmem = NULL;
+
+	      if (do_merge)
+		{
+		  __set_h_errno (NETDB_INTERNAL);
+		  __set_errno (EBUSY);
+		  break;
+		}
+
 	      no_data = 0;
 	      nss_gethostbyname4_r *fct4 = NULL;
 
@@ -744,12 +772,14 @@ gaih_inet (const char *name, const struct gaih_service *service,
 		{
 		  while (1)
 		    {
-		      status = DL_CALL_FCT (fct4, (name, pat,
+		      status = DL_CALL_FCT (fct4, (name, &at,
 						   tmpbuf->data, tmpbuf->length,
 						   &errno, &h_errno,
 						   NULL));
 		      if (status == NSS_STATUS_SUCCESS)
 			break;
+		      /* gethostbyname4_r may write into AT, so reset it.  */
+		      at = NULL;
 		      if (status != NSS_STATUS_TRYAGAIN
 			  || errno != ERANGE || h_errno != NETDB_INTERNAL)
 			{
@@ -774,7 +804,9 @@ gaih_inet (const char *name, const struct gaih_service *service,
 		      no_data = 1;
 
 		      if ((req->ai_flags & AI_CANONNAME) != 0 && canon == NULL)
-			canon = (*pat)->name;
+			canon = at->name;
+
+		      struct gaih_addrtuple **pat = &at;
 
 		      while (*pat != NULL)
 			{
@@ -826,6 +858,8 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
 		  if (fct != NULL)
 		    {
+		      struct gaih_addrtuple **pat = &at;
+
 		      if (req->ai_family == AF_INET6
 			  || req->ai_family == AF_UNSPEC)
 			{
@@ -899,6 +933,10 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	      if (nss_next_action (nip, status) == NSS_ACTION_RETURN)
 		break;
 
+	      /* The hosts database does not support MERGE.  */
+	      if (nss_next_action (nip, status) == NSS_ACTION_MERGE)
+		do_merge = true;
+
 	      nip++;
 	      if (nip->module == NULL)
 		no_more = -1;
@@ -930,7 +968,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	}
 
     process_list:
-      if (at->family == AF_UNSPEC)
+      if (at == NULL)
 	{
 	  result = -EAI_NONAME;
 	  goto free_and_return;


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-03-14 14:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-08 14:09 [glibc/siddhesh/gai-cleanup2] Simplify allocations and fix merge and continue actions [BZ #28931] Siddhesh Poyarekar
  -- strict thread matches above, loose matches on Subject: below --
2022-03-14 14:16 Siddhesh Poyarekar
2022-03-07 16:55 Siddhesh Poyarekar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).