[pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix

public inbox for libc-stable@sourceware.org
 help / color / mirror / Atom feed

* [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix
@ 2023-09-16  0:48 Siddhesh Poyarekar
  2023-09-16  0:48 ` [pushed 2.34 01/14] nss: Sort tests and tests-container and put one test per line Siddhesh Poyarekar
                   ` (13 more replies)
  0 siblings, 14 replies; 15+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-16  0:48 UTC (permalink / raw)
  To: libc-stable

Backport refactoring of gaih_inet to clean up allocations and fix
CVE-2023-4806.

Siddhesh Poyarekar (14):
  nss: Sort tests and tests-container and put one test per line
  gethosts: Remove unused argument _type
  gaih_inet: Simplify canon name resolution
  getaddrinfo: Fix leak with AI_ALL [BZ #28852]
  gaih_inet: Simplify service resolution
  gaih_inet: make numeric lookup a separate routine
  gaih_inet: Split simple gethostbyname into its own function
  gaih_inet: Split nscd lookup code into its own function.
  gaih_inet: separate nss lookup loop into its own function
  gaih_inet: make gethosts into a function
  gaih_inet: split loopback lookup into its own function
  gaih_inet: Split result generation into its own function
  gethosts: Return EAI_MEMORY on allocation failure
  getaddrinfo: Fix use after free in getcanonname (CVE-2023-4806)

 nss/Makefile                                  |   55 +-
 nss/nss_test_gai_hv2_canonname.c              |   56 +
 nss/tst-nss-gai-hv2-canonname.c               |   63 +
 nss/tst-nss-gai-hv2-canonname.h               |    1 +
 .../postclean.req                             |    0
 .../tst-nss-gai-hv2-canonname.script          |    2 +
 sysdeps/posix/getaddrinfo.c                   | 1574 +++++++++--------
 7 files changed, 976 insertions(+), 775 deletions(-)
 create mode 100644 nss/nss_test_gai_hv2_canonname.c
 create mode 100644 nss/tst-nss-gai-hv2-canonname.c
 create mode 100644 nss/tst-nss-gai-hv2-canonname.h
 create mode 100644 nss/tst-nss-gai-hv2-canonname.root/postclean.req
 create mode 100644 nss/tst-nss-gai-hv2-canonname.root/tst-nss-gai-hv2-canonname.script

-- 
2.41.0

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

* [pushed 2.34 01/14] nss: Sort tests and tests-container and put one test per line
  2023-09-16  0:48 [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
@ 2023-09-16  0:48 ` Siddhesh Poyarekar
  2023-09-16  0:48 ` [pushed 2.34 02/14] gethosts: Remove unused argument _type Siddhesh Poyarekar
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-16  0:48 UTC (permalink / raw)
  To: libc-stable

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
(cherry picked from commit e2f68b54e8052da14680074fc5df03153216f218)
---
 nss/Makefile | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/nss/Makefile b/nss/Makefile
index 2908c21be2..5192db6097 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -56,22 +56,31 @@ extra-objs		+= $(makedb-modules:=.o)
 
 tests-static            = tst-field
 tests-internal		= tst-field
-tests			= test-netdb test-digits-dots tst-nss-getpwent bug17079 \
-			  tst-nss-test1 \
-			  tst-nss-test2 \
-			  tst-nss-test4 \
-			  tst-nss-test5 \
-			  tst-nss-test_errno
-xtests			= bug-erange
-
-tests-container = \
-			  tst-nss-compat1 \
-			  tst-nss-test3 \
-			  tst-nss-files-hosts-long \
-			  tst-nss-db-endpwent \
-			  tst-nss-db-endgrent \
-			  tst-reload1 tst-reload2 \
-			  tst-nss-gai-actions
+
+tests := \
+  bug17079 \
+  test-digits-dots \
+  test-netdb \
+  tst-nss-getpwent \
+  tst-nss-test1 \
+  tst-nss-test2 \
+  tst-nss-test4 \
+  tst-nss-test5 \
+  tst-nss-test_errno \
+# tests
+
+xtests = bug-erange
+
+tests-container := \
+  tst-nss-compat1 \
+  tst-nss-db-endgrent \
+  tst-nss-db-endpwent \
+  tst-nss-files-hosts-long \
+  tst-nss-gai-actions \
+  tst-nss-test3 \
+  tst-reload1 \
+  tst-reload2 \
+# tests-container
 
 # Tests which need libdl
 ifeq (yes,$(build-shared))
-- 
2.41.0


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

* [pushed 2.34 02/14] gethosts: Remove unused argument _type
  2023-09-16  0:48 [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
  2023-09-16  0:48 ` [pushed 2.34 01/14] nss: Sort tests and tests-container and put one test per line Siddhesh Poyarekar
@ 2023-09-16  0:48 ` Siddhesh Poyarekar
  2023-09-16  0:48 ` [pushed 2.34 03/14] gaih_inet: Simplify canon name resolution Siddhesh Poyarekar
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-16  0:48 UTC (permalink / raw)
  To: libc-stable

The generated code is unchanged.

(cherry picked from commit b17e842a60819098d2a203ecc8b8371b7e1d6c65)
---
 sysdeps/posix/getaddrinfo.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index f391dc0a59..702d8a50e0 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -239,7 +239,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
   return true;
 }
 
-#define gethosts(_family, _type) \
+#define gethosts(_family) \
  {									      \
   struct hostent th;							      \
   char *localcanon = NULL;						      \
@@ -864,7 +864,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 		      if (req->ai_family == AF_INET6
 			  || req->ai_family == AF_UNSPEC)
 			{
-			  gethosts (AF_INET6, struct in6_addr);
+			  gethosts (AF_INET6);
 			  no_inet6_data = no_data;
 			  inet6_status = status;
 			}
@@ -876,7 +876,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 				 know we are not going to need them.  */
 			      && ((req->ai_flags & AI_ALL) || !got_ipv6)))
 			{
-			  gethosts (AF_INET, struct in_addr);
+			  gethosts (AF_INET);
 
 			  if (req->ai_family == AF_INET)
 			    {
-- 
2.41.0


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

* [pushed 2.34 03/14] gaih_inet: Simplify canon name resolution
  2023-09-16  0:48 [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
  2023-09-16  0:48 ` [pushed 2.34 01/14] nss: Sort tests and tests-container and put one test per line Siddhesh Poyarekar
  2023-09-16  0:48 ` [pushed 2.34 02/14] gethosts: Remove unused argument _type Siddhesh Poyarekar
@ 2023-09-16  0:48 ` Siddhesh Poyarekar
  2023-09-16  0:48 ` [pushed 2.34 04/14] getaddrinfo: Fix leak with AI_ALL [BZ #28852] Siddhesh Poyarekar
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-16  0:48 UTC (permalink / raw)
  To: libc-stable; +Cc: DJ Delorie

Simplify logic for allocation of canon to remove the canonbuf variable;
canon now always points to an allocated block.  Also pull the canon name
set into a separate function.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit d01411f6bc61429fc027c38827bf3103b48eef2e)
---
 sysdeps/posix/getaddrinfo.c | 130 +++++++++++++++++++++---------------
 1 file changed, 75 insertions(+), 55 deletions(-)

diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 702d8a50e0..5c0d873e1d 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -285,7 +285,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
 									      \
       if (localcanon != NULL && canon == NULL)				      \
 	{								      \
-	  canonbuf = __strdup (localcanon);				      \
+	  char *canonbuf = __strdup (localcanon);			      \
 	  if (canonbuf == NULL)						      \
 	    {								      \
 	      __resolv_context_put (res_ctx);				      \
@@ -323,6 +323,41 @@ getcanonname (nss_action_list nip, struct gaih_addrtuple *at, const char *name)
   return __strdup (name);
 }
 
+/* Process looked up canonical name and if necessary, decode to IDNA.  Result
+   is a new string written to CANONP and the earlier string is freed.  */
+
+static int
+process_canonname (const struct addrinfo *req, const char *orig_name,
+		   char **canonp)
+{
+  char *canon = *canonp;
+
+  if ((req->ai_flags & AI_CANONNAME) != 0)
+    {
+      bool do_idn = req->ai_flags & AI_CANONIDN;
+      if (do_idn)
+	{
+	  char *out;
+	  int rc = __idna_from_dns_encoding (canon ?: orig_name, &out);
+	  if (rc == 0)
+	    {
+	      free (canon);
+	      canon = out;
+	    }
+	  else if (rc == EAI_IDN_ENCODE)
+	    /* Use the punycode name as a fallback.  */
+	    do_idn = false;
+	  else
+	    return -rc;
+	}
+      if (!do_idn && canon == NULL && (canon = __strdup (orig_name)) == NULL)
+	return -EAI_MEMORY;
+    }
+
+  *canonp = canon;
+  return 0;
+}
+
 static int
 gaih_inet (const char *name, const struct gaih_service *service,
 	   const struct addrinfo *req, struct addrinfo **pai,
@@ -332,7 +367,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
   struct gaih_servtuple *st = (struct gaih_servtuple *) &nullserv;
   struct gaih_addrtuple *at = NULL;
   bool got_ipv6 = false;
-  const char *canon = NULL;
+  char *canon = NULL;
   const char *orig_name = name;
 
   /* Reserve stack memory for the scratch buffer in the getaddrinfo
@@ -453,7 +488,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
   bool malloc_name = false;
   struct gaih_addrtuple *addrmem = NULL;
-  char *canonbuf = NULL;
   int result = 0;
 
   if (name != NULL)
@@ -495,7 +529,15 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	    }
 
 	  if (req->ai_flags & AI_CANONNAME)
-	    canon = name;
+	    {
+	      char *canonbuf = __strdup (name);
+	      if (canonbuf == NULL)
+		{
+		  result = -EAI_MEMORY;
+		  goto free_and_return;
+		}
+	      canon = canonbuf;
+	    }
 
 	  goto process_list;
 	}
@@ -545,7 +587,15 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	    }
 
 	  if (req->ai_flags & AI_CANONNAME)
-	    canon = name;
+	    {
+	      char *canonbuf = __strdup (name);
+	      if (canonbuf == NULL)
+		{
+		  result = -EAI_MEMORY;
+		  goto free_and_return;
+		}
+	      canon = canonbuf;
+	    }
 
 	  goto process_list;
 	}
@@ -676,9 +726,9 @@ gaih_inet (const char *name, const struct gaih_service *service,
 		      (*pat)->next = NULL;
 		      if (added_canon || air->canon == NULL)
 			(*pat)->name = NULL;
-		      else if (canonbuf == NULL)
+		      else if (canon == NULL)
 			{
-			  canonbuf = __strdup (air->canon);
+			  char *canonbuf = __strdup (air->canon);
 			  if (canonbuf == NULL)
 			    {
 			      result = -EAI_MEMORY;
@@ -748,9 +798,9 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	      /* Always start afresh; continue should discard previous results
 		 and the hosts database does not support merge.  */
 	      at = NULL;
-	      free (canonbuf);
+	      free (canon);
 	      free (addrmem);
-	      canon = canonbuf = NULL;
+	      canon = NULL;
 	      addrmem = NULL;
 	      got_ipv6 = false;
 
@@ -805,7 +855,16 @@ gaih_inet (const char *name, const struct gaih_service *service,
 		      no_data = 1;
 
 		      if ((req->ai_flags & AI_CANONNAME) != 0 && canon == NULL)
-			canon = at->name;
+			{
+			  char *canonbuf = __strdup (at->name);
+			  if (canonbuf == NULL)
+			    {
+			      __resolv_context_put (res_ctx);
+			      result = -EAI_MEMORY;
+			      goto free_and_return;
+			    }
+			  canon = canonbuf;
+			}
 
 		      struct gaih_addrtuple **pat = &at;
 
@@ -893,7 +952,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 			  if ((req->ai_flags & AI_CANONNAME) != 0
 			      && canon == NULL)
 			    {
-			      canonbuf = getcanonname (nip, at, name);
+			      char *canonbuf = getcanonname (nip, at, name);
 			      if (canonbuf == NULL)
 				{
 				  __resolv_context_put (res_ctx);
@@ -1004,6 +1063,10 @@ gaih_inet (const char *name, const struct gaih_service *service,
     }
 
   {
+    /* Set up the canonical name if we need it.  */
+    if ((result = process_canonname (req, orig_name, &canon)) != 0)
+      goto free_and_return;
+
     struct gaih_servtuple *st2;
     struct gaih_addrtuple *at2 = at;
     size_t socklen;
@@ -1014,48 +1077,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
      */
     while (at2 != NULL)
       {
-	/* Only the first entry gets the canonical name.  */
-	if (at2 == at && (req->ai_flags & AI_CANONNAME) != 0)
-	  {
-	    if (canon == NULL)
-	      /* If the canonical name cannot be determined, use
-		 the passed in string.  */
-	      canon = orig_name;
-
-	    bool do_idn = req->ai_flags & AI_CANONIDN;
-	    if (do_idn)
-	      {
-		char *out;
-		int rc = __idna_from_dns_encoding (canon, &out);
-		if (rc == 0)
-		  canon = out;
-		else if (rc == EAI_IDN_ENCODE)
-		  /* Use the punycode name as a fallback.  */
-		  do_idn = false;
-		else
-		  {
-		    result = -rc;
-		    goto free_and_return;
-		  }
-	      }
-	    if (!do_idn)
-	      {
-		if (canonbuf != NULL)
-		  /* We already allocated the string using malloc, but
-		     the buffer is now owned by canon.  */
-		  canonbuf = NULL;
-		else
-		  {
-		    canon = __strdup (canon);
-		    if (canon == NULL)
-		      {
-			result = -EAI_MEMORY;
-			goto free_and_return;
-		      }
-		  }
-	      }
-	  }
-
 	family = at2->family;
 	if (family == AF_INET6)
 	  {
@@ -1078,7 +1099,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	    ai = *pai = malloc (sizeof (struct addrinfo) + socklen);
 	    if (ai == NULL)
 	      {
-		free ((char *) canon);
 		result = -EAI_MEMORY;
 		goto free_and_return;
 	      }
@@ -1138,7 +1158,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
   if (malloc_name)
     free ((char *) name);
   free (addrmem);
-  free (canonbuf);
+  free (canon);
 
   return result;
 }
-- 
2.41.0


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

* [pushed 2.34 04/14] getaddrinfo: Fix leak with AI_ALL [BZ #28852]
  2023-09-16  0:48 [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
                   ` (2 preceding siblings ...)
  2023-09-16  0:48 ` [pushed 2.34 03/14] gaih_inet: Simplify canon name resolution Siddhesh Poyarekar
@ 2023-09-16  0:48 ` Siddhesh Poyarekar
  2023-09-16  0:48 ` [pushed 2.34 05/14] gaih_inet: Simplify service resolution Siddhesh Poyarekar
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-16  0:48 UTC (permalink / raw)
  To: libc-stable; +Cc: DJ Delorie

Use realloc in convert_hostent_to_gaih_addrtuple and fix up pointers in
the result list so that a single block is maintained for
hostbyname3_r/hostbyname2_r and freed in gaih_inet.  This result is
never merged with any other results, since the hosts database does not
permit merging.

Resolves BZ #28852.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit 300460460706ce3ffe29a7df8966e68323ec5bf1)
---
 sysdeps/posix/getaddrinfo.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 5c0d873e1d..ed70e6cb39 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -189,19 +189,16 @@ gaih_inet_serv (const char *servicename, const struct gaih_typeproto *tp,
   return 0;
 }
 
-/* Convert struct hostent to a list of struct gaih_addrtuple objects.
-   h_name is not copied, and the struct hostent object must not be
-   deallocated prematurely.  *RESULT must be NULL or a pointer to a
-   linked-list.  The new addresses are appended at the end.  */
+/* Convert struct hostent to a list of struct gaih_addrtuple objects.  h_name
+   is not copied, and the struct hostent object must not be deallocated
+   prematurely.  The new addresses are appended to the tuple array in
+   RESULT.  */
 static bool
 convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
 				   int family,
 				   struct hostent *h,
 				   struct gaih_addrtuple **result)
 {
-  while (*result)
-    result = &(*result)->next;
-
   /* Count the number of addresses in h->h_addr_list.  */
   size_t count = 0;
   for (char **p = h->h_addr_list; *p != NULL; ++p)
@@ -212,10 +209,30 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
   if (count == 0 || h->h_length > sizeof (((struct gaih_addrtuple) {}).addr))
     return true;
 
-  struct gaih_addrtuple *array = calloc (count, sizeof (*array));
+  struct gaih_addrtuple *array = *result;
+  size_t old = 0;
+
+  while (array != NULL)
+    {
+      old++;
+      array = array->next;
+    }
+
+  array = realloc (*result, (old + count) * sizeof (*array));
+
   if (array == NULL)
     return false;
 
+  *result = array;
+
+  /* Update the next pointers on reallocation.  */
+  for (size_t i = 0; i < old; i++)
+    array[i].next = array + i + 1;
+
+  array += old;
+
+  memset (array, 0, count * sizeof (*array));
+
   for (size_t i = 0; i < count; ++i)
     {
       if (family == AF_INET && req->ai_family == AF_INET6)
@@ -235,7 +252,6 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
   array[0].name = h->h_name;
   array[count - 1].next = NULL;
 
-  *result = array;
   return true;
 }
 
-- 
2.41.0


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

* [pushed 2.34 05/14] gaih_inet: Simplify service resolution
  2023-09-16  0:48 [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
                   ` (3 preceding siblings ...)
  2023-09-16  0:48 ` [pushed 2.34 04/14] getaddrinfo: Fix leak with AI_ALL [BZ #28852] Siddhesh Poyarekar
@ 2023-09-16  0:48 ` Siddhesh Poyarekar
  2023-09-16  0:48 ` [pushed 2.34 06/14] gaih_inet: make numeric lookup a separate routine Siddhesh Poyarekar
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-16  0:48 UTC (permalink / raw)
  To: libc-stable; +Cc: DJ Delorie

Refactor the code to split out the service resolution code into a
separate function.  Allocate the service tuples array just once to the
size of the typeproto array, thus avoiding the unnecessary pointer
chasing and stack allocations.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit 8d6cf99f2fb81a097f9334c125e5c23604af1a98)
---
 sysdeps/posix/getaddrinfo.c | 178 ++++++++++++++++--------------------
 1 file changed, 78 insertions(+), 100 deletions(-)

diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index ed70e6cb39..8c78ef9570 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -100,14 +100,12 @@ struct gaih_service
 
 struct gaih_servtuple
   {
-    struct gaih_servtuple *next;
     int socktype;
     int protocol;
     int port;
+    bool set;
   };
 
-static const struct gaih_servtuple nullserv;
-
 
 struct gaih_typeproto
   {
@@ -180,11 +178,11 @@ gaih_inet_serv (const char *servicename, const struct gaih_typeproto *tp,
     }
   while (r);
 
-  st->next = NULL;
   st->socktype = tp->socktype;
   st->protocol = ((tp->protoflag & GAI_PROTO_PROTOANY)
 		  ? req->ai_protocol : tp->protocol);
   st->port = s->s_port;
+  st->set = true;
 
   return 0;
 }
@@ -375,20 +373,11 @@ process_canonname (const struct addrinfo *req, const char *orig_name,
 }
 
 static int
-gaih_inet (const char *name, const struct gaih_service *service,
-	   const struct addrinfo *req, struct addrinfo **pai,
-	   unsigned int *naddrs, struct scratch_buffer *tmpbuf)
+get_servtuples (const struct gaih_service *service, const struct addrinfo *req,
+		struct gaih_servtuple *st, struct scratch_buffer *tmpbuf)
 {
+  int i;
   const struct gaih_typeproto *tp = gaih_inet_typeproto;
-  struct gaih_servtuple *st = (struct gaih_servtuple *) &nullserv;
-  struct gaih_addrtuple *at = NULL;
-  bool got_ipv6 = false;
-  char *canon = NULL;
-  const char *orig_name = name;
-
-  /* Reserve stack memory for the scratch buffer in the getaddrinfo
-     function.  */
-  size_t alloca_used = sizeof (struct scratch_buffer);
 
   if (req->ai_protocol || req->ai_socktype)
     {
@@ -410,98 +399,88 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	}
     }
 
-  int port = 0;
-  if (service != NULL)
+  if (service != NULL && (tp->protoflag & GAI_PROTO_NOSERVICE) != 0)
+    return -EAI_SERVICE;
+
+  if (service == NULL || service->num >= 0)
     {
-      if ((tp->protoflag & GAI_PROTO_NOSERVICE) != 0)
-	return -EAI_SERVICE;
+      int port = service != NULL ? htons (service->num) : 0;
 
-      if (service->num < 0)
+      if (req->ai_socktype || req->ai_protocol)
 	{
-	  if (tp->name[0])
-	    {
-	      st = (struct gaih_servtuple *)
-		alloca_account (sizeof (struct gaih_servtuple), alloca_used);
-
-	      int rc = gaih_inet_serv (service->name, tp, req, st, tmpbuf);
-	      if (__glibc_unlikely (rc != 0))
-		return rc;
-	    }
-	  else
-	    {
-	      struct gaih_servtuple **pst = &st;
-	      for (tp++; tp->name[0]; tp++)
-		{
-		  struct gaih_servtuple *newp;
+	  st[0].socktype = tp->socktype;
+	  st[0].protocol = ((tp->protoflag & GAI_PROTO_PROTOANY)
+			  ? req->ai_protocol : tp->protocol);
+	  st[0].port = port;
+	  st[0].set = true;
 
-		  if ((tp->protoflag & GAI_PROTO_NOSERVICE) != 0)
-		    continue;
+	  return 0;
+	}
 
-		  if (req->ai_socktype != 0
-		      && req->ai_socktype != tp->socktype)
-		    continue;
-		  if (req->ai_protocol != 0
-		      && !(tp->protoflag & GAI_PROTO_PROTOANY)
-		      && req->ai_protocol != tp->protocol)
-		    continue;
+      /* Neither socket type nor protocol is set.  Return all socket types
+	 we know about.  */
+      for (i = 0, ++tp; tp->name[0]; ++tp)
+	if (tp->defaultflag)
+	  {
+	    st[i].socktype = tp->socktype;
+	    st[i].protocol = tp->protocol;
+	    st[i].port = port;
+	    st[i++].set = true;
+	  }
 
-		  newp = (struct gaih_servtuple *)
-		    alloca_account (sizeof (struct gaih_servtuple),
-				    alloca_used);
+      return 0;
+    }
 
-		  if (gaih_inet_serv (service->name,
-				      tp, req, newp, tmpbuf) != 0)
-		    continue;
+  if (tp->name[0])
+    return gaih_inet_serv (service->name, tp, req, st, tmpbuf);
 
-		  *pst = newp;
-		  pst = &(newp->next);
-		}
-	      if (st == (struct gaih_servtuple *) &nullserv)
-		return -EAI_SERVICE;
-	    }
-	}
-      else
-	{
-	  port = htons (service->num);
-	  goto got_port;
-	}
-    }
-  else
+  for (i = 0, tp++; tp->name[0]; tp++)
     {
-    got_port:
+      if ((tp->protoflag & GAI_PROTO_NOSERVICE) != 0)
+	continue;
 
-      if (req->ai_socktype || req->ai_protocol)
-	{
-	  st = alloca_account (sizeof (struct gaih_servtuple), alloca_used);
-	  st->next = NULL;
-	  st->socktype = tp->socktype;
-	  st->protocol = ((tp->protoflag & GAI_PROTO_PROTOANY)
-			  ? req->ai_protocol : tp->protocol);
-	  st->port = port;
-	}
-      else
-	{
-	  /* Neither socket type nor protocol is set.  Return all socket types
-	     we know about.  */
-	  struct gaih_servtuple **lastp = &st;
-	  for (++tp; tp->name[0]; ++tp)
-	    if (tp->defaultflag)
-	      {
-		struct gaih_servtuple *newp;
+      if (req->ai_socktype != 0
+	  && req->ai_socktype != tp->socktype)
+	continue;
+      if (req->ai_protocol != 0
+	  && !(tp->protoflag & GAI_PROTO_PROTOANY)
+	  && req->ai_protocol != tp->protocol)
+	continue;
 
-		newp = alloca_account (sizeof (struct gaih_servtuple),
-				       alloca_used);
-		newp->next = NULL;
-		newp->socktype = tp->socktype;
-		newp->protocol = tp->protocol;
-		newp->port = port;
+      if (gaih_inet_serv (service->name,
+			  tp, req, &st[i], tmpbuf) != 0)
+	continue;
 
-		*lastp = newp;
-		lastp = &newp->next;
-	      }
-	}
+      i++;
     }
 
+  if (!st[0].set)
+    return -EAI_SERVICE;
+
+  return 0;
+}
+
+static int
+gaih_inet (const char *name, const struct gaih_service *service,
+	   const struct addrinfo *req, struct addrinfo **pai,
+	   unsigned int *naddrs, struct scratch_buffer *tmpbuf)
+{
+  struct gaih_servtuple st[sizeof (gaih_inet_typeproto)
+			   / sizeof (struct gaih_typeproto)] = {0};
+
+  struct gaih_addrtuple *at = NULL;
+  bool got_ipv6 = false;
+  char *canon = NULL;
+  const char *orig_name = name;
+
+  /* Reserve stack memory for the scratch buffer in the getaddrinfo
+     function.  */
+  size_t alloca_used = sizeof (struct scratch_buffer);
+
+  int rc;
+  if ((rc = get_servtuples (service, req, st, tmpbuf)) != 0)
+    return rc;
+
   bool malloc_name = false;
   struct gaih_addrtuple *addrmem = NULL;
   int result = 0;
@@ -1083,7 +1062,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
     if ((result = process_canonname (req, orig_name, &canon)) != 0)
       goto free_and_return;
 
-    struct gaih_servtuple *st2;
     struct gaih_addrtuple *at2 = at;
     size_t socklen;
     sa_family_t family;
@@ -1109,7 +1087,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	else
 	  socklen = sizeof (struct sockaddr_in);
 
-	for (st2 = st; st2 != NULL; st2 = st2->next)
+	for (int i = 0; st[i].set; i++)
 	  {
 	    struct addrinfo *ai;
 	    ai = *pai = malloc (sizeof (struct addrinfo) + socklen);
@@ -1121,8 +1099,8 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
 	    ai->ai_flags = req->ai_flags;
 	    ai->ai_family = family;
-	    ai->ai_socktype = st2->socktype;
-	    ai->ai_protocol = st2->protocol;
+	    ai->ai_socktype = st[i].socktype;
+	    ai->ai_protocol = st[i].protocol;
 	    ai->ai_addrlen = socklen;
 	    ai->ai_addr = (void *) (ai + 1);
 
@@ -1144,7 +1122,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 		struct sockaddr_in6 *sin6p =
 		  (struct sockaddr_in6 *) ai->ai_addr;
 
-		sin6p->sin6_port = st2->port;
+		sin6p->sin6_port = st[i].port;
 		sin6p->sin6_flowinfo = 0;
 		memcpy (&sin6p->sin6_addr,
 			at2->addr, sizeof (struct in6_addr));
@@ -1154,7 +1132,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	      {
 		struct sockaddr_in *sinp =
 		  (struct sockaddr_in *) ai->ai_addr;
-		sinp->sin_port = st2->port;
+		sinp->sin_port = st[i].port;
 		memcpy (&sinp->sin_addr,
 			at2->addr, sizeof (struct in_addr));
 		memset (sinp->sin_zero, '\0', sizeof (sinp->sin_zero));
-- 
2.41.0


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

* [pushed 2.34 06/14] gaih_inet: make numeric lookup a separate routine
  2023-09-16  0:48 [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
                   ` (4 preceding siblings ...)
  2023-09-16  0:48 ` [pushed 2.34 05/14] gaih_inet: Simplify service resolution Siddhesh Poyarekar
@ 2023-09-16  0:48 ` Siddhesh Poyarekar
  2023-09-16  0:48 ` [pushed 2.34 07/14] gaih_inet: Split simple gethostbyname into its own function Siddhesh Poyarekar
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-16  0:48 UTC (permalink / raw)
  To: libc-stable; +Cc: DJ Delorie

Introduce the gaih_result structure and general paradigm for cleanups
that follow to process the lookup request and return a result.  A lookup
function (like text_to_binary_address), should return an integer error
code and set members of gaih_result based on what it finds.  If the
function does not have a result and no errors have occurred during the
lookup, it should return 0 and res.at should be set to NULL, allowing a
subsequent function to do the lookup until we run out of options.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit 26dea461191cca519b498890a9682fe4bc8e4c2f)
---
 sysdeps/posix/getaddrinfo.c | 891 ++++++++++++++++++------------------
 1 file changed, 452 insertions(+), 439 deletions(-)

diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 8c78ef9570..57b6834c8b 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -116,6 +116,12 @@ struct gaih_typeproto
     char name[8];
   };
 
+struct gaih_result
+{
+  struct gaih_addrtuple *at;
+  char *canon;
+};
+
 /* Values for `protoflag'.  */
 #define GAI_PROTO_NOSERVICE	1
 #define GAI_PROTO_PROTOANY	2
@@ -297,7 +303,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
 	}								      \
       *pat = addrmem;							      \
 									      \
-      if (localcanon != NULL && canon == NULL)				      \
+      if (localcanon != NULL && res.canon == NULL)			      \
 	{								      \
 	  char *canonbuf = __strdup (localcanon);			      \
 	  if (canonbuf == NULL)						      \
@@ -306,7 +312,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
 	      result = -EAI_SYSTEM;					      \
 	      goto free_and_return;					      \
 	    }								      \
-	  canon = canonbuf;						      \
+	  res.canon = canonbuf;						      \
 	}								      \
       if (_family == AF_INET6 && *pat != NULL)				      \
 	got_ipv6 = true;						      \
@@ -342,9 +348,9 @@ getcanonname (nss_action_list nip, struct gaih_addrtuple *at, const char *name)
 
 static int
 process_canonname (const struct addrinfo *req, const char *orig_name,
-		   char **canonp)
+		   struct gaih_result *res)
 {
-  char *canon = *canonp;
+  char *canon = res->canon;
 
   if ((req->ai_flags & AI_CANONNAME) != 0)
     {
@@ -368,7 +374,7 @@ process_canonname (const struct addrinfo *req, const char *orig_name,
 	return -EAI_MEMORY;
     }
 
-  *canonp = canon;
+  res->canon = canon;
   return 0;
 }
 
@@ -460,6 +466,105 @@ get_servtuples (const struct gaih_service *service, const struct addrinfo *req,
   return 0;
 }
 
+/* Convert numeric addresses to binary into RES.  On failure, RES->AT is set to
+   NULL and an error code is returned.  If AI_NUMERIC_HOST is not requested and
+   the function cannot determine a result, RES->AT is set to NULL and 0
+   returned.  */
+
+static int
+text_to_binary_address (const char *name, const struct addrinfo *req,
+			struct gaih_result *res)
+{
+  struct gaih_addrtuple *at = res->at;
+  int result = 0;
+
+  assert (at != NULL);
+
+  memset (at->addr, 0, sizeof (at->addr));
+  if (__inet_aton_exact (name, (struct in_addr *) at->addr) != 0)
+    {
+      if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
+	at->family = AF_INET;
+      else if (req->ai_family == AF_INET6 && (req->ai_flags & AI_V4MAPPED))
+	{
+	  at->addr[3] = at->addr[0];
+	  at->addr[2] = htonl (0xffff);
+	  at->addr[1] = 0;
+	  at->addr[0] = 0;
+	  at->family = AF_INET6;
+	}
+      else
+	{
+	  result = -EAI_ADDRFAMILY;
+	  goto out;
+	}
+
+      if (req->ai_flags & AI_CANONNAME)
+	{
+	  char *canonbuf = __strdup (name);
+	  if (canonbuf == NULL)
+	    {
+	      result = -EAI_MEMORY;
+	      goto out;
+	    }
+	  res->canon = canonbuf;
+	}
+      return 0;
+    }
+
+  char *scope_delim = strchr (name, SCOPE_DELIMITER);
+  int e;
+
+  if (scope_delim == NULL)
+    e = inet_pton (AF_INET6, name, at->addr);
+  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 out;
+	}
+
+      if (scope_delim != NULL
+	  && __inet6_scopeid_pton ((struct in6_addr *) at->addr,
+				   scope_delim + 1, &at->scopeid) != 0)
+	{
+	  result = -EAI_NONAME;
+	  goto out;
+	}
+
+      if (req->ai_flags & AI_CANONNAME)
+	{
+	  char *canonbuf = __strdup (name);
+	  if (canonbuf == NULL)
+	    {
+	      result = -EAI_MEMORY;
+	      goto out;
+	    }
+	  res->canon = canonbuf;
+	}
+      return 0;
+    }
+
+  if ((req->ai_flags & AI_NUMERICHOST))
+    result = -EAI_NONAME;
+
+out:
+  res->at = NULL;
+  return result;
+}
+
 static int
 gaih_inet (const char *name, const struct gaih_service *service,
 	   const struct addrinfo *req, struct addrinfo **pai,
@@ -468,9 +573,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
   struct gaih_servtuple st[sizeof (gaih_inet_typeproto)
 			   / sizeof (struct gaih_typeproto)] = {0};
 
-  struct gaih_addrtuple *at = NULL;
   bool got_ipv6 = false;
-  char *canon = NULL;
   const char *orig_name = name;
 
   /* Reserve stack memory for the scratch buffer in the getaddrinfo
@@ -485,6 +588,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
   struct gaih_addrtuple *addrmem = NULL;
   int result = 0;
 
+  struct gaih_result res = {0};
   if (name != NULL)
     {
       if (req->ai_flags & AI_IDN)
@@ -497,533 +601,441 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	  malloc_name = true;
 	}
 
-      uint32_t addr[4];
-      if (__inet_aton_exact (name, (struct in_addr *) addr) != 0)
+      res.at = alloca_account (sizeof (struct gaih_addrtuple), alloca_used);
+      res.at->scopeid = 0;
+      res.at->next = NULL;
+
+      if ((result = text_to_binary_address (name, req, &res)) != 0)
+	goto free_and_return;
+      else if (res.at != NULL)
+	goto process_list;
+
+      int no_data = 0;
+      int no_inet6_data = 0;
+      nss_action_list nip;
+      enum nss_status inet6_status = NSS_STATUS_UNAVAIL;
+      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
+	 IPv6 scope ids, nor retrieving the canonical name.  */
+      if (req->ai_family == AF_INET
+	  && (req->ai_flags & AI_CANONNAME) == 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)
-	    {
-	      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] = addr[0];
-	      at->addr[2] = htonl (0xffff);
-	      at->addr[1] = 0;
-	      at->addr[0] = 0;
-	      at->family = AF_INET6;
-	    }
-	  else
-	    {
-	      result = -EAI_ADDRFAMILY;
-	      goto free_and_return;
-	    }
+	  int rc;
+	  struct hostent th;
+	  struct hostent *h;
 
-	  if (req->ai_flags & AI_CANONNAME)
+	  while (1)
 	    {
-	      char *canonbuf = __strdup (name);
-	      if (canonbuf == NULL)
+	      rc = __gethostbyname2_r (name, AF_INET, &th,
+				       tmpbuf->data, tmpbuf->length,
+				       &h, &h_errno);
+	      if (rc != ERANGE || h_errno != NETDB_INTERNAL)
+		break;
+	      if (!scratch_buffer_grow (tmpbuf))
 		{
 		  result = -EAI_MEMORY;
 		  goto free_and_return;
 		}
-	      canon = canonbuf;
 	    }
 
-	  goto process_list;
-	}
-
-      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)
-	{
-	  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))
+	  if (rc == 0)
 	    {
-	      at->addr[0] = addr[3];
-	      at->addr[1] = addr[1];
-	      at->addr[2] = addr[2];
-	      at->addr[3] = addr[3];
-	      at->family = AF_INET;
+	      if (h != NULL)
+		{
+		  /* We found data, convert it.  */
+		  if (!convert_hostent_to_gaih_addrtuple
+		      (req, AF_INET, h, &addrmem))
+		    {
+		      result = -EAI_MEMORY;
+		      goto free_and_return;
+		    }
+		  res.at = addrmem;
+		}
+	      else
+		{
+		  if (h_errno == NO_DATA)
+		    result = -EAI_NODATA;
+		  else
+		    result = -EAI_NONAME;
+		  goto free_and_return;
+		}
 	    }
 	  else
 	    {
-	      result = -EAI_ADDRFAMILY;
-	      goto free_and_return;
-	    }
+	      if (h_errno == NETDB_INTERNAL)
+		result = -EAI_SYSTEM;
+	      else if (h_errno == TRY_AGAIN)
+		result = -EAI_AGAIN;
+	      else
+		/* We made requests but they turned out no data.
+		   The name is known, though.  */
+		result = -EAI_NODATA;
 
-	  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)
+	  goto process_list;
+	}
+
+#ifdef USE_NSCD
+      if (__nss_not_use_nscd_hosts > 0
+	  && ++__nss_not_use_nscd_hosts > NSS_NSCD_RETRY)
+	__nss_not_use_nscd_hosts = 0;
+
+      if (!__nss_not_use_nscd_hosts
+	  && !__nss_database_custom[NSS_DBSIDX_hosts])
+	{
+	  /* Try to use nscd.  */
+	  struct nscd_ai_result *air = NULL;
+	  int err = __nscd_getai (name, &air, &h_errno);
+	  if (air != NULL)
 	    {
-	      char *canonbuf = __strdup (name);
-	      if (canonbuf == NULL)
+	      /* Transform into gaih_addrtuple list.  */
+	      bool added_canon = (req->ai_flags & AI_CANONNAME) == 0;
+	      char *addrs = air->addrs;
+
+	      addrmem = calloc (air->naddrs, sizeof (*addrmem));
+	      if (addrmem == NULL)
 		{
 		  result = -EAI_MEMORY;
 		  goto free_and_return;
 		}
-	      canon = canonbuf;
-	    }
 
-	  goto process_list;
-	}
-
-      if ((req->ai_flags & AI_NUMERICHOST) == 0)
-	{
-	  int no_data = 0;
-	  int no_inet6_data = 0;
-	  nss_action_list nip;
-	  enum nss_status inet6_status = NSS_STATUS_UNAVAIL;
-	  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
-	     IPv6 scope ids, nor retrieving the canonical name.  */
-	  if (req->ai_family == AF_INET
-	      && (req->ai_flags & AI_CANONNAME) == 0)
-	    {
-	      int rc;
-	      struct hostent th;
-	      struct hostent *h;
+	      struct gaih_addrtuple *addrfree = addrmem;
+	      struct gaih_addrtuple **pat = &res.at;
 
-	      while (1)
+	      for (int i = 0; i < air->naddrs; ++i)
 		{
-		  rc = __gethostbyname2_r (name, AF_INET, &th,
-					   tmpbuf->data, tmpbuf->length,
-					   &h, &h_errno);
-		  if (rc != ERANGE || h_errno != NETDB_INTERNAL)
-		    break;
-		  if (!scratch_buffer_grow (tmpbuf))
+		  socklen_t size = (air->family[i] == AF_INET
+				    ? INADDRSZ : IN6ADDRSZ);
+
+		  if (!((air->family[i] == AF_INET
+			 && req->ai_family == AF_INET6
+			 && (req->ai_flags & AI_V4MAPPED) != 0)
+			|| req->ai_family == AF_UNSPEC
+			|| air->family[i] == req->ai_family))
 		    {
-		      result = -EAI_MEMORY;
-		      goto free_and_return;
+		      /* Skip over non-matching result.  */
+		      addrs += size;
+		      continue;
 		    }
-		}
 
-	      if (rc == 0)
-		{
-		  if (h != NULL)
+		  if (*pat == NULL)
+		    {
+		      *pat = addrfree++;
+		      (*pat)->scopeid = 0;
+		    }
+		  uint32_t *pataddr = (*pat)->addr;
+		  (*pat)->next = NULL;
+		  if (added_canon || air->canon == NULL)
+		    (*pat)->name = NULL;
+		  else if (res.canon == NULL)
 		    {
-		      /* We found data, convert it.  */
-		      if (!convert_hostent_to_gaih_addrtuple
-			  (req, AF_INET, h, &addrmem))
+		      char *canonbuf = __strdup (air->canon);
+		      if (canonbuf == NULL)
 			{
 			  result = -EAI_MEMORY;
 			  goto free_and_return;
 			}
-		      at = addrmem;
+		      res.canon = (*pat)->name = canonbuf;
 		    }
-		  else
+
+		  if (air->family[i] == AF_INET
+		      && req->ai_family == AF_INET6
+		      && (req->ai_flags & AI_V4MAPPED))
 		    {
-		      if (h_errno == NO_DATA)
-			result = -EAI_NODATA;
-		      else
-			result = -EAI_NONAME;
-		      goto free_and_return;
+		      (*pat)->family = AF_INET6;
+		      pataddr[3] = *(uint32_t *) addrs;
+		      pataddr[2] = htonl (0xffff);
+		      pataddr[1] = 0;
+		      pataddr[0] = 0;
+		      pat = &((*pat)->next);
+		      added_canon = true;
+		    }
+		  else if (req->ai_family == AF_UNSPEC
+			   || air->family[i] == req->ai_family)
+		    {
+		      (*pat)->family = air->family[i];
+		      memcpy (pataddr, addrs, size);
+		      pat = &((*pat)->next);
+		      added_canon = true;
+		      if (air->family[i] == AF_INET6)
+			got_ipv6 = true;
 		    }
+		  addrs += size;
 		}
-	      else
-		{
-		  if (h_errno == NETDB_INTERNAL)
-		    result = -EAI_SYSTEM;
-		  else if (h_errno == TRY_AGAIN)
-		    result = -EAI_AGAIN;
-		  else
-		    /* We made requests but they turned out no data.
-		       The name is known, though.  */
-		    result = -EAI_NODATA;
 
-		  goto free_and_return;
-		}
+	      free (air);
 
 	      goto process_list;
 	    }
+	  else if (err == 0)
+	    /* The database contains a negative entry.  */
+	    goto free_and_return;
+	  else if (__nss_not_use_nscd_hosts == 0)
+	    {
+	      if (h_errno == NETDB_INTERNAL && errno == ENOMEM)
+		result = -EAI_MEMORY;
+	      else if (h_errno == TRY_AGAIN)
+		result = -EAI_AGAIN;
+	      else
+		result = -EAI_SYSTEM;
 
-#ifdef USE_NSCD
-	  if (__nss_not_use_nscd_hosts > 0
-	      && ++__nss_not_use_nscd_hosts > NSS_NSCD_RETRY)
-	    __nss_not_use_nscd_hosts = 0;
+	      goto free_and_return;
+	    }
+	}
+#endif
+
+      no_more = !__nss_database_get (nss_database_hosts, &nip);
 
-	  if (!__nss_not_use_nscd_hosts
-	      && !__nss_database_custom[NSS_DBSIDX_hosts])
+      /* If we are looking for both IPv4 and IPv6 address we don't
+	 want the lookup functions to automatically promote IPv4
+	 addresses to IPv6 addresses, so we use the no_inet6
+	 function variant.  */
+      res_ctx = __resolv_context_get ();
+      if (res_ctx == NULL)
+	no_more = 1;
+
+      while (!no_more)
+	{
+	  /* Always start afresh; continue should discard previous results
+	     and the hosts database does not support merge.  */
+	  res.at = NULL;
+	  free (res.canon);
+	  free (addrmem);
+	  res.canon = NULL;
+	  addrmem = NULL;
+	  got_ipv6 = false;
+
+	  if (do_merge)
 	    {
-	      /* Try to use nscd.  */
-	      struct nscd_ai_result *air = NULL;
-	      int err = __nscd_getai (name, &air, &h_errno);
-	      if (air != NULL)
+	      __set_h_errno (NETDB_INTERNAL);
+	      __set_errno (EBUSY);
+	      break;
+	    }
+
+	  no_data = 0;
+	  nss_gethostbyname4_r *fct4 = NULL;
+
+	  /* gethostbyname4_r sends out parallel A and AAAA queries and
+	     is thus only suitable for PF_UNSPEC.  */
+	  if (req->ai_family == PF_UNSPEC)
+	    fct4 = __nss_lookup_function (nip, "gethostbyname4_r");
+
+	  if (fct4 != NULL)
+	    {
+	      while (1)
 		{
-		  /* Transform into gaih_addrtuple list.  */
-		  bool added_canon = (req->ai_flags & AI_CANONNAME) == 0;
-		  char *addrs = air->addrs;
+		  status = DL_CALL_FCT (fct4, (name, &res.at,
+					       tmpbuf->data, tmpbuf->length,
+					       &errno, &h_errno,
+					       NULL));
+		  if (status == NSS_STATUS_SUCCESS)
+		    break;
+		  /* gethostbyname4_r may write into AT, so reset it.  */
+		  res.at = NULL;
+		  if (status != NSS_STATUS_TRYAGAIN
+		      || errno != ERANGE || h_errno != NETDB_INTERNAL)
+		    {
+		      if (h_errno == TRY_AGAIN)
+			no_data = EAI_AGAIN;
+		      else
+			no_data = h_errno == NO_DATA;
+		      break;
+		    }
 
-		  addrmem = calloc (air->naddrs, sizeof (*addrmem));
-		  if (addrmem == NULL)
+		  if (!scratch_buffer_grow (tmpbuf))
 		    {
+		      __resolv_context_put (res_ctx);
 		      result = -EAI_MEMORY;
 		      goto free_and_return;
 		    }
+		}
 
-		  struct gaih_addrtuple *addrfree = addrmem;
-		  struct gaih_addrtuple **pat = &at;
+	      if (status == NSS_STATUS_SUCCESS)
+		{
+		  assert (!no_data);
+		  no_data = 1;
 
-		  for (int i = 0; i < air->naddrs; ++i)
+		  if ((req->ai_flags & AI_CANONNAME) != 0 && res.canon == NULL)
 		    {
-		      socklen_t size = (air->family[i] == AF_INET
-					? INADDRSZ : IN6ADDRSZ);
-
-		      if (!((air->family[i] == AF_INET
-			     && req->ai_family == AF_INET6
-			     && (req->ai_flags & AI_V4MAPPED) != 0)
-			    || req->ai_family == AF_UNSPEC
-			    || air->family[i] == req->ai_family))
+		      char *canonbuf = __strdup (res.at->name);
+		      if (canonbuf == NULL)
 			{
-			  /* Skip over non-matching result.  */
-			  addrs += size;
-			  continue;
+			  __resolv_context_put (res_ctx);
+			  result = -EAI_MEMORY;
+			  goto free_and_return;
 			}
+		      res.canon = canonbuf;
+		    }
 
-		      if (*pat == NULL)
-			{
-			  *pat = addrfree++;
-			  (*pat)->scopeid = 0;
-			}
-		      uint32_t *pataddr = (*pat)->addr;
-		      (*pat)->next = NULL;
-		      if (added_canon || air->canon == NULL)
-			(*pat)->name = NULL;
-		      else if (canon == NULL)
-			{
-			  char *canonbuf = __strdup (air->canon);
-			  if (canonbuf == NULL)
-			    {
-			      result = -EAI_MEMORY;
-			      goto free_and_return;
-			    }
-			  canon = (*pat)->name = canonbuf;
-			}
+		  struct gaih_addrtuple **pat = &res.at;
 
-		      if (air->family[i] == AF_INET
+		  while (*pat != NULL)
+		    {
+		      if ((*pat)->family == AF_INET
 			  && req->ai_family == AF_INET6
-			  && (req->ai_flags & AI_V4MAPPED))
+			  && (req->ai_flags & AI_V4MAPPED) != 0)
 			{
+			  uint32_t *pataddr = (*pat)->addr;
 			  (*pat)->family = AF_INET6;
-			  pataddr[3] = *(uint32_t *) addrs;
+			  pataddr[3] = pataddr[0];
 			  pataddr[2] = htonl (0xffff);
 			  pataddr[1] = 0;
 			  pataddr[0] = 0;
 			  pat = &((*pat)->next);
-			  added_canon = true;
+			  no_data = 0;
 			}
 		      else if (req->ai_family == AF_UNSPEC
-			       || air->family[i] == req->ai_family)
+			       || (*pat)->family == req->ai_family)
 			{
-			  (*pat)->family = air->family[i];
-			  memcpy (pataddr, addrs, size);
 			  pat = &((*pat)->next);
-			  added_canon = true;
-			  if (air->family[i] == AF_INET6)
+
+			  no_data = 0;
+			  if (req->ai_family == AF_INET6)
 			    got_ipv6 = true;
 			}
-		      addrs += size;
+		      else
+			*pat = ((*pat)->next);
 		    }
-
-		  free (air);
-
-		  goto process_list;
 		}
-	      else if (err == 0)
-		/* The database contains a negative entry.  */
-		goto free_and_return;
-	      else if (__nss_not_use_nscd_hosts == 0)
-		{
-		  if (h_errno == NETDB_INTERNAL && errno == ENOMEM)
-		    result = -EAI_MEMORY;
-		  else if (h_errno == TRY_AGAIN)
-		    result = -EAI_AGAIN;
-		  else
-		    result = -EAI_SYSTEM;
 
-		  goto free_and_return;
-		}
+	      no_inet6_data = no_data;
 	    }
-#endif
-
-	  no_more = !__nss_database_get (nss_database_hosts, &nip);
-
-	  /* If we are looking for both IPv4 and IPv6 address we don't
-	     want the lookup functions to automatically promote IPv4
-	     addresses to IPv6 addresses, so we use the no_inet6
-	     function variant.  */
-	  res_ctx = __resolv_context_get ();
-	  if (res_ctx == NULL)
-	    no_more = 1;
-
-	  while (!no_more)
+	  else
 	    {
-	      /* Always start afresh; continue should discard previous results
-		 and the hosts database does not support merge.  */
-	      at = NULL;
-	      free (canon);
-	      free (addrmem);
-	      canon = NULL;
-	      addrmem = NULL;
-	      got_ipv6 = false;
-
-	      if (do_merge)
+	      nss_gethostbyname3_r *fct = NULL;
+	      if (req->ai_flags & AI_CANONNAME)
+		/* No need to use this function if we do not look for
+		   the canonical name.  The function does not exist in
+		   all NSS modules and therefore the lookup would
+		   often fail.  */
+		fct = __nss_lookup_function (nip, "gethostbyname3_r");
+	      if (fct == NULL)
+		/* We are cheating here.  The gethostbyname2_r
+		   function does not have the same interface as
+		   gethostbyname3_r but the extra arguments the
+		   latter takes are added at the end.  So the
+		   gethostbyname2_r code will just ignore them.  */
+		fct = __nss_lookup_function (nip, "gethostbyname2_r");
+
+	      if (fct != NULL)
 		{
-		  __set_h_errno (NETDB_INTERNAL);
-		  __set_errno (EBUSY);
-		  break;
-		}
-
-	      no_data = 0;
-	      nss_gethostbyname4_r *fct4 = NULL;
-
-	      /* gethostbyname4_r sends out parallel A and AAAA queries and
-		 is thus only suitable for PF_UNSPEC.  */
-	      if (req->ai_family == PF_UNSPEC)
-		fct4 = __nss_lookup_function (nip, "gethostbyname4_r");
+		  struct gaih_addrtuple **pat = &res.at;
 
-	      if (fct4 != NULL)
-		{
-		  while (1)
+		  if (req->ai_family == AF_INET6
+		      || req->ai_family == AF_UNSPEC)
 		    {
-		      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)
-			{
-			  if (h_errno == TRY_AGAIN)
-			    no_data = EAI_AGAIN;
-			  else
-			    no_data = h_errno == NO_DATA;
-			  break;
-			}
+		      gethosts (AF_INET6);
+		      no_inet6_data = no_data;
+		      inet6_status = status;
+		    }
+		  if (req->ai_family == AF_INET
+		      || req->ai_family == AF_UNSPEC
+		      || (req->ai_family == AF_INET6
+			  && (req->ai_flags & AI_V4MAPPED)
+			  /* Avoid generating the mapped addresses if we
+			     know we are not going to need them.  */
+			  && ((req->ai_flags & AI_ALL) || !got_ipv6)))
+		    {
+		      gethosts (AF_INET);
 
-		      if (!scratch_buffer_grow (tmpbuf))
+		      if (req->ai_family == AF_INET)
 			{
-			  __resolv_context_put (res_ctx);
-			  result = -EAI_MEMORY;
-			  goto free_and_return;
+			  no_inet6_data = no_data;
+			  inet6_status = status;
 			}
 		    }
 
-		  if (status == NSS_STATUS_SUCCESS)
+		  /* If we found one address for AF_INET or AF_INET6,
+		     don't continue the search.  */
+		  if (inet6_status == NSS_STATUS_SUCCESS
+		      || status == NSS_STATUS_SUCCESS)
 		    {
-		      assert (!no_data);
-		      no_data = 1;
-
-		      if ((req->ai_flags & AI_CANONNAME) != 0 && canon == NULL)
+		      if ((req->ai_flags & AI_CANONNAME) != 0
+			  && res.canon == NULL)
 			{
-			  char *canonbuf = __strdup (at->name);
+			  char *canonbuf = getcanonname (nip, res.at, name);
 			  if (canonbuf == NULL)
 			    {
 			      __resolv_context_put (res_ctx);
 			      result = -EAI_MEMORY;
 			      goto free_and_return;
 			    }
-			  canon = canonbuf;
-			}
-
-		      struct gaih_addrtuple **pat = &at;
-
-		      while (*pat != NULL)
-			{
-			  if ((*pat)->family == AF_INET
-			      && req->ai_family == AF_INET6
-			      && (req->ai_flags & AI_V4MAPPED) != 0)
-			    {
-			      uint32_t *pataddr = (*pat)->addr;
-			      (*pat)->family = AF_INET6;
-			      pataddr[3] = pataddr[0];
-			      pataddr[2] = htonl (0xffff);
-			      pataddr[1] = 0;
-			      pataddr[0] = 0;
-			      pat = &((*pat)->next);
-			      no_data = 0;
-			    }
-			  else if (req->ai_family == AF_UNSPEC
-				   || (*pat)->family == req->ai_family)
-			    {
-			      pat = &((*pat)->next);
-
-			      no_data = 0;
-			      if (req->ai_family == AF_INET6)
-				got_ipv6 = true;
-			    }
-			  else
-			    *pat = ((*pat)->next);
-			}
-		    }
-
-		  no_inet6_data = no_data;
-		}
-	      else
-		{
-		  nss_gethostbyname3_r *fct = NULL;
-		  if (req->ai_flags & AI_CANONNAME)
-		    /* No need to use this function if we do not look for
-		       the canonical name.  The function does not exist in
-		       all NSS modules and therefore the lookup would
-		       often fail.  */
-		    fct = __nss_lookup_function (nip, "gethostbyname3_r");
-		  if (fct == NULL)
-		    /* We are cheating here.  The gethostbyname2_r
-		       function does not have the same interface as
-		       gethostbyname3_r but the extra arguments the
-		       latter takes are added at the end.  So the
-		       gethostbyname2_r code will just ignore them.  */
-		    fct = __nss_lookup_function (nip, "gethostbyname2_r");
-
-		  if (fct != NULL)
-		    {
-		      struct gaih_addrtuple **pat = &at;
-
-		      if (req->ai_family == AF_INET6
-			  || req->ai_family == AF_UNSPEC)
-			{
-			  gethosts (AF_INET6);
-			  no_inet6_data = no_data;
-			  inet6_status = status;
-			}
-		      if (req->ai_family == AF_INET
-			  || req->ai_family == AF_UNSPEC
-			  || (req->ai_family == AF_INET6
-			      && (req->ai_flags & AI_V4MAPPED)
-			      /* Avoid generating the mapped addresses if we
-				 know we are not going to need them.  */
-			      && ((req->ai_flags & AI_ALL) || !got_ipv6)))
-			{
-			  gethosts (AF_INET);
-
-			  if (req->ai_family == AF_INET)
-			    {
-			      no_inet6_data = no_data;
-			      inet6_status = status;
-			    }
-			}
-
-		      /* If we found one address for AF_INET or AF_INET6,
-			 don't continue the search.  */
-		      if (inet6_status == NSS_STATUS_SUCCESS
-			  || status == NSS_STATUS_SUCCESS)
-			{
-			  if ((req->ai_flags & AI_CANONNAME) != 0
-			      && canon == NULL)
-			    {
-			      char *canonbuf = getcanonname (nip, at, name);
-			      if (canonbuf == NULL)
-				{
-				  __resolv_context_put (res_ctx);
-				  result = -EAI_MEMORY;
-				  goto free_and_return;
-				}
-			      canon = canonbuf;
-			    }
-			  status = NSS_STATUS_SUCCESS;
-			}
-		      else
-			{
-			  /* We can have different states for AF_INET and
-			     AF_INET6.  Try to find a useful one for both.  */
-			  if (inet6_status == NSS_STATUS_TRYAGAIN)
-			    status = NSS_STATUS_TRYAGAIN;
-			  else if (status == NSS_STATUS_UNAVAIL
-				   && inet6_status != NSS_STATUS_UNAVAIL)
-			    status = inet6_status;
+			  res.canon = canonbuf;
 			}
+		      status = NSS_STATUS_SUCCESS;
 		    }
 		  else
 		    {
-		      /* Could not locate any of the lookup functions.
-			 The NSS lookup code does not consistently set
-			 errno, so we need to supply our own error
-			 code here.  The root cause could either be a
-			 resource allocation failure, or a missing
-			 service function in the DSO (so it should not
-			 be listed in /etc/nsswitch.conf).  Assume the
-			 former, and return EBUSY.  */
-		      status = NSS_STATUS_UNAVAIL;
-		     __set_h_errno (NETDB_INTERNAL);
-		     __set_errno (EBUSY);
+		      /* We can have different states for AF_INET and
+			 AF_INET6.  Try to find a useful one for both.  */
+		      if (inet6_status == NSS_STATUS_TRYAGAIN)
+			status = NSS_STATUS_TRYAGAIN;
+		      else if (status == NSS_STATUS_UNAVAIL
+			       && inet6_status != NSS_STATUS_UNAVAIL)
+			status = inet6_status;
 		    }
 		}
+	      else
+		{
+		  /* Could not locate any of the lookup functions.
+		     The NSS lookup code does not consistently set
+		     errno, so we need to supply our own error
+		     code here.  The root cause could either be a
+		     resource allocation failure, or a missing
+		     service function in the DSO (so it should not
+		     be listed in /etc/nsswitch.conf).  Assume the
+		     former, and return EBUSY.  */
+		  status = NSS_STATUS_UNAVAIL;
+		  __set_h_errno (NETDB_INTERNAL);
+		  __set_errno (EBUSY);
+		}
+	    }
 
-	      if (nss_next_action (nip, status) == NSS_ACTION_RETURN)
-		break;
+	  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;
+	  /* 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;
-	    }
+	  nip++;
+	  if (nip->module == NULL)
+	    no_more = -1;
+	}
 
-	  __resolv_context_put (res_ctx);
+      __resolv_context_put (res_ctx);
 
-	  /* If we have a failure which sets errno, report it using
-	     EAI_SYSTEM.  */
-	  if ((status == NSS_STATUS_TRYAGAIN || status == NSS_STATUS_UNAVAIL)
-	      && h_errno == NETDB_INTERNAL)
-	    {
-	      result = -EAI_SYSTEM;
-	      goto free_and_return;
-	    }
+      /* If we have a failure which sets errno, report it using
+	 EAI_SYSTEM.  */
+      if ((status == NSS_STATUS_TRYAGAIN || status == NSS_STATUS_UNAVAIL)
+	  && h_errno == NETDB_INTERNAL)
+	{
+	  result = -EAI_SYSTEM;
+	  goto free_and_return;
+	}
 
-	  if (no_data != 0 && no_inet6_data != 0)
-	    {
-	      /* If both requests timed out report this.  */
-	      if (no_data == EAI_AGAIN && no_inet6_data == EAI_AGAIN)
-		result = -EAI_AGAIN;
-	      else
-		/* We made requests but they turned out no data.  The name
-		   is known, though.  */
-		result = -EAI_NODATA;
+      if (no_data != 0 && no_inet6_data != 0)
+	{
+	  /* If both requests timed out report this.  */
+	  if (no_data == EAI_AGAIN && no_inet6_data == EAI_AGAIN)
+	    result = -EAI_AGAIN;
+	  else
+	    /* We made requests but they turned out no data.  The name
+	       is known, though.  */
+	    result = -EAI_NODATA;
 
-	      goto free_and_return;
-	    }
+	  goto free_and_return;
 	}
 
     process_list:
-      if (at == NULL)
+      if (res.at == NULL)
 	{
 	  result = -EAI_NONAME;
 	  goto free_and_return;
@@ -1032,21 +1044,22 @@ gaih_inet (const char *name, const struct gaih_service *service,
   else
     {
       struct gaih_addrtuple *atr;
-      atr = at = alloca_account (sizeof (struct gaih_addrtuple), alloca_used);
-      memset (at, '\0', sizeof (struct gaih_addrtuple));
+      atr = res.at = alloca_account (sizeof (struct gaih_addrtuple),
+				     alloca_used);
+      memset (res.at, '\0', sizeof (struct gaih_addrtuple));
 
       if (req->ai_family == AF_UNSPEC)
 	{
-	  at->next = __alloca (sizeof (struct gaih_addrtuple));
-	  memset (at->next, '\0', sizeof (struct gaih_addrtuple));
+	  res.at->next = __alloca (sizeof (struct gaih_addrtuple));
+	  memset (res.at->next, '\0', sizeof (struct gaih_addrtuple));
 	}
 
       if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET6)
 	{
-	  at->family = AF_INET6;
+	  res.at->family = AF_INET6;
 	  if ((req->ai_flags & AI_PASSIVE) == 0)
-	    memcpy (at->addr, &in6addr_loopback, sizeof (struct in6_addr));
-	  atr = at->next;
+	    memcpy (res.at->addr, &in6addr_loopback, sizeof (struct in6_addr));
+	  atr = res.at->next;
 	}
 
       if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
@@ -1059,10 +1072,10 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
   {
     /* Set up the canonical name if we need it.  */
-    if ((result = process_canonname (req, orig_name, &canon)) != 0)
+    if ((result = process_canonname (req, orig_name, &res)) != 0)
       goto free_and_return;
 
-    struct gaih_addrtuple *at2 = at;
+    struct gaih_addrtuple *at2 = res.at;
     size_t socklen;
     sa_family_t family;
 
@@ -1105,8 +1118,8 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	    ai->ai_addr = (void *) (ai + 1);
 
 	    /* We only add the canonical name once.  */
-	    ai->ai_canonname = (char *) canon;
-	    canon = NULL;
+	    ai->ai_canonname = res.canon;
+	    res.canon = NULL;
 
 #ifdef _HAVE_SA_LEN
 	    ai->ai_addr->sa_len = socklen;
@@ -1152,7 +1165,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
   if (malloc_name)
     free ((char *) name);
   free (addrmem);
-  free (canon);
+  free (res.canon);
 
   return result;
 }
-- 
2.41.0


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

* [pushed 2.34 07/14] gaih_inet: Split simple gethostbyname into its own function
  2023-09-16  0:48 [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
                   ` (5 preceding siblings ...)
  2023-09-16  0:48 ` [pushed 2.34 06/14] gaih_inet: make numeric lookup a separate routine Siddhesh Poyarekar
@ 2023-09-16  0:48 ` Siddhesh Poyarekar
  2023-09-16  0:48 ` [pushed 2.34 08/14] gaih_inet: Split nscd lookup code " Siddhesh Poyarekar
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-16  0:48 UTC (permalink / raw)
  To: libc-stable; +Cc: DJ Delorie

Add a free_at flag in gaih_result to indicate if res.at needs to be
freed by the caller.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit b44389cb7fa28a59804571dac09cc32ebfac03d1)
---
 sysdeps/posix/getaddrinfo.c | 127 ++++++++++++++++++------------------
 1 file changed, 64 insertions(+), 63 deletions(-)

diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 57b6834c8b..3870b2dc2e 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -120,6 +120,7 @@ struct gaih_result
 {
   struct gaih_addrtuple *at;
   char *canon;
+  bool free_at;
 };
 
 /* Values for `protoflag'.  */
@@ -565,6 +566,62 @@ out:
   return result;
 }
 
+/* If possible, call the simple, old functions, which do not support IPv6 scope
+   ids, nor retrieving the canonical name.  */
+
+static int
+try_simple_gethostbyname (const char *name, const struct addrinfo *req,
+			  struct scratch_buffer *tmpbuf,
+			  struct gaih_result *res)
+{
+  res->at = NULL;
+
+  if (req->ai_family != AF_INET || (req->ai_flags & AI_CANONNAME) != 0)
+    return 0;
+
+  int rc;
+  struct hostent th;
+  struct hostent *h;
+
+  while (1)
+    {
+      rc = __gethostbyname2_r (name, AF_INET, &th, tmpbuf->data,
+			       tmpbuf->length, &h, &h_errno);
+      if (rc != ERANGE || h_errno != NETDB_INTERNAL)
+	break;
+      if (!scratch_buffer_grow (tmpbuf))
+	return -EAI_MEMORY;
+    }
+
+  if (rc == 0)
+    {
+      if (h != NULL)
+	{
+	  /* We found data, convert it.  RES->AT from the conversion will
+	     either be an allocated block or NULL, both of which are safe to
+	     pass to free ().  */
+	  if (!convert_hostent_to_gaih_addrtuple (req, AF_INET, h, &res->at))
+	    return -EAI_MEMORY;
+
+	  res->free_at = true;
+	  return 0;
+	}
+      if (h_errno == NO_DATA)
+	return -EAI_NODATA;
+
+      return -EAI_NONAME;
+    }
+
+  if (h_errno == NETDB_INTERNAL)
+    return -EAI_SYSTEM;
+  if (h_errno == TRY_AGAIN)
+    return -EAI_AGAIN;
+
+  /* We made requests but they turned out no data.
+     The name is known, though.  */
+  return -EAI_NODATA;
+}
+
 static int
 gaih_inet (const char *name, const struct gaih_service *service,
 	   const struct addrinfo *req, struct addrinfo **pai,
@@ -610,6 +667,11 @@ gaih_inet (const char *name, const struct gaih_service *service,
       else if (res.at != NULL)
 	goto process_list;
 
+      if ((result = try_simple_gethostbyname (name, req, tmpbuf, &res)) != 0)
+	goto free_and_return;
+      else if (res.at != NULL)
+	goto process_list;
+
       int no_data = 0;
       int no_inet6_data = 0;
       nss_action_list nip;
@@ -619,69 +681,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
       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
-	 IPv6 scope ids, nor retrieving the canonical name.  */
-      if (req->ai_family == AF_INET
-	  && (req->ai_flags & AI_CANONNAME) == 0)
-	{
-	  int rc;
-	  struct hostent th;
-	  struct hostent *h;
-
-	  while (1)
-	    {
-	      rc = __gethostbyname2_r (name, AF_INET, &th,
-				       tmpbuf->data, tmpbuf->length,
-				       &h, &h_errno);
-	      if (rc != ERANGE || h_errno != NETDB_INTERNAL)
-		break;
-	      if (!scratch_buffer_grow (tmpbuf))
-		{
-		  result = -EAI_MEMORY;
-		  goto free_and_return;
-		}
-	    }
-
-	  if (rc == 0)
-	    {
-	      if (h != NULL)
-		{
-		  /* We found data, convert it.  */
-		  if (!convert_hostent_to_gaih_addrtuple
-		      (req, AF_INET, h, &addrmem))
-		    {
-		      result = -EAI_MEMORY;
-		      goto free_and_return;
-		    }
-		  res.at = addrmem;
-		}
-	      else
-		{
-		  if (h_errno == NO_DATA)
-		    result = -EAI_NODATA;
-		  else
-		    result = -EAI_NONAME;
-		  goto free_and_return;
-		}
-	    }
-	  else
-	    {
-	      if (h_errno == NETDB_INTERNAL)
-		result = -EAI_SYSTEM;
-	      else if (h_errno == TRY_AGAIN)
-		result = -EAI_AGAIN;
-	      else
-		/* We made requests but they turned out no data.
-		   The name is known, though.  */
-		result = -EAI_NODATA;
-
-	      goto free_and_return;
-	    }
-
-	  goto process_list;
-	}
-
 #ifdef USE_NSCD
       if (__nss_not_use_nscd_hosts > 0
 	  && ++__nss_not_use_nscd_hosts > NSS_NSCD_RETRY)
@@ -1165,6 +1164,8 @@ gaih_inet (const char *name, const struct gaih_service *service,
   if (malloc_name)
     free ((char *) name);
   free (addrmem);
+  if (res.free_at)
+    free (res.at);
   free (res.canon);
 
   return result;
-- 
2.41.0


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

* [pushed 2.34 08/14] gaih_inet: Split nscd lookup code into its own function.
  2023-09-16  0:48 [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
                   ` (6 preceding siblings ...)
  2023-09-16  0:48 ` [pushed 2.34 07/14] gaih_inet: Split simple gethostbyname into its own function Siddhesh Poyarekar
@ 2023-09-16  0:48 ` Siddhesh Poyarekar
  2023-09-16  0:48 ` [pushed 2.34 09/14] gaih_inet: separate nss lookup loop " Siddhesh Poyarekar
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-16  0:48 UTC (permalink / raw)
  To: libc-stable; +Cc: DJ Delorie

Add a new member got_ipv6 to indicate if the results have an IPv6
result and use it instead of the local got_ipv6.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit e7e5315b7fa065a9c8bf525ca9a32f46fa4837e5)
---
 sysdeps/posix/getaddrinfo.c | 248 +++++++++++++++++++-----------------
 1 file changed, 134 insertions(+), 114 deletions(-)

diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 3870b2dc2e..7c497a88f8 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -121,6 +121,7 @@ struct gaih_result
   struct gaih_addrtuple *at;
   char *canon;
   bool free_at;
+  bool got_ipv6;
 };
 
 /* Values for `protoflag'.  */
@@ -316,7 +317,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
 	  res.canon = canonbuf;						      \
 	}								      \
       if (_family == AF_INET6 && *pat != NULL)				      \
-	got_ipv6 = true;						      \
+	res.got_ipv6 = true;						      \
     }									      \
  }
 
@@ -467,6 +468,128 @@ get_servtuples (const struct gaih_service *service, const struct addrinfo *req,
   return 0;
 }
 
+#ifdef USE_NSCD
+/* Query addresses from nscd cache, returning a non-zero value on error.
+   RES members have the lookup result; RES->AT is NULL if there were no errors
+   but also no results.  */
+
+static int
+get_nscd_addresses (const char *name, const struct addrinfo *req,
+		    struct gaih_result *res)
+{
+  if (__nss_not_use_nscd_hosts > 0
+      && ++__nss_not_use_nscd_hosts > NSS_NSCD_RETRY)
+    __nss_not_use_nscd_hosts = 0;
+
+  res->at = NULL;
+
+  if (__nss_not_use_nscd_hosts || __nss_database_custom[NSS_DBSIDX_hosts])
+    return 0;
+
+  /* Try to use nscd.  */
+  struct nscd_ai_result *air = NULL;
+  int err = __nscd_getai (name, &air, &h_errno);
+
+  if (__glibc_unlikely (air == NULL))
+    {
+      /* The database contains a negative entry.  */
+      if (err == 0)
+	return -EAI_NONAME;
+      if (__nss_not_use_nscd_hosts == 0)
+	{
+	  if (h_errno == NETDB_INTERNAL && errno == ENOMEM)
+	    return -EAI_MEMORY;
+	  if (h_errno == TRY_AGAIN)
+	    return -EAI_AGAIN;
+	  return -EAI_SYSTEM;
+	}
+      return 0;
+    }
+
+  /* Transform into gaih_addrtuple list.  */
+  int result = 0;
+  char *addrs = air->addrs;
+
+  struct gaih_addrtuple *addrfree = calloc (air->naddrs, sizeof (*addrfree));
+  struct gaih_addrtuple *at = calloc (air->naddrs, sizeof (*at));
+  if (at == NULL)
+    {
+      result = -EAI_MEMORY;
+      goto out;
+    }
+
+  res->free_at = true;
+
+  int count = 0;
+  for (int i = 0; i < air->naddrs; ++i)
+    {
+      socklen_t size = (air->family[i] == AF_INET
+			? INADDRSZ : IN6ADDRSZ);
+
+      if (!((air->family[i] == AF_INET
+	     && req->ai_family == AF_INET6
+	     && (req->ai_flags & AI_V4MAPPED) != 0)
+	    || req->ai_family == AF_UNSPEC
+	    || air->family[i] == req->ai_family))
+	{
+	  /* Skip over non-matching result.  */
+	  addrs += size;
+	  continue;
+	}
+
+      if (air->family[i] == AF_INET && req->ai_family == AF_INET6
+	  && (req->ai_flags & AI_V4MAPPED))
+	{
+	  at[count].family = AF_INET6;
+	  at[count].addr[3] = *(uint32_t *) addrs;
+	  at[count].addr[2] = htonl (0xffff);
+	}
+      else if (req->ai_family == AF_UNSPEC
+	       || air->family[count] == req->ai_family)
+	{
+	  at[count].family = air->family[count];
+	  memcpy (at[count].addr, addrs, size);
+	  if (air->family[count] == AF_INET6)
+	    res->got_ipv6 = true;
+	}
+      at[count].next = at + count + 1;
+      count++;
+      addrs += size;
+    }
+
+  if ((req->ai_flags & AI_CANONNAME) && air->canon != NULL)
+    {
+      char *canonbuf = __strdup (air->canon);
+      if (canonbuf == NULL)
+	{
+	  result = -EAI_MEMORY;
+	  goto out;
+	}
+      res->canon = canonbuf;
+    }
+
+  if (count == 0)
+    {
+      result = -EAI_NONAME;
+      goto out;
+    }
+
+  at[count - 1].next = NULL;
+
+  res->at = at;
+
+out:
+  free (air);
+  if (result != 0)
+    {
+      free (at);
+      res->free_at = false;
+    }
+
+  return result;
+}
+#endif
+
 /* Convert numeric addresses to binary into RES.  On failure, RES->AT is set to
    NULL and an error code is returned.  If AI_NUMERIC_HOST is not requested and
    the function cannot determine a result, RES->AT is set to NULL and 0
@@ -630,7 +753,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
   struct gaih_servtuple st[sizeof (gaih_inet_typeproto)
 			   / sizeof (struct gaih_typeproto)] = {0};
 
-  bool got_ipv6 = false;
   const char *orig_name = name;
 
   /* Reserve stack memory for the scratch buffer in the getaddrinfo
@@ -672,6 +794,13 @@ gaih_inet (const char *name, const struct gaih_service *service,
       else if (res.at != NULL)
 	goto process_list;
 
+#ifdef USE_NSCD
+      if ((result = get_nscd_addresses (name, req, &res)) != 0)
+	goto free_and_return;
+      else if (res.at != NULL)
+	goto process_list;
+#endif
+
       int no_data = 0;
       int no_inet6_data = 0;
       nss_action_list nip;
@@ -681,115 +810,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
       struct resolv_context *res_ctx = NULL;
       bool do_merge = false;
 
-#ifdef USE_NSCD
-      if (__nss_not_use_nscd_hosts > 0
-	  && ++__nss_not_use_nscd_hosts > NSS_NSCD_RETRY)
-	__nss_not_use_nscd_hosts = 0;
-
-      if (!__nss_not_use_nscd_hosts
-	  && !__nss_database_custom[NSS_DBSIDX_hosts])
-	{
-	  /* Try to use nscd.  */
-	  struct nscd_ai_result *air = NULL;
-	  int err = __nscd_getai (name, &air, &h_errno);
-	  if (air != NULL)
-	    {
-	      /* Transform into gaih_addrtuple list.  */
-	      bool added_canon = (req->ai_flags & AI_CANONNAME) == 0;
-	      char *addrs = air->addrs;
-
-	      addrmem = calloc (air->naddrs, sizeof (*addrmem));
-	      if (addrmem == NULL)
-		{
-		  result = -EAI_MEMORY;
-		  goto free_and_return;
-		}
-
-	      struct gaih_addrtuple *addrfree = addrmem;
-	      struct gaih_addrtuple **pat = &res.at;
-
-	      for (int i = 0; i < air->naddrs; ++i)
-		{
-		  socklen_t size = (air->family[i] == AF_INET
-				    ? INADDRSZ : IN6ADDRSZ);
-
-		  if (!((air->family[i] == AF_INET
-			 && req->ai_family == AF_INET6
-			 && (req->ai_flags & AI_V4MAPPED) != 0)
-			|| req->ai_family == AF_UNSPEC
-			|| air->family[i] == req->ai_family))
-		    {
-		      /* Skip over non-matching result.  */
-		      addrs += size;
-		      continue;
-		    }
-
-		  if (*pat == NULL)
-		    {
-		      *pat = addrfree++;
-		      (*pat)->scopeid = 0;
-		    }
-		  uint32_t *pataddr = (*pat)->addr;
-		  (*pat)->next = NULL;
-		  if (added_canon || air->canon == NULL)
-		    (*pat)->name = NULL;
-		  else if (res.canon == NULL)
-		    {
-		      char *canonbuf = __strdup (air->canon);
-		      if (canonbuf == NULL)
-			{
-			  result = -EAI_MEMORY;
-			  goto free_and_return;
-			}
-		      res.canon = (*pat)->name = canonbuf;
-		    }
-
-		  if (air->family[i] == AF_INET
-		      && req->ai_family == AF_INET6
-		      && (req->ai_flags & AI_V4MAPPED))
-		    {
-		      (*pat)->family = AF_INET6;
-		      pataddr[3] = *(uint32_t *) addrs;
-		      pataddr[2] = htonl (0xffff);
-		      pataddr[1] = 0;
-		      pataddr[0] = 0;
-		      pat = &((*pat)->next);
-		      added_canon = true;
-		    }
-		  else if (req->ai_family == AF_UNSPEC
-			   || air->family[i] == req->ai_family)
-		    {
-		      (*pat)->family = air->family[i];
-		      memcpy (pataddr, addrs, size);
-		      pat = &((*pat)->next);
-		      added_canon = true;
-		      if (air->family[i] == AF_INET6)
-			got_ipv6 = true;
-		    }
-		  addrs += size;
-		}
-
-	      free (air);
-
-	      goto process_list;
-	    }
-	  else if (err == 0)
-	    /* The database contains a negative entry.  */
-	    goto free_and_return;
-	  else if (__nss_not_use_nscd_hosts == 0)
-	    {
-	      if (h_errno == NETDB_INTERNAL && errno == ENOMEM)
-		result = -EAI_MEMORY;
-	      else if (h_errno == TRY_AGAIN)
-		result = -EAI_AGAIN;
-	      else
-		result = -EAI_SYSTEM;
-
-	      goto free_and_return;
-	    }
-	}
-#endif
-
       no_more = !__nss_database_get (nss_database_hosts, &nip);
 
       /* If we are looking for both IPv4 and IPv6 address we don't
@@ -897,7 +917,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
 			  no_data = 0;
 			  if (req->ai_family == AF_INET6)
-			    got_ipv6 = true;
+			    res.got_ipv6 = true;
 			}
 		      else
 			*pat = ((*pat)->next);
@@ -940,7 +960,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 			  && (req->ai_flags & AI_V4MAPPED)
 			  /* Avoid generating the mapped addresses if we
 			     know we are not going to need them.  */
-			  && ((req->ai_flags & AI_ALL) || !got_ipv6)))
+			  && ((req->ai_flags & AI_ALL) || !res.got_ipv6)))
 		    {
 		      gethosts (AF_INET);
 
@@ -1091,7 +1111,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	    /* If we looked up IPv4 mapped address discard them here if
 	       the caller isn't interested in all address and we have
 	       found at least one IPv6 address.  */
-	    if (got_ipv6
+	    if (res.got_ipv6
 		&& (req->ai_flags & (AI_V4MAPPED|AI_ALL)) == AI_V4MAPPED
 		&& IN6_IS_ADDR_V4MAPPED (at2->addr))
 	      goto ignore;
-- 
2.41.0


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

* [pushed 2.34 09/14] gaih_inet: separate nss lookup loop into its own function
  2023-09-16  0:48 [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
                   ` (7 preceding siblings ...)
  2023-09-16  0:48 ` [pushed 2.34 08/14] gaih_inet: Split nscd lookup code " Siddhesh Poyarekar
@ 2023-09-16  0:48 ` Siddhesh Poyarekar
  2023-09-16  0:48 ` [pushed 2.34 10/14] gaih_inet: make gethosts into a function Siddhesh Poyarekar
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-16  0:48 UTC (permalink / raw)
  To: libc-stable; +Cc: DJ Delorie

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit 906cecbe0889e601c91d9aba738049c73ebe4dd2)
---
 sysdeps/posix/getaddrinfo.c | 563 ++++++++++++++++++------------------
 1 file changed, 286 insertions(+), 277 deletions(-)

diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 7c497a88f8..145ea6fa38 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -159,6 +159,14 @@ static const struct addrinfo default_hints =
     .ai_next = NULL
   };
 
+static void
+gaih_result_reset (struct gaih_result *res)
+{
+  if (res->free_at)
+    free (res->at);
+  free (res->canon);
+  memset (res, 0, sizeof (*res));
+}
 
 static int
 gaih_inet_serv (const char *servicename, const struct gaih_typeproto *tp,
@@ -197,13 +205,10 @@ gaih_inet_serv (const char *servicename, const struct gaih_typeproto *tp,
 
 /* Convert struct hostent to a list of struct gaih_addrtuple objects.  h_name
    is not copied, and the struct hostent object must not be deallocated
-   prematurely.  The new addresses are appended to the tuple array in
-   RESULT.  */
+   prematurely.  The new addresses are appended to the tuple array in RES.  */
 static bool
-convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
-				   int family,
-				   struct hostent *h,
-				   struct gaih_addrtuple **result)
+convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, int family,
+				   struct hostent *h, struct gaih_result *res)
 {
   /* Count the number of addresses in h->h_addr_list.  */
   size_t count = 0;
@@ -215,7 +220,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
   if (count == 0 || h->h_length > sizeof (((struct gaih_addrtuple) {}).addr))
     return true;
 
-  struct gaih_addrtuple *array = *result;
+  struct gaih_addrtuple *array = res->at;
   size_t old = 0;
 
   while (array != NULL)
@@ -224,12 +229,14 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
       array = array->next;
     }
 
-  array = realloc (*result, (old + count) * sizeof (*array));
+  array = realloc (res->at, (old + count) * sizeof (*array));
 
   if (array == NULL)
     return false;
 
-  *result = array;
+  res->got_ipv6 = family == AF_INET6;
+  res->at = array;
+  res->free_at = true;
 
   /* Update the next pointers on reallocation.  */
   for (size_t i = 0; i < old; i++)
@@ -278,7 +285,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
 	{								      \
 	  __resolv_context_put (res_ctx);				      \
 	  result = -EAI_MEMORY;						      \
-	  goto free_and_return;						      \
+	  goto out;							      \
 	}								      \
     }									      \
   if (status == NSS_STATUS_NOTFOUND					      \
@@ -288,7 +295,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
 	{								      \
 	  __resolv_context_put (res_ctx);				      \
 	  result = -EAI_SYSTEM;						      \
-	  goto free_and_return;						      \
+	  goto out;							      \
 	}								      \
       if (h_errno == TRY_AGAIN)						      \
 	no_data = EAI_AGAIN;						      \
@@ -297,27 +304,24 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
     }									      \
   else if (status == NSS_STATUS_SUCCESS)				      \
     {									      \
-      if (!convert_hostent_to_gaih_addrtuple (req, _family, &th, &addrmem))   \
+      if (!convert_hostent_to_gaih_addrtuple (req, _family, &th, res))	      \
 	{								      \
 	  __resolv_context_put (res_ctx);				      \
 	  result = -EAI_SYSTEM;						      \
-	  goto free_and_return;						      \
+	  goto out;							      \
 	}								      \
-      *pat = addrmem;							      \
 									      \
-      if (localcanon != NULL && res.canon == NULL)			      \
+      if (localcanon != NULL && res->canon == NULL)			      \
 	{								      \
 	  char *canonbuf = __strdup (localcanon);			      \
 	  if (canonbuf == NULL)						      \
 	    {								      \
 	      __resolv_context_put (res_ctx);				      \
 	      result = -EAI_SYSTEM;					      \
-	      goto free_and_return;					      \
+	      goto out;							      \
 	    }								      \
-	  res.canon = canonbuf;						      \
+	  res->canon = canonbuf;					      \
 	}								      \
-      if (_family == AF_INET6 && *pat != NULL)				      \
-	res.got_ipv6 = true;						      \
     }									      \
  }
 
@@ -590,6 +594,260 @@ out:
 }
 #endif
 
+static int
+get_nss_addresses (const char *name, const struct addrinfo *req,
+		   struct scratch_buffer *tmpbuf, struct gaih_result *res)
+{
+  int no_data = 0;
+  int no_inet6_data = 0;
+  nss_action_list nip;
+  enum nss_status inet6_status = NSS_STATUS_UNAVAIL;
+  enum nss_status status = NSS_STATUS_UNAVAIL;
+  int no_more;
+  struct resolv_context *res_ctx = NULL;
+  bool do_merge = false;
+  int result = 0;
+
+  no_more = !__nss_database_get (nss_database_hosts, &nip);
+
+  /* If we are looking for both IPv4 and IPv6 address we don't
+     want the lookup functions to automatically promote IPv4
+     addresses to IPv6 addresses, so we use the no_inet6
+     function variant.  */
+  res_ctx = __resolv_context_get ();
+  if (res_ctx == NULL)
+    no_more = 1;
+
+  while (!no_more)
+    {
+      /* Always start afresh; continue should discard previous results
+	 and the hosts database does not support merge.  */
+      gaih_result_reset (res);
+
+      if (do_merge)
+	{
+	  __set_h_errno (NETDB_INTERNAL);
+	  __set_errno (EBUSY);
+	  break;
+	}
+
+      no_data = 0;
+      nss_gethostbyname4_r *fct4 = NULL;
+
+      /* gethostbyname4_r sends out parallel A and AAAA queries and
+	 is thus only suitable for PF_UNSPEC.  */
+      if (req->ai_family == PF_UNSPEC)
+	fct4 = __nss_lookup_function (nip, "gethostbyname4_r");
+
+      if (fct4 != NULL)
+	{
+	  while (1)
+	    {
+	      status = DL_CALL_FCT (fct4, (name, &res->at,
+					   tmpbuf->data, tmpbuf->length,
+					   &errno, &h_errno,
+					   NULL));
+	      if (status == NSS_STATUS_SUCCESS)
+		break;
+	      /* gethostbyname4_r may write into AT, so reset it.  */
+	      res->at = NULL;
+	      if (status != NSS_STATUS_TRYAGAIN
+		  || errno != ERANGE || h_errno != NETDB_INTERNAL)
+		{
+		  if (h_errno == TRY_AGAIN)
+		    no_data = EAI_AGAIN;
+		  else
+		    no_data = h_errno == NO_DATA;
+		  break;
+		}
+
+	      if (!scratch_buffer_grow (tmpbuf))
+		{
+		  __resolv_context_put (res_ctx);
+		  result = -EAI_MEMORY;
+		  goto out;
+		}
+	    }
+
+	  if (status == NSS_STATUS_SUCCESS)
+	    {
+	      assert (!no_data);
+	      no_data = 1;
+
+	      if ((req->ai_flags & AI_CANONNAME) != 0 && res->canon == NULL)
+		{
+		  char *canonbuf = __strdup (res->at->name);
+		  if (canonbuf == NULL)
+		    {
+		      __resolv_context_put (res_ctx);
+		      result = -EAI_MEMORY;
+		      goto out;
+		    }
+		  res->canon = canonbuf;
+		}
+
+	      struct gaih_addrtuple **pat = &res->at;
+
+	      while (*pat != NULL)
+		{
+		  if ((*pat)->family == AF_INET
+		      && req->ai_family == AF_INET6
+		      && (req->ai_flags & AI_V4MAPPED) != 0)
+		    {
+		      uint32_t *pataddr = (*pat)->addr;
+		      (*pat)->family = AF_INET6;
+		      pataddr[3] = pataddr[0];
+		      pataddr[2] = htonl (0xffff);
+		      pataddr[1] = 0;
+		      pataddr[0] = 0;
+		      pat = &((*pat)->next);
+		      no_data = 0;
+		    }
+		  else if (req->ai_family == AF_UNSPEC
+			   || (*pat)->family == req->ai_family)
+		    {
+		      pat = &((*pat)->next);
+
+		      no_data = 0;
+		      if (req->ai_family == AF_INET6)
+			res->got_ipv6 = true;
+		    }
+		  else
+		    *pat = ((*pat)->next);
+		}
+	    }
+
+	  no_inet6_data = no_data;
+	}
+      else
+	{
+	  nss_gethostbyname3_r *fct = NULL;
+	  if (req->ai_flags & AI_CANONNAME)
+	    /* No need to use this function if we do not look for
+	       the canonical name.  The function does not exist in
+	       all NSS modules and therefore the lookup would
+	       often fail.  */
+	    fct = __nss_lookup_function (nip, "gethostbyname3_r");
+	  if (fct == NULL)
+	    /* We are cheating here.  The gethostbyname2_r
+	       function does not have the same interface as
+	       gethostbyname3_r but the extra arguments the
+	       latter takes are added at the end.  So the
+	       gethostbyname2_r code will just ignore them.  */
+	    fct = __nss_lookup_function (nip, "gethostbyname2_r");
+
+	  if (fct != NULL)
+	    {
+	      if (req->ai_family == AF_INET6
+		  || req->ai_family == AF_UNSPEC)
+		{
+		  gethosts (AF_INET6);
+		  no_inet6_data = no_data;
+		  inet6_status = status;
+		}
+	      if (req->ai_family == AF_INET
+		  || req->ai_family == AF_UNSPEC
+		  || (req->ai_family == AF_INET6
+		      && (req->ai_flags & AI_V4MAPPED)
+		      /* Avoid generating the mapped addresses if we
+			 know we are not going to need them.  */
+		      && ((req->ai_flags & AI_ALL) || !res->got_ipv6)))
+		{
+		  gethosts (AF_INET);
+
+		  if (req->ai_family == AF_INET)
+		    {
+		      no_inet6_data = no_data;
+		      inet6_status = status;
+		    }
+		}
+
+	      /* If we found one address for AF_INET or AF_INET6,
+		 don't continue the search.  */
+	      if (inet6_status == NSS_STATUS_SUCCESS
+		  || status == NSS_STATUS_SUCCESS)
+		{
+		  if ((req->ai_flags & AI_CANONNAME) != 0
+		      && res->canon == NULL)
+		    {
+		      char *canonbuf = getcanonname (nip, res->at, name);
+		      if (canonbuf == NULL)
+			{
+			  __resolv_context_put (res_ctx);
+			  result = -EAI_MEMORY;
+			  goto out;
+			}
+		      res->canon = canonbuf;
+		    }
+		  status = NSS_STATUS_SUCCESS;
+		}
+	      else
+		{
+		  /* We can have different states for AF_INET and
+		     AF_INET6.  Try to find a useful one for both.  */
+		  if (inet6_status == NSS_STATUS_TRYAGAIN)
+		    status = NSS_STATUS_TRYAGAIN;
+		  else if (status == NSS_STATUS_UNAVAIL
+			   && inet6_status != NSS_STATUS_UNAVAIL)
+		    status = inet6_status;
+		}
+	    }
+	  else
+	    {
+	      /* Could not locate any of the lookup functions.
+		 The NSS lookup code does not consistently set
+		 errno, so we need to supply our own error
+		 code here.  The root cause could either be a
+		 resource allocation failure, or a missing
+		 service function in the DSO (so it should not
+		 be listed in /etc/nsswitch.conf).  Assume the
+		 former, and return EBUSY.  */
+	      status = NSS_STATUS_UNAVAIL;
+	      __set_h_errno (NETDB_INTERNAL);
+	      __set_errno (EBUSY);
+	    }
+	}
+
+      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;
+    }
+
+  __resolv_context_put (res_ctx);
+
+  /* If we have a failure which sets errno, report it using
+     EAI_SYSTEM.  */
+  if ((status == NSS_STATUS_TRYAGAIN || status == NSS_STATUS_UNAVAIL)
+      && h_errno == NETDB_INTERNAL)
+    {
+      result = -EAI_SYSTEM;
+      goto out;
+    }
+
+  if (no_data != 0 && no_inet6_data != 0)
+    {
+      /* If both requests timed out report this.  */
+      if (no_data == EAI_AGAIN && no_inet6_data == EAI_AGAIN)
+	result = -EAI_AGAIN;
+      else
+	/* We made requests but they turned out no data.  The name
+	   is known, though.  */
+	result = -EAI_NODATA;
+    }
+
+out:
+  if (result != 0)
+    gaih_result_reset (res);
+  return result;
+}
+
 /* Convert numeric addresses to binary into RES.  On failure, RES->AT is set to
    NULL and an error code is returned.  If AI_NUMERIC_HOST is not requested and
    the function cannot determine a result, RES->AT is set to NULL and 0
@@ -723,7 +981,7 @@ try_simple_gethostbyname (const char *name, const struct addrinfo *req,
 	  /* We found data, convert it.  RES->AT from the conversion will
 	     either be an allocated block or NULL, both of which are safe to
 	     pass to free ().  */
-	  if (!convert_hostent_to_gaih_addrtuple (req, AF_INET, h, &res->at))
+	  if (!convert_hostent_to_gaih_addrtuple (req, AF_INET, h, res))
 	    return -EAI_MEMORY;
 
 	  res->free_at = true;
@@ -801,264 +1059,14 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	goto process_list;
 #endif
 
-      int no_data = 0;
-      int no_inet6_data = 0;
-      nss_action_list nip;
-      enum nss_status inet6_status = NSS_STATUS_UNAVAIL;
-      enum nss_status status = NSS_STATUS_UNAVAIL;
-      int no_more;
-      struct resolv_context *res_ctx = NULL;
-      bool do_merge = false;
-
-      no_more = !__nss_database_get (nss_database_hosts, &nip);
-
-      /* If we are looking for both IPv4 and IPv6 address we don't
-	 want the lookup functions to automatically promote IPv4
-	 addresses to IPv6 addresses, so we use the no_inet6
-	 function variant.  */
-      res_ctx = __resolv_context_get ();
-      if (res_ctx == NULL)
-	no_more = 1;
-
-      while (!no_more)
-	{
-	  /* Always start afresh; continue should discard previous results
-	     and the hosts database does not support merge.  */
-	  res.at = NULL;
-	  free (res.canon);
-	  free (addrmem);
-	  res.canon = NULL;
-	  addrmem = NULL;
-	  got_ipv6 = false;
-
-	  if (do_merge)
-	    {
-	      __set_h_errno (NETDB_INTERNAL);
-	      __set_errno (EBUSY);
-	      break;
-	    }
-
-	  no_data = 0;
-	  nss_gethostbyname4_r *fct4 = NULL;
-
-	  /* gethostbyname4_r sends out parallel A and AAAA queries and
-	     is thus only suitable for PF_UNSPEC.  */
-	  if (req->ai_family == PF_UNSPEC)
-	    fct4 = __nss_lookup_function (nip, "gethostbyname4_r");
-
-	  if (fct4 != NULL)
-	    {
-	      while (1)
-		{
-		  status = DL_CALL_FCT (fct4, (name, &res.at,
-					       tmpbuf->data, tmpbuf->length,
-					       &errno, &h_errno,
-					       NULL));
-		  if (status == NSS_STATUS_SUCCESS)
-		    break;
-		  /* gethostbyname4_r may write into AT, so reset it.  */
-		  res.at = NULL;
-		  if (status != NSS_STATUS_TRYAGAIN
-		      || errno != ERANGE || h_errno != NETDB_INTERNAL)
-		    {
-		      if (h_errno == TRY_AGAIN)
-			no_data = EAI_AGAIN;
-		      else
-			no_data = h_errno == NO_DATA;
-		      break;
-		    }
-
-		  if (!scratch_buffer_grow (tmpbuf))
-		    {
-		      __resolv_context_put (res_ctx);
-		      result = -EAI_MEMORY;
-		      goto free_and_return;
-		    }
-		}
-
-	      if (status == NSS_STATUS_SUCCESS)
-		{
-		  assert (!no_data);
-		  no_data = 1;
-
-		  if ((req->ai_flags & AI_CANONNAME) != 0 && res.canon == NULL)
-		    {
-		      char *canonbuf = __strdup (res.at->name);
-		      if (canonbuf == NULL)
-			{
-			  __resolv_context_put (res_ctx);
-			  result = -EAI_MEMORY;
-			  goto free_and_return;
-			}
-		      res.canon = canonbuf;
-		    }
-
-		  struct gaih_addrtuple **pat = &res.at;
-
-		  while (*pat != NULL)
-		    {
-		      if ((*pat)->family == AF_INET
-			  && req->ai_family == AF_INET6
-			  && (req->ai_flags & AI_V4MAPPED) != 0)
-			{
-			  uint32_t *pataddr = (*pat)->addr;
-			  (*pat)->family = AF_INET6;
-			  pataddr[3] = pataddr[0];
-			  pataddr[2] = htonl (0xffff);
-			  pataddr[1] = 0;
-			  pataddr[0] = 0;
-			  pat = &((*pat)->next);
-			  no_data = 0;
-			}
-		      else if (req->ai_family == AF_UNSPEC
-			       || (*pat)->family == req->ai_family)
-			{
-			  pat = &((*pat)->next);
-
-			  no_data = 0;
-			  if (req->ai_family == AF_INET6)
-			    res.got_ipv6 = true;
-			}
-		      else
-			*pat = ((*pat)->next);
-		    }
-		}
-
-	      no_inet6_data = no_data;
-	    }
-	  else
-	    {
-	      nss_gethostbyname3_r *fct = NULL;
-	      if (req->ai_flags & AI_CANONNAME)
-		/* No need to use this function if we do not look for
-		   the canonical name.  The function does not exist in
-		   all NSS modules and therefore the lookup would
-		   often fail.  */
-		fct = __nss_lookup_function (nip, "gethostbyname3_r");
-	      if (fct == NULL)
-		/* We are cheating here.  The gethostbyname2_r
-		   function does not have the same interface as
-		   gethostbyname3_r but the extra arguments the
-		   latter takes are added at the end.  So the
-		   gethostbyname2_r code will just ignore them.  */
-		fct = __nss_lookup_function (nip, "gethostbyname2_r");
-
-	      if (fct != NULL)
-		{
-		  struct gaih_addrtuple **pat = &res.at;
-
-		  if (req->ai_family == AF_INET6
-		      || req->ai_family == AF_UNSPEC)
-		    {
-		      gethosts (AF_INET6);
-		      no_inet6_data = no_data;
-		      inet6_status = status;
-		    }
-		  if (req->ai_family == AF_INET
-		      || req->ai_family == AF_UNSPEC
-		      || (req->ai_family == AF_INET6
-			  && (req->ai_flags & AI_V4MAPPED)
-			  /* Avoid generating the mapped addresses if we
-			     know we are not going to need them.  */
-			  && ((req->ai_flags & AI_ALL) || !res.got_ipv6)))
-		    {
-		      gethosts (AF_INET);
-
-		      if (req->ai_family == AF_INET)
-			{
-			  no_inet6_data = no_data;
-			  inet6_status = status;
-			}
-		    }
-
-		  /* If we found one address for AF_INET or AF_INET6,
-		     don't continue the search.  */
-		  if (inet6_status == NSS_STATUS_SUCCESS
-		      || status == NSS_STATUS_SUCCESS)
-		    {
-		      if ((req->ai_flags & AI_CANONNAME) != 0
-			  && res.canon == NULL)
-			{
-			  char *canonbuf = getcanonname (nip, res.at, name);
-			  if (canonbuf == NULL)
-			    {
-			      __resolv_context_put (res_ctx);
-			      result = -EAI_MEMORY;
-			      goto free_and_return;
-			    }
-			  res.canon = canonbuf;
-			}
-		      status = NSS_STATUS_SUCCESS;
-		    }
-		  else
-		    {
-		      /* We can have different states for AF_INET and
-			 AF_INET6.  Try to find a useful one for both.  */
-		      if (inet6_status == NSS_STATUS_TRYAGAIN)
-			status = NSS_STATUS_TRYAGAIN;
-		      else if (status == NSS_STATUS_UNAVAIL
-			       && inet6_status != NSS_STATUS_UNAVAIL)
-			status = inet6_status;
-		    }
-		}
-	      else
-		{
-		  /* Could not locate any of the lookup functions.
-		     The NSS lookup code does not consistently set
-		     errno, so we need to supply our own error
-		     code here.  The root cause could either be a
-		     resource allocation failure, or a missing
-		     service function in the DSO (so it should not
-		     be listed in /etc/nsswitch.conf).  Assume the
-		     former, and return EBUSY.  */
-		  status = NSS_STATUS_UNAVAIL;
-		  __set_h_errno (NETDB_INTERNAL);
-		  __set_errno (EBUSY);
-		}
-	    }
-
-	  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;
-	}
-
-      __resolv_context_put (res_ctx);
-
-      /* If we have a failure which sets errno, report it using
-	 EAI_SYSTEM.  */
-      if ((status == NSS_STATUS_TRYAGAIN || status == NSS_STATUS_UNAVAIL)
-	  && h_errno == NETDB_INTERNAL)
-	{
-	  result = -EAI_SYSTEM;
-	  goto free_and_return;
-	}
-
-      if (no_data != 0 && no_inet6_data != 0)
-	{
-	  /* If both requests timed out report this.  */
-	  if (no_data == EAI_AGAIN && no_inet6_data == EAI_AGAIN)
-	    result = -EAI_AGAIN;
-	  else
-	    /* We made requests but they turned out no data.  The name
-	       is known, though.  */
-	    result = -EAI_NODATA;
-
-	  goto free_and_return;
-	}
+      if ((result = get_nss_addresses (name, req, tmpbuf, &res)) != 0)
+	goto free_and_return;
+      else if (res.at != NULL)
+	goto process_list;
 
-    process_list:
-      if (res.at == NULL)
-	{
-	  result = -EAI_NONAME;
-	  goto free_and_return;
-	}
+      /* None of the lookups worked, so name not found.  */
+      result = -EAI_NONAME;
+      goto free_and_return;
     }
   else
     {
@@ -1089,6 +1097,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	}
     }
 
+process_list:
   {
     /* Set up the canonical name if we need it.  */
     if ((result = process_canonname (req, orig_name, &res)) != 0)
-- 
2.41.0


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

* [pushed 2.34 10/14] gaih_inet: make gethosts into a function
  2023-09-16  0:48 [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
                   ` (8 preceding siblings ...)
  2023-09-16  0:48 ` [pushed 2.34 09/14] gaih_inet: separate nss lookup loop " Siddhesh Poyarekar
@ 2023-09-16  0:48 ` Siddhesh Poyarekar
  2023-09-16  0:48 ` [pushed 2.34 11/14] gaih_inet: split loopback lookup into its own function Siddhesh Poyarekar
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-16  0:48 UTC (permalink / raw)
  To: libc-stable; +Cc: DJ Delorie

The macro is quite a pain to debug, so make gethosts into a function to
make it easier to maintain.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit cfa3bd48cb19a70e4367a9978dbba09d9df27a72)
---
 sysdeps/posix/getaddrinfo.c | 117 ++++++++++++++++++------------------
 1 file changed, 59 insertions(+), 58 deletions(-)

diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 145ea6fa38..6be109d07f 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -268,63 +268,54 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, int family,
   return true;
 }
 
-#define gethosts(_family) \
- {									      \
-  struct hostent th;							      \
-  char *localcanon = NULL;						      \
-  no_data = 0;								      \
-  while (1)								      \
-    {									      \
-      status = DL_CALL_FCT (fct, (name, _family, &th,			      \
-				  tmpbuf->data, tmpbuf->length,		      \
-				  &errno, &h_errno, NULL, &localcanon));      \
-      if (status != NSS_STATUS_TRYAGAIN || h_errno != NETDB_INTERNAL	      \
-	  || errno != ERANGE)						      \
-	break;								      \
-      if (!scratch_buffer_grow (tmpbuf))				      \
-	{								      \
-	  __resolv_context_put (res_ctx);				      \
-	  result = -EAI_MEMORY;						      \
-	  goto out;							      \
-	}								      \
-    }									      \
-  if (status == NSS_STATUS_NOTFOUND					      \
-      || status == NSS_STATUS_TRYAGAIN || status == NSS_STATUS_UNAVAIL)	      \
-    {									      \
-      if (h_errno == NETDB_INTERNAL)					      \
-	{								      \
-	  __resolv_context_put (res_ctx);				      \
-	  result = -EAI_SYSTEM;						      \
-	  goto out;							      \
-	}								      \
-      if (h_errno == TRY_AGAIN)						      \
-	no_data = EAI_AGAIN;						      \
-      else								      \
-	no_data = h_errno == NO_DATA;					      \
-    }									      \
-  else if (status == NSS_STATUS_SUCCESS)				      \
-    {									      \
-      if (!convert_hostent_to_gaih_addrtuple (req, _family, &th, res))	      \
-	{								      \
-	  __resolv_context_put (res_ctx);				      \
-	  result = -EAI_SYSTEM;						      \
-	  goto out;							      \
-	}								      \
-									      \
-      if (localcanon != NULL && res->canon == NULL)			      \
-	{								      \
-	  char *canonbuf = __strdup (localcanon);			      \
-	  if (canonbuf == NULL)						      \
-	    {								      \
-	      __resolv_context_put (res_ctx);				      \
-	      result = -EAI_SYSTEM;					      \
-	      goto out;							      \
-	    }								      \
-	  res->canon = canonbuf;					      \
-	}								      \
-    }									      \
- }
+static int
+gethosts (nss_gethostbyname3_r fct, int family, const char *name,
+	  const struct addrinfo *req, struct scratch_buffer *tmpbuf,
+	  struct gaih_result *res, enum nss_status *statusp, int *no_datap)
+{
+  struct hostent th;
+  char *localcanon = NULL;
+  enum nss_status status;
+
+  *no_datap = 0;
+  while (1)
+    {
+      *statusp = status = DL_CALL_FCT (fct, (name, family, &th,
+					     tmpbuf->data, tmpbuf->length,
+					     &errno, &h_errno, NULL,
+					     &localcanon));
+      if (status != NSS_STATUS_TRYAGAIN || h_errno != NETDB_INTERNAL
+	  || errno != ERANGE)
+	break;
+      if (!scratch_buffer_grow (tmpbuf))
+	return -EAI_MEMORY;
+    }
+  if (status == NSS_STATUS_NOTFOUND
+      || status == NSS_STATUS_TRYAGAIN || status == NSS_STATUS_UNAVAIL)
+    {
+      if (h_errno == NETDB_INTERNAL)
+	return -EAI_SYSTEM;
+      if (h_errno == TRY_AGAIN)
+	*no_datap = EAI_AGAIN;
+      else
+	*no_datap = h_errno == NO_DATA;
+    }
+  else if (status == NSS_STATUS_SUCCESS)
+    {
+      if (!convert_hostent_to_gaih_addrtuple (req, family, &th, res))
+	return -EAI_SYSTEM;
+
+      if (localcanon != NULL && res->canon == NULL)
+	{
+	  char *canonbuf = __strdup (localcanon);
+	  if (canonbuf == NULL)
+	    return  -EAI_SYSTEM;
+	  res->canon = canonbuf;
+	}
+    }
 
+  return 0;
+}
 
 /* This function is called if a canonical name is requested, but if
    the service function did not provide it.  It tries to obtain the
@@ -741,7 +732,12 @@ get_nss_addresses (const char *name, const struct addrinfo *req,
 	      if (req->ai_family == AF_INET6
 		  || req->ai_family == AF_UNSPEC)
 		{
-		  gethosts (AF_INET6);
+		  if ((result = gethosts (fct, AF_INET6, name, req, tmpbuf,
+					  res, &status, &no_data)) != 0)
+		    {
+		      __resolv_context_put (res_ctx);
+		      goto out;
+		    }
 		  no_inet6_data = no_data;
 		  inet6_status = status;
 		}
@@ -753,7 +749,12 @@ get_nss_addresses (const char *name, const struct addrinfo *req,
 			 know we are not going to need them.  */
 		      && ((req->ai_flags & AI_ALL) || !res->got_ipv6)))
 		{
-		  gethosts (AF_INET);
+		  if ((result = gethosts (fct, AF_INET, name, req, tmpbuf,
+					  res, &status, &no_data)) != 0)
+		    {
+		      __resolv_context_put (res_ctx);
+		      goto out;
+		    }
 
 		  if (req->ai_family == AF_INET)
 		    {
-- 
2.41.0


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

* [pushed 2.34 11/14] gaih_inet: split loopback lookup into its own function
  2023-09-16  0:48 [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
                   ` (9 preceding siblings ...)
  2023-09-16  0:48 ` [pushed 2.34 10/14] gaih_inet: make gethosts into a function Siddhesh Poyarekar
@ 2023-09-16  0:48 ` Siddhesh Poyarekar
  2023-09-16  0:48 ` [pushed 2.34 12/14] gaih_inet: Split result generation " Siddhesh Poyarekar
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-16  0:48 UTC (permalink / raw)
  To: libc-stable; +Cc: DJ Delorie

Flatten the condition nesting and replace the alloca for RET.AT/ATR with
a single array LOCAL_AT[2].  This gets rid of alloca and alloca
accounting.

`git diff -b` is probably the best way to view this change since much of
the diff is whitespace changes.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit 657472b2a50f67b12e5bbe5827582c9c2bb82dc3)
---
 sysdeps/posix/getaddrinfo.c | 127 ++++++++++++++++++------------------
 1 file changed, 62 insertions(+), 65 deletions(-)

diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 6be109d07f..827c43b369 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -1004,6 +1004,32 @@ try_simple_gethostbyname (const char *name, const struct addrinfo *req,
   return -EAI_NODATA;
 }
 
+/* Add local address information into RES.  RES->AT is assumed to have enough
+   space for two tuples and is zeroed out.  */
+
+static void
+get_local_addresses (const struct addrinfo *req, struct gaih_result *res)
+{
+  struct gaih_addrtuple *atr = res->at;
+  if (req->ai_family == AF_UNSPEC)
+    res->at->next = res->at + 1;
+
+  if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET6)
+    {
+      res->at->family = AF_INET6;
+      if ((req->ai_flags & AI_PASSIVE) == 0)
+	memcpy (res->at->addr, &in6addr_loopback, sizeof (struct in6_addr));
+      atr = res->at->next;
+    }
+
+  if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
+    {
+      atr->family = AF_INET;
+      if ((req->ai_flags & AI_PASSIVE) == 0)
+	atr->addr[0] = htonl (INADDR_LOOPBACK);
+    }
+}
+
 static int
 gaih_inet (const char *name, const struct gaih_service *service,
 	   const struct addrinfo *req, struct addrinfo **pai,
@@ -1014,10 +1040,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
   const char *orig_name = name;
 
-  /* Reserve stack memory for the scratch buffer in the getaddrinfo
-     function.  */
-  size_t alloca_used = sizeof (struct scratch_buffer);
-
   int rc;
   if ((rc = get_servtuples (service, req, st, tmpbuf)) != 0)
     return rc;
@@ -1027,76 +1049,51 @@ gaih_inet (const char *name, const struct gaih_service *service,
   int result = 0;
 
   struct gaih_result res = {0};
-  if (name != NULL)
+  struct gaih_addrtuple local_at[2] = {0};
+
+  res.at = local_at;
+
+  if (__glibc_unlikely (name == NULL))
     {
-      if (req->ai_flags & AI_IDN)
-	{
-	  char *out;
-	  result = __idna_to_dns_encoding (name, &out);
-	  if (result != 0)
-	    return -result;
-	  name = out;
-	  malloc_name = true;
-	}
+      get_local_addresses (req, &res);
+      goto process_list;
+    }
 
-      res.at = alloca_account (sizeof (struct gaih_addrtuple), alloca_used);
-      res.at->scopeid = 0;
-      res.at->next = NULL;
+  if (req->ai_flags & AI_IDN)
+    {
+      char *out;
+      result = __idna_to_dns_encoding (name, &out);
+      if (result != 0)
+	return -result;
+      name = out;
+      malloc_name = true;
+    }
 
-      if ((result = text_to_binary_address (name, req, &res)) != 0)
-	goto free_and_return;
-      else if (res.at != NULL)
-	goto process_list;
+  if ((result = text_to_binary_address (name, req, &res)) != 0)
+    goto free_and_return;
+  else if (res.at != NULL)
+    goto process_list;
 
-      if ((result = try_simple_gethostbyname (name, req, tmpbuf, &res)) != 0)
-	goto free_and_return;
-      else if (res.at != NULL)
-	goto process_list;
+  if ((result = try_simple_gethostbyname (name, req, tmpbuf, &res)) != 0)
+    goto free_and_return;
+  else if (res.at != NULL)
+    goto process_list;
 
 #ifdef USE_NSCD
-      if ((result = get_nscd_addresses (name, req, &res)) != 0)
-	goto free_and_return;
-      else if (res.at != NULL)
-	goto process_list;
+  if ((result = get_nscd_addresses (name, req, &res)) != 0)
+    goto free_and_return;
+  else if (res.at != NULL)
+    goto process_list;
 #endif
 
-      if ((result = get_nss_addresses (name, req, tmpbuf, &res)) != 0)
-	goto free_and_return;
-      else if (res.at != NULL)
-	goto process_list;
-
-      /* None of the lookups worked, so name not found.  */
-      result = -EAI_NONAME;
-      goto free_and_return;
-    }
-  else
-    {
-      struct gaih_addrtuple *atr;
-      atr = res.at = alloca_account (sizeof (struct gaih_addrtuple),
-				     alloca_used);
-      memset (res.at, '\0', sizeof (struct gaih_addrtuple));
-
-      if (req->ai_family == AF_UNSPEC)
-	{
-	  res.at->next = __alloca (sizeof (struct gaih_addrtuple));
-	  memset (res.at->next, '\0', sizeof (struct gaih_addrtuple));
-	}
-
-      if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET6)
-	{
-	  res.at->family = AF_INET6;
-	  if ((req->ai_flags & AI_PASSIVE) == 0)
-	    memcpy (res.at->addr, &in6addr_loopback, sizeof (struct in6_addr));
-	  atr = res.at->next;
-	}
+  if ((result = get_nss_addresses (name, req, tmpbuf, &res)) != 0)
+    goto free_and_return;
+  else if (res.at != NULL)
+    goto process_list;
 
-      if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
-	{
-	  atr->family = AF_INET;
-	  if ((req->ai_flags & AI_PASSIVE) == 0)
-	    atr->addr[0] = htonl (INADDR_LOOPBACK);
-	}
-    }
+  /* None of the lookups worked, so name not found.  */
+  result = -EAI_NONAME;
+  goto free_and_return;
 
 process_list:
   {
-- 
2.41.0


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

* [pushed 2.34 12/14] gaih_inet: Split result generation into its own function
  2023-09-16  0:48 [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
                   ` (10 preceding siblings ...)
  2023-09-16  0:48 ` [pushed 2.34 11/14] gaih_inet: split loopback lookup into its own function Siddhesh Poyarekar
@ 2023-09-16  0:48 ` Siddhesh Poyarekar
  2023-09-16  0:49 ` [pushed 2.34 13/14] gethosts: Return EAI_MEMORY on allocation failure Siddhesh Poyarekar
  2023-09-16  0:49 ` [pushed 2.34 14/14] getaddrinfo: Fix use after free in getcanonname (CVE-2023-4806) Siddhesh Poyarekar
  13 siblings, 0 replies; 15+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-16  0:48 UTC (permalink / raw)
  To: libc-stable; +Cc: DJ Delorie

Simplify the loop a wee bit and clean up variable names too.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit ac4653ef503d1e87893d1a6714748a1cdf4bf7ad)
---
 sysdeps/posix/getaddrinfo.c | 176 ++++++++++++++++++------------------
 1 file changed, 86 insertions(+), 90 deletions(-)

diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 827c43b369..1008f24736 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -1030,6 +1030,87 @@ get_local_addresses (const struct addrinfo *req, struct gaih_result *res)
     }
 }
 
+/* Generate results in PAI and its count in NADDRS.  Return 0 on success or an
+   error code on failure.  */
+
+static int
+generate_addrinfo (const struct addrinfo *req, struct gaih_result *res,
+		   const struct gaih_servtuple *st, struct addrinfo **pai,
+		   unsigned int *naddrs)
+{
+  size_t socklen;
+  sa_family_t family;
+
+  /* Buffer is the size of an unformatted IPv6 address in printable format.  */
+  for (struct gaih_addrtuple *at = res->at; at != NULL; at = at->next)
+    {
+      family = at->family;
+      if (family == AF_INET6)
+	{
+	  socklen = sizeof (struct sockaddr_in6);
+
+	  /* If we looked up IPv4 mapped address discard them here if
+	     the caller isn't interested in all address and we have
+	     found at least one IPv6 address.  */
+	  if (res->got_ipv6
+	      && (req->ai_flags & (AI_V4MAPPED|AI_ALL)) == AI_V4MAPPED
+	      && IN6_IS_ADDR_V4MAPPED (at->addr))
+	    continue;
+	}
+      else
+	socklen = sizeof (struct sockaddr_in);
+
+      for (int i = 0; st[i].set; i++)
+	{
+	  struct addrinfo *ai;
+	  ai = *pai = malloc (sizeof (struct addrinfo) + socklen);
+	  if (ai == NULL)
+	    return -EAI_MEMORY;
+
+	  ai->ai_flags = req->ai_flags;
+	  ai->ai_family = family;
+	  ai->ai_socktype = st[i].socktype;
+	  ai->ai_protocol = st[i].protocol;
+	  ai->ai_addrlen = socklen;
+	  ai->ai_addr = (void *) (ai + 1);
+
+	  /* We only add the canonical name once.  */
+	  ai->ai_canonname = res->canon;
+	  res->canon = NULL;
+
+#ifdef _HAVE_SA_LEN
+	  ai->ai_addr->sa_len = socklen;
+#endif /* _HAVE_SA_LEN */
+	  ai->ai_addr->sa_family = family;
+
+	  /* In case of an allocation error the list must be NULL
+	     terminated.  */
+	  ai->ai_next = NULL;
+
+	  if (family == AF_INET6)
+	    {
+	      struct sockaddr_in6 *sin6p = (struct sockaddr_in6 *) ai->ai_addr;
+	      sin6p->sin6_port = st[i].port;
+	      sin6p->sin6_flowinfo = 0;
+	      memcpy (&sin6p->sin6_addr, at->addr, sizeof (struct in6_addr));
+	      sin6p->sin6_scope_id = at->scopeid;
+	    }
+	  else
+	    {
+	      struct sockaddr_in *sinp = (struct sockaddr_in *) ai->ai_addr;
+	      sinp->sin_port = st[i].port;
+	      memcpy (&sinp->sin_addr, at->addr, sizeof (struct in_addr));
+	      memset (sinp->sin_zero, '\0', sizeof (sinp->sin_zero));
+	    }
+
+	  pai = &(ai->ai_next);
+	}
+
+      ++*naddrs;
+    }
+  return 0;
+}
+
 static int
 gaih_inet (const char *name, const struct gaih_service *service,
 	   const struct addrinfo *req, struct addrinfo **pai,
@@ -1096,98 +1177,13 @@ gaih_inet (const char *name, const struct gaih_service *service,
   goto free_and_return;
 
 process_list:
-  {
-    /* Set up the canonical name if we need it.  */
-    if ((result = process_canonname (req, orig_name, &res)) != 0)
-      goto free_and_return;
-
-    struct gaih_addrtuple *at2 = res.at;
-    size_t socklen;
-    sa_family_t family;
-
-    /*
-      buffer is the size of an unformatted IPv6 address in printable format.
-     */
-    while (at2 != NULL)
-      {
-	family = at2->family;
-	if (family == AF_INET6)
-	  {
-	    socklen = sizeof (struct sockaddr_in6);
-
-	    /* If we looked up IPv4 mapped address discard them here if
-	       the caller isn't interested in all address and we have
-	       found at least one IPv6 address.  */
-	    if (res.got_ipv6
-		&& (req->ai_flags & (AI_V4MAPPED|AI_ALL)) == AI_V4MAPPED
-		&& IN6_IS_ADDR_V4MAPPED (at2->addr))
-	      goto ignore;
-	  }
-	else
-	  socklen = sizeof (struct sockaddr_in);
-
-	for (int i = 0; st[i].set; i++)
-	  {
-	    struct addrinfo *ai;
-	    ai = *pai = malloc (sizeof (struct addrinfo) + socklen);
-	    if (ai == NULL)
-	      {
-		result = -EAI_MEMORY;
-		goto free_and_return;
-	      }
-
-	    ai->ai_flags = req->ai_flags;
-	    ai->ai_family = family;
-	    ai->ai_socktype = st[i].socktype;
-	    ai->ai_protocol = st[i].protocol;
-	    ai->ai_addrlen = socklen;
-	    ai->ai_addr = (void *) (ai + 1);
-
-	    /* We only add the canonical name once.  */
-	    ai->ai_canonname = res.canon;
-	    res.canon = NULL;
-
-#ifdef _HAVE_SA_LEN
-	    ai->ai_addr->sa_len = socklen;
-#endif /* _HAVE_SA_LEN */
-	    ai->ai_addr->sa_family = family;
-
-	    /* In case of an allocation error the list must be NULL
-	       terminated.  */
-	    ai->ai_next = NULL;
-
-	    if (family == AF_INET6)
-	      {
-		struct sockaddr_in6 *sin6p =
-		  (struct sockaddr_in6 *) ai->ai_addr;
-
-		sin6p->sin6_port = st[i].port;
-		sin6p->sin6_flowinfo = 0;
-		memcpy (&sin6p->sin6_addr,
-			at2->addr, sizeof (struct in6_addr));
-		sin6p->sin6_scope_id = at2->scopeid;
-	      }
-	    else
-	      {
-		struct sockaddr_in *sinp =
-		  (struct sockaddr_in *) ai->ai_addr;
-		sinp->sin_port = st[i].port;
-		memcpy (&sinp->sin_addr,
-			at2->addr, sizeof (struct in_addr));
-		memset (sinp->sin_zero, '\0', sizeof (sinp->sin_zero));
-	      }
-
-	    pai = &(ai->ai_next);
-	  }
-
-	++*naddrs;
+  /* Set up the canonical name if we need it.  */
+  if ((result = process_canonname (req, orig_name, &res)) != 0)
+    goto free_and_return;
 
-      ignore:
-	at2 = at2->next;
-      }
-  }
+  result = generate_addrinfo (req, &res, st, pai, naddrs);
 
- free_and_return:
+free_and_return:
   if (malloc_name)
     free ((char *) name);
   free (addrmem);
-- 
2.41.0


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

* [pushed 2.34 13/14] gethosts: Return EAI_MEMORY on allocation failure
  2023-09-16  0:48 [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
                   ` (11 preceding siblings ...)
  2023-09-16  0:48 ` [pushed 2.34 12/14] gaih_inet: Split result generation " Siddhesh Poyarekar
@ 2023-09-16  0:49 ` Siddhesh Poyarekar
  2023-09-16  0:49 ` [pushed 2.34 14/14] getaddrinfo: Fix use after free in getcanonname (CVE-2023-4806) Siddhesh Poyarekar
  13 siblings, 0 replies; 15+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-16  0:49 UTC (permalink / raw)
  To: libc-stable; +Cc: DJ Delorie

All other cases of failures due to lack of memory return EAI_MEMORY, so
it seems wrong to return EAI_SYSTEM here.  The only reason
convert_hostent_to_gaih_addrtuple could fail is on calloc failure.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit b587456c0e7b59dcfdbd2d44db000a3bc8244e57)
---
 sysdeps/posix/getaddrinfo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 1008f24736..37260d6e6f 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -303,13 +303,13 @@ gethosts (nss_gethostbyname3_r fct, int family, const char *name,
   else if (status == NSS_STATUS_SUCCESS)
     {
       if (!convert_hostent_to_gaih_addrtuple (req, family, &th, res))
-	return -EAI_SYSTEM;
+	return -EAI_MEMORY;
 
       if (localcanon != NULL && res->canon == NULL)
 	{
 	  char *canonbuf = __strdup (localcanon);
 	  if (canonbuf == NULL)
-	    return  -EAI_SYSTEM;
+	    return  -EAI_MEMORY;
 	  res->canon = canonbuf;
 	}
     }
-- 
2.41.0


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

* [pushed 2.34 14/14] getaddrinfo: Fix use after free in getcanonname (CVE-2023-4806)
  2023-09-16  0:48 [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
                   ` (12 preceding siblings ...)
  2023-09-16  0:49 ` [pushed 2.34 13/14] gethosts: Return EAI_MEMORY on allocation failure Siddhesh Poyarekar
@ 2023-09-16  0:49 ` Siddhesh Poyarekar
  13 siblings, 0 replies; 15+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-16  0:49 UTC (permalink / raw)
  To: libc-stable

When an NSS plugin only implements the _gethostbyname2_r and
_getcanonname_r callbacks, getaddrinfo could use memory that was freed
during tmpbuf resizing, through h_name in a previous query response.

The backing store for res->at->name when doing a query with
gethostbyname3_r or gethostbyname2_r is tmpbuf, which is reallocated in
gethosts during the query.  For AF_INET6 lookup with AI_ALL |
AI_V4MAPPED, gethosts gets called twice, once for a v6 lookup and second
for a v4 lookup.  In this case, if the first call reallocates tmpbuf
enough number of times, resulting in a malloc, th->h_name (that
res->at->name refers to) ends up on a heap allocated storage in tmpbuf.
Now if the second call to gethosts also causes the plugin callback to
return NSS_STATUS_TRYAGAIN, tmpbuf will get freed, resulting in a UAF
reference in res->at->name.  This then gets dereferenced in the
getcanonname_r plugin call, resulting in the use after free.

Fix this by copying h_name over and freeing it at the end.  This
resolves BZ #30843, which is assigned CVE-2023-4806.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
(cherry picked from commit 973fe93a5675c42798b2161c6f29c01b0e243994)
---
 nss/Makefile                                  | 14 ++++-
 nss/nss_test_gai_hv2_canonname.c              | 56 +++++++++++++++++
 nss/tst-nss-gai-hv2-canonname.c               | 63 +++++++++++++++++++
 nss/tst-nss-gai-hv2-canonname.h               |  1 +
 .../postclean.req                             |  0
 .../tst-nss-gai-hv2-canonname.script          |  2 +
 sysdeps/posix/getaddrinfo.c                   | 25 +++++---
 7 files changed, 151 insertions(+), 10 deletions(-)
 create mode 100644 nss/nss_test_gai_hv2_canonname.c
 create mode 100644 nss/tst-nss-gai-hv2-canonname.c
 create mode 100644 nss/tst-nss-gai-hv2-canonname.h
 create mode 100644 nss/tst-nss-gai-hv2-canonname.root/postclean.req
 create mode 100644 nss/tst-nss-gai-hv2-canonname.root/tst-nss-gai-hv2-canonname.script

diff --git a/nss/Makefile b/nss/Makefile
index 5192db6097..18a27d79d2 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -80,6 +80,7 @@ tests-container := \
   tst-nss-test3 \
   tst-reload1 \
   tst-reload2 \
+  tst-nss-gai-hv2-canonname \
 # tests-container
 
 # Tests which need libdl
@@ -143,7 +144,8 @@ libnss_compat-inhibit-o	= $(filter-out .os,$(object-suffixes))
 ifeq ($(build-static-nss),yes)
 tests-static		+= tst-nss-static
 endif
-extra-test-objs		+= nss_test1.os nss_test2.os nss_test_errno.os
+extra-test-objs		+= nss_test1.os nss_test2.os nss_test_errno.os \
+			   nss_test_gai_hv2_canonname.os
 
 include ../Rules
 
@@ -178,12 +180,16 @@ rtld-tests-LDFLAGS += -Wl,--dynamic-list=nss_test.ver
 libof-nss_test1 = extramodules
 libof-nss_test2 = extramodules
 libof-nss_test_errno = extramodules
+libof-nss_test_gai_hv2_canonname = extramodules
 $(objpfx)/libnss_test1.so: $(objpfx)nss_test1.os $(link-libc-deps)
 	$(build-module)
 $(objpfx)/libnss_test2.so: $(objpfx)nss_test2.os $(link-libc-deps)
 	$(build-module)
 $(objpfx)/libnss_test_errno.so: $(objpfx)nss_test_errno.os $(link-libc-deps)
 	$(build-module)
+$(objpfx)/libnss_test_gai_hv2_canonname.so: \
+  $(objpfx)nss_test_gai_hv2_canonname.os $(link-libc-deps)
+	$(build-module)
 $(objpfx)nss_test2.os : nss_test1.c
 # Use the nss_files suffix for these objects as well.
 $(objpfx)/libnss_test1.so$(libnss_files.so-version): $(objpfx)/libnss_test1.so
@@ -193,10 +199,14 @@ $(objpfx)/libnss_test2.so$(libnss_files.so-version): $(objpfx)/libnss_test2.so
 $(objpfx)/libnss_test_errno.so$(libnss_files.so-version): \
   $(objpfx)/libnss_test_errno.so
 	$(make-link)
+$(objpfx)/libnss_test_gai_hv2_canonname.so$(libnss_files.so-version): \
+  $(objpfx)/libnss_test_gai_hv2_canonname.so
+	$(make-link)
 $(patsubst %,$(objpfx)%.out,$(tests) $(tests-container)) : \
 	$(objpfx)/libnss_test1.so$(libnss_files.so-version) \
 	$(objpfx)/libnss_test2.so$(libnss_files.so-version) \
-	$(objpfx)/libnss_test_errno.so$(libnss_files.so-version)
+	$(objpfx)/libnss_test_errno.so$(libnss_files.so-version) \
+	$(objpfx)/libnss_test_gai_hv2_canonname.so$(libnss_files.so-version)
 
 ifeq (yes,$(have-thread-library))
 $(objpfx)tst-cancel-getpwuid_r: $(shared-thread-library)
diff --git a/nss/nss_test_gai_hv2_canonname.c b/nss/nss_test_gai_hv2_canonname.c
new file mode 100644
index 0000000000..4439c83c9f
--- /dev/null
+++ b/nss/nss_test_gai_hv2_canonname.c
@@ -0,0 +1,56 @@
+/* NSS service provider that only provides gethostbyname2_r.
+   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 <nss.h>
+#include <stdlib.h>
+#include <string.h>
+#include "nss/tst-nss-gai-hv2-canonname.h"
+
+/* Catch misnamed and functions.  */
+#pragma GCC diagnostic error "-Wmissing-prototypes"
+NSS_DECLARE_MODULE_FUNCTIONS (test_gai_hv2_canonname)
+
+extern enum nss_status _nss_files_gethostbyname2_r (const char *, int,
+						    struct hostent *, char *,
+						    size_t, int *, int *);
+
+enum nss_status
+_nss_test_gai_hv2_canonname_gethostbyname2_r (const char *name, int af,
+					      struct hostent *result,
+					      char *buffer, size_t buflen,
+					      int *errnop, int *herrnop)
+{
+  return _nss_files_gethostbyname2_r (name, af, result, buffer, buflen, errnop,
+				      herrnop);
+}
+
+enum nss_status
+_nss_test_gai_hv2_canonname_getcanonname_r (const char *name, char *buffer,
+					    size_t buflen, char **result,
+					    int *errnop, int *h_errnop)
+{
+  /* We expect QUERYNAME, which is a small enough string that it shouldn't fail
+     the test.  */
+  if (memcmp (QUERYNAME, name, sizeof (QUERYNAME))
+      || buflen < sizeof (QUERYNAME))
+    abort ();
+
+  strncpy (buffer, name, buflen);
+  *result = buffer;
+  return NSS_STATUS_SUCCESS;
+}
diff --git a/nss/tst-nss-gai-hv2-canonname.c b/nss/tst-nss-gai-hv2-canonname.c
new file mode 100644
index 0000000000..d5f10c07d6
--- /dev/null
+++ b/nss/tst-nss-gai-hv2-canonname.c
@@ -0,0 +1,63 @@
+/* Test NSS query path for plugins that only implement gethostbyname2
+   (#30843).
+   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 <nss.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+#include <support/xstdio.h>
+#include "nss/tst-nss-gai-hv2-canonname.h"
+
+#define PREPARE do_prepare
+
+static void do_prepare (int a, char **av)
+{
+  FILE *hosts = xfopen ("/etc/hosts", "w");
+  for (unsigned i = 2; i < 255; i++)
+    {
+      fprintf (hosts, "ff01::ff02:ff03:%u:2\ttest.example.com\n", i);
+      fprintf (hosts, "192.168.0.%u\ttest.example.com\n", i);
+    }
+  xfclose (hosts);
+}
+
+static int
+do_test (void)
+{
+  __nss_configure_lookup ("hosts", "test_gai_hv2_canonname");
+
+  struct addrinfo hints = {};
+  struct addrinfo *result = NULL;
+
+  hints.ai_family = AF_INET6;
+  hints.ai_flags = AI_ALL | AI_V4MAPPED | AI_CANONNAME;
+
+  int ret = getaddrinfo (QUERYNAME, NULL, &hints, &result);
+
+  if (ret != 0)
+    FAIL_EXIT1 ("getaddrinfo failed: %s\n", gai_strerror (ret));
+
+  TEST_COMPARE_STRING (result->ai_canonname, QUERYNAME);
+
+  freeaddrinfo(result);
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/nss/tst-nss-gai-hv2-canonname.h b/nss/tst-nss-gai-hv2-canonname.h
new file mode 100644
index 0000000000..14f2a9cb08
--- /dev/null
+++ b/nss/tst-nss-gai-hv2-canonname.h
@@ -0,0 +1 @@
+#define QUERYNAME "test.example.com"
diff --git a/nss/tst-nss-gai-hv2-canonname.root/postclean.req b/nss/tst-nss-gai-hv2-canonname.root/postclean.req
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/nss/tst-nss-gai-hv2-canonname.root/tst-nss-gai-hv2-canonname.script b/nss/tst-nss-gai-hv2-canonname.root/tst-nss-gai-hv2-canonname.script
new file mode 100644
index 0000000000..31848b4a28
--- /dev/null
+++ b/nss/tst-nss-gai-hv2-canonname.root/tst-nss-gai-hv2-canonname.script
@@ -0,0 +1,2 @@
+cp $B/nss/libnss_test_gai_hv2_canonname.so $L/libnss_test_gai_hv2_canonname.so.2
+su
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 37260d6e6f..10dc63542f 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -120,6 +120,7 @@ struct gaih_result
 {
   struct gaih_addrtuple *at;
   char *canon;
+  char *h_name;
   bool free_at;
   bool got_ipv6;
 };
@@ -165,6 +166,7 @@ gaih_result_reset (struct gaih_result *res)
   if (res->free_at)
     free (res->at);
   free (res->canon);
+  free (res->h_name);
   memset (res, 0, sizeof (*res));
 }
 
@@ -203,9 +205,8 @@ gaih_inet_serv (const char *servicename, const struct gaih_typeproto *tp,
   return 0;
 }
 
-/* Convert struct hostent to a list of struct gaih_addrtuple objects.  h_name
-   is not copied, and the struct hostent object must not be deallocated
-   prematurely.  The new addresses are appended to the tuple array in RES.  */
+/* Convert struct hostent to a list of struct gaih_addrtuple objects.  The new
+   addresses are appended to the tuple array in RES.  */
 static bool
 convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, int family,
 				   struct hostent *h, struct gaih_result *res)
@@ -238,6 +239,15 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, int family,
   res->at = array;
   res->free_at = true;
 
+  /* Duplicate h_name because it may get reclaimed when the underlying storage
+     is freed.  */
+  if (res->h_name == NULL)
+    {
+      res->h_name = __strdup (h->h_name);
+      if (res->h_name == NULL)
+	return false;
+    }
+
   /* Update the next pointers on reallocation.  */
   for (size_t i = 0; i < old; i++)
     array[i].next = array + i + 1;
@@ -262,7 +272,6 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, int family,
 	}
       array[i].next = array + i + 1;
     }
-  array[0].name = h->h_name;
   array[count - 1].next = NULL;
 
   return true;
@@ -324,15 +333,15 @@ gethosts (nss_gethostbyname3_r fct, int family, const char *name,
    memory allocation failure.  The returned string is allocated on the
    heap; the caller has to free it.  */
 static char *
-getcanonname (nss_action_list nip, struct gaih_addrtuple *at, const char *name)
+getcanonname (nss_action_list nip, const char *hname, const char *name)
 {
   nss_getcanonname_r *cfct = __nss_lookup_function (nip, "getcanonname_r");
   char *s = (char *) name;
   if (cfct != NULL)
     {
       char buf[256];
-      if (DL_CALL_FCT (cfct, (at->name ?: name, buf, sizeof (buf),
-			      &s, &errno, &h_errno)) != NSS_STATUS_SUCCESS)
+      if (DL_CALL_FCT (cfct, (hname ?: name, buf, sizeof (buf), &s, &errno,
+			      &h_errno)) != NSS_STATUS_SUCCESS)
 	/* If the canonical name cannot be determined, use the passed
 	   string.  */
 	s = (char *) name;
@@ -771,7 +780,7 @@ get_nss_addresses (const char *name, const struct addrinfo *req,
 		  if ((req->ai_flags & AI_CANONNAME) != 0
 		      && res->canon == NULL)
 		    {
-		      char *canonbuf = getcanonname (nip, res->at, name);
+		      char *canonbuf = getcanonname (nip, res->h_name, name);
 		      if (canonbuf == NULL)
 			{
 			  __resolv_context_put (res_ctx);
-- 
2.41.0


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

end of thread, other threads:[~2023-09-16  0:49 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-16  0:48 [pushed 2.34 00/14] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
2023-09-16  0:48 ` [pushed 2.34 01/14] nss: Sort tests and tests-container and put one test per line Siddhesh Poyarekar
2023-09-16  0:48 ` [pushed 2.34 02/14] gethosts: Remove unused argument _type Siddhesh Poyarekar
2023-09-16  0:48 ` [pushed 2.34 03/14] gaih_inet: Simplify canon name resolution Siddhesh Poyarekar
2023-09-16  0:48 ` [pushed 2.34 04/14] getaddrinfo: Fix leak with AI_ALL [BZ #28852] Siddhesh Poyarekar
2023-09-16  0:48 ` [pushed 2.34 05/14] gaih_inet: Simplify service resolution Siddhesh Poyarekar
2023-09-16  0:48 ` [pushed 2.34 06/14] gaih_inet: make numeric lookup a separate routine Siddhesh Poyarekar
2023-09-16  0:48 ` [pushed 2.34 07/14] gaih_inet: Split simple gethostbyname into its own function Siddhesh Poyarekar
2023-09-16  0:48 ` [pushed 2.34 08/14] gaih_inet: Split nscd lookup code " Siddhesh Poyarekar
2023-09-16  0:48 ` [pushed 2.34 09/14] gaih_inet: separate nss lookup loop " Siddhesh Poyarekar
2023-09-16  0:48 ` [pushed 2.34 10/14] gaih_inet: make gethosts into a function Siddhesh Poyarekar
2023-09-16  0:48 ` [pushed 2.34 11/14] gaih_inet: split loopback lookup into its own function Siddhesh Poyarekar
2023-09-16  0:48 ` [pushed 2.34 12/14] gaih_inet: Split result generation " Siddhesh Poyarekar
2023-09-16  0:49 ` [pushed 2.34 13/14] gethosts: Return EAI_MEMORY on allocation failure Siddhesh Poyarekar
2023-09-16  0:49 ` [pushed 2.34 14/14] getaddrinfo: Fix use after free in getcanonname (CVE-2023-4806) 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).