nss_dns: Adjust gethostby* processing to struct alloc_buffer

Message ID 20190308205540.6D9F980DD6B5@oldenburg2.str.redhat.com
State New, archived
Headers

Commit Message

Florian Weimer March 8, 2019, 8:55 p.m. UTC
  In particular, this eliminates the struct host_data type in favor
of explicit allocations from the buffer.  _nss_dns_gethostbyaddr2_r
used to poke into the struct host_data allocation made by
getanswer_r, by carefully aligning pointers in the same way.  This
complexity is now gone.

The old code used the bp pointer in key points to point to the
textual version of the owner name of the current record.  After
the channges, the current_name variable assumes this task.  This
aspect of the code is rather tricky because to conserve buffer space,
the allocation may actually be used for something else besides the
decoded domain name (e.g. if the record is not a CNAME record and the
name matches the previos name).

2019-03-08  Florian Weimer  <fweimer@redhat.com>

	* resolv/nss_dns/dns-host.c (getanswer_r): Use struct alloc_buffer.
	(gethostbyname3_context): Adjust.
	(_nss_dns_gethostbyaddr2_r): Likewise.
	* resolv/mapv4v6hostent.h (align): Remove typedef.
	(map_v4v6_hostent): Use struct
	alloc_buffer.
  

Patch

diff --git a/resolv/mapv4v6hostent.h b/resolv/mapv4v6hostent.h
index c11038adf3..9addf37754 100644
--- a/resolv/mapv4v6hostent.h
+++ b/resolv/mapv4v6hostent.h
@@ -52,13 +52,8 @@ 
 #include <arpa/nameser.h>
 #include <sys/socket.h>
 
-typedef union {
-    int32_t al;
-    char ac;
-} align;
-
 static int
-map_v4v6_hostent (struct hostent *hp, char **bpp, int *lenp)
+map_v4v6_hostent (struct hostent *hp, struct alloc_buffer *abuffer)
 {
   char **ap;
 
@@ -68,17 +63,15 @@  map_v4v6_hostent (struct hostent *hp, char **bpp, int *lenp)
   hp->h_length = IN6ADDRSZ;
   for (ap = hp->h_addr_list; *ap; ap++)
     {
-      int i = sizeof (align) - ((u_long) *bpp % sizeof (align));
-
-      if (*lenp < (i + IN6ADDRSZ))
+      struct in6_addr *addr = alloc_buffer_alloc (abuffer, struct in6_addr);
+      if (addr == NULL)
 	/* Out of memory.  */
 	return 1;
-      *bpp += i;
-      *lenp -= i;
-      map_v4v6_address (*ap, *bpp);
-      *ap = *bpp;
-      *bpp += IN6ADDRSZ;
-      *lenp -= IN6ADDRSZ;
+      addr->s6_addr32[0] = htonl (0);
+      addr->s6_addr32[1] = htonl (0);
+      addr->s6_addr32[2] = htonl (0xffff);
+      memcpy (&addr->s6_addr32[3], *ap, sizeof (addr->s6_addr32[3]));
+      *ap = (char *) addr;
     }
   return 0;
 }
diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c
index f04e08016c..c5a2ca5e28 100644
--- a/resolv/nss_dns/dns-host.c
+++ b/resolv/nss_dns/dns-host.c
@@ -82,11 +82,12 @@ 
 #include "nsswitch.h"
 #include <arpa/nameser.h>
 
+#include <alloc_buffer.h>
+
 #include <resolv/resolv-internal.h>
 #include <resolv/resolv_context.h>
 
 /* Get implementations of some internal functions.  */
-#include <resolv/mapv4v6addr.h>
 #include <resolv/mapv4v6hostent.h>
 
 #define RESOLVSORT
@@ -112,8 +113,9 @@  typedef union querybuf
 static enum nss_status getanswer_r (struct resolv_context *ctx,
 				    const querybuf *answer, int anslen,
 				    const char *qname, int qtype,
-				    struct hostent *result, char *buffer,
-				    size_t buflen, int *errnop, int *h_errnop,
+				    struct hostent *result,
+				    struct alloc_buffer *buffer,
+				    int *errnop, int *h_errnop,
 				    int map, int32_t *ttlp, char **canonp);
 
 static enum nss_status gaih_getanswer (const querybuf *answer1, int anslen1,
@@ -266,8 +268,9 @@  gethostbyname3_context (struct resolv_context *ctx,
       result->h_length = INADDRSZ;
     }
 
+  struct alloc_buffer abuffer = alloc_buffer_create (buffer, buflen);
   status = getanswer_r
-    (ctx, host_buffer.buf, n, name, type, result, buffer, buflen,
+    (ctx, host_buffer.buf, n, name, type, result, &abuffer,
      errnop, h_errnop, map, ttlp, canonp);
   if (host_buffer.buf != orig_host_buffer)
     free (host_buffer.buf);
@@ -435,13 +438,6 @@  _nss_dns_gethostbyaddr2_r (const void *addr, socklen_t len, int af,
   static const u_char tunnelled[] = { 0,0, 0,0, 0,0, 0,0, 0,0, 0,0 };
   static const u_char v6local[] = { 0,0, 0,1 };
   const u_char *uaddr = (const u_char *)addr;
-  struct host_data
-  {
-    char *aliases[MAX_NR_ALIASES];
-    unsigned char host_addr[16];	/* IPv4 or IPv6 */
-    char *h_addr_ptrs[MAX_NR_ADDRS + 1];
-    char linebuffer[0];
-  } *host_data = (struct host_data *) buffer;
   union
   {
     querybuf *buf;
@@ -453,19 +449,6 @@  _nss_dns_gethostbyaddr2_r (const void *addr, socklen_t len, int af,
   int n, status;
   int olderr = errno;
 
- uintptr_t pad = -(uintptr_t) buffer % __alignof__ (struct host_data);
- buffer += pad;
- buflen = buflen > pad ? buflen - pad : 0;
-
- if (__glibc_unlikely (buflen < sizeof (struct host_data)))
-   {
-     *errnop = ERANGE;
-     *h_errnop = NETDB_INTERNAL;
-     return NSS_STATUS_TRYAGAIN;
-   }
-
- host_data = (struct host_data *) buffer;
-
   struct resolv_context *ctx = __resolv_context_get ();
   if (ctx == NULL)
     {
@@ -545,8 +528,9 @@  _nss_dns_gethostbyaddr2_r (const void *addr, socklen_t len, int af,
       return errno == ECONNREFUSED ? NSS_STATUS_UNAVAIL : NSS_STATUS_NOTFOUND;
     }
 
+  struct alloc_buffer abuffer = alloc_buffer_create (buffer, buflen);
   status = getanswer_r
-    (ctx, host_buffer.buf, n, qbuf, T_PTR, result, buffer, buflen,
+    (ctx, host_buffer.buf, n, qbuf, T_PTR, result, &abuffer,
      errnop, h_errnop, 0 /* XXX */, ttlp, NULL);
   if (host_buffer.buf != orig_host_buffer)
     free (host_buffer.buf);
@@ -558,9 +542,21 @@  _nss_dns_gethostbyaddr2_r (const void *addr, socklen_t len, int af,
 
   result->h_addrtype = af;
   result->h_length = len;
-  memcpy (host_data->host_addr, addr, len);
-  host_data->h_addr_ptrs[0] = (char *) host_data->host_addr;
-  host_data->h_addr_ptrs[1] = NULL;
+  result->h_addr_list = alloc_buffer_alloc_array (&abuffer, char *, 2);
+  if (result->h_addr_list != NULL)
+    {
+      char *host_addr = __alloc_buffer_alloc (&abuffer, len,
+					      __alignof (struct in_addr));
+      memcpy (host_addr, addr, len);
+      result->h_addr_list[0] = host_addr;
+      result->h_addr_list[1] = NULL;
+    }
+  if (alloc_buffer_has_failed (&abuffer))
+    {
+      *errnop = ERANGE;
+      *h_errnop = NETDB_INTERNAL;
+      return NSS_STATUS_TRYAGAIN;
+    }
   *h_errnop = NETDB_SUCCESS;
   __resolv_context_put (ctx);
   return NSS_STATUS_SUCCESS;
@@ -626,41 +622,19 @@  addrsort (struct resolv_context *ctx, char **ap, int num)
 static enum nss_status
 getanswer_r (struct resolv_context *ctx,
 	     const querybuf *answer, int anslen, const char *qname, int qtype,
-	     struct hostent *result, char *buffer, size_t buflen,
+	     struct hostent *result, struct alloc_buffer *buffer,
 	     int *errnop, int *h_errnop, int map, int32_t *ttlp, char **canonp)
 {
-  struct host_data
-  {
-    char *aliases[MAX_NR_ALIASES];
-    unsigned char host_addr[16];	/* IPv4 or IPv6 */
-    char *h_addr_ptrs[0];
-  } *host_data;
-  int linebuflen;
   const HEADER *hp;
   const u_char *end_of_message, *cp;
   int n, ancount, qdcount;
   int haveanswer, had_error;
-  char *bp, **ap, **hap;
+  char **ap, **hap;
   char tbuf[MAXDNAME];
   const char *tname;
   int (*name_ok) (const char *);
   u_char packtmp[NS_MAXCDNAME];
   int have_to_map = 0;
-  uintptr_t pad = -(uintptr_t) buffer % __alignof__ (struct host_data);
-  buffer += pad;
-  buflen = buflen > pad ? buflen - pad : 0;
-  if (__glibc_unlikely (buflen < sizeof (struct host_data)))
-    {
-      /* The buffer is too small.  */
-    too_small:
-      *errnop = ERANGE;
-      *h_errnop = NETDB_INTERNAL;
-      return NSS_STATUS_TRYAGAIN;
-    }
-  host_data = (struct host_data *) buffer;
-  linebuflen = buflen - sizeof (struct host_data);
-  if (buflen - sizeof (struct host_data) != linebuflen)
-    linebuflen = INT_MAX;
 
   tname = qname;
   result->h_name = NULL;
@@ -691,10 +665,18 @@  getanswer_r (struct resolv_context *ctx,
       *h_errnop = NO_RECOVERY;
       return NSS_STATUS_UNAVAIL;
     }
-  if (sizeof (struct host_data) + (ancount + 1) * sizeof (char *) >= buflen)
-    goto too_small;
-  bp = (char *) &host_data->h_addr_ptrs[ancount + 1];
-  linebuflen -= (ancount + 1) * sizeof (char *);
+
+  result->h_aliases = alloc_buffer_alloc_array (buffer, char *,
+						MAX_NR_ALIASES);
+  result->h_addr_list = alloc_buffer_alloc_array (buffer, char *,
+						  ancount + 1);
+  if (result->h_addr_list == NULL)
+    {
+    too_small:
+      *errnop = ERANGE;
+      *h_errnop = NETDB_INTERNAL;
+      return NSS_STATUS_TRYAGAIN;
+    }
 
   n = __ns_name_unpack (answer->buf, end_of_message, cp,
 			packtmp, sizeof packtmp);
@@ -704,13 +686,21 @@  getanswer_r (struct resolv_context *ctx,
       *h_errnop = NO_RECOVERY;
       return NSS_STATUS_UNAVAIL;
     }
-  if (__ns_name_ntop (packtmp, bp, linebuflen) < 0)
+
+  if (__ns_name_ntop (packtmp, alloc_buffer_next (buffer, char),
+		      alloc_buffer_size (buffer)) < 0)
     goto too_small;
 
-  if (bp[0] == '.')
-    bp[0] = '\0';
+  /* Store the currently processed name in the unallocated tail of the
+     buffer.  In subsequent iterations of the loop, the decoded name
+     may be discarded if it is not needed after all, and current_name
+     becomes invalid at this point.  */
+  char *current_name = alloc_buffer_next (buffer, char);
 
-  if (__glibc_unlikely (name_ok (bp) == 0))
+  if (current_name[0] == '.')
+    current_name[0] = '\0';
+
+  if (__glibc_unlikely (name_ok (current_name) == 0))
     {
       errno = EBADMSG;
       *errnop = EBADMSG;
@@ -725,28 +715,23 @@  getanswer_r (struct resolv_context *ctx,
        * same as the one we sent; this just gets the expanded name
        * (i.e., with the succeeding search-domain tacked on).
        */
-      n = strlen (bp) + 1;             /* for the \0 */
+      n = strlen (current_name) + 1; /* For the '\0'.  */
       if (n >= MAXHOSTNAMELEN)
 	{
 	  *h_errnop = NO_RECOVERY;
 	  *errnop = ENOENT;
 	  return NSS_STATUS_TRYAGAIN;
 	}
-      result->h_name = bp;
-      bp += n;
-      linebuflen -= n;
-      if (linebuflen < 0)
-	goto too_small;
+      result->h_name = current_name;
+      alloc_buffer_alloc_bytes (buffer, n);
       /* The qname can be abbreviated, but h_name is now absolute. */
       qname = result->h_name;
     }
 
-  ap = host_data->aliases;
+  ap = result->h_aliases;
   *ap = NULL;
-  result->h_aliases = host_data->aliases;
-  hap = host_data->h_addr_ptrs;
+  hap = result->h_addr_list;
   *hap = NULL;
-  result->h_addr_list = host_data->h_addr_ptrs;
   haveanswer = 0;
   had_error = 0;
 
@@ -762,10 +747,12 @@  getanswer_r (struct resolv_context *ctx,
 	  continue;
 	}
 
-      if (__ns_name_ntop (packtmp, bp, linebuflen) < 0)
+      if (__ns_name_ntop (packtmp, alloc_buffer_next (buffer, char),
+			  alloc_buffer_size (buffer)) < 0)
 	goto too_small;
+      current_name = alloc_buffer_next (buffer, char);
 
-      if (__glibc_unlikely ((*name_ok) (bp) == 0))
+      if (__glibc_unlikely ((*name_ok) (current_name) == 0))
 	{
 	  ++had_error;
 	  continue;
@@ -807,7 +794,7 @@  getanswer_r (struct resolv_context *ctx,
 	  if (ttlp != NULL && ttl < *ttlp)
 	      *ttlp = ttl;
 
-	  if (ap >= &host_data->aliases[MAX_NR_ALIASES - 1])
+	  if (ap >= &result->h_aliases[MAX_NR_ALIASES - 1])
 	    continue;
 	  n = dn_expand (answer->buf, end_of_message, cp, tbuf, sizeof tbuf);
 	  if (__glibc_unlikely (n < 0 || (*name_ok) (tbuf) == 0))
@@ -817,27 +804,24 @@  getanswer_r (struct resolv_context *ctx,
 	    }
 	  cp += n;
 	  /* Store alias.  */
-	  *ap++ = bp;
-	  n = strlen (bp) + 1;		/* For the \0.  */
+	  *ap++ = current_name;
+	  n = strlen (current_name) + 1; /* For the \0.  */
 	  if (__glibc_unlikely (n >= MAXHOSTNAMELEN))
 	    {
 	      ++had_error;
 	      continue;
 	    }
-	  bp += n;
-	  linebuflen -= n;
+	  alloc_buffer_alloc_bytes (buffer, n);
 	  /* Get canonical name.  */
-	  n = strlen (tbuf) + 1;	/* For the \0.  */
-	  if (__glibc_unlikely (n > linebuflen))
+	  result->h_name = alloc_buffer_copy_string (buffer, tbuf);
+	  current_name = NULL;
+	  if (result->h_name == NULL)
 	    goto too_small;
-	  if (__glibc_unlikely (n >= MAXHOSTNAMELEN))
+	  if (strlen (tbuf) >= MAXHOSTNAMELEN)
 	    {
 	      ++had_error;
 	      continue;
 	    }
-	  result->h_name = bp;
-	  bp = __mempcpy (bp, tbuf, n);	/* Cannot overflow.  */
-	  linebuflen -= n;
 	  continue;
 	}
 
@@ -855,17 +839,15 @@  getanswer_r (struct resolv_context *ctx,
 	    }
 	  cp += n;
 	  /* Get canonical name.  */
-	  n = strlen (tbuf) + 1;   /* For the \0.  */
-	  if (__glibc_unlikely (n > linebuflen))
+	  tname = alloc_buffer_copy_string (buffer, tbuf);
+	  if (tname == NULL)
 	    goto too_small;
-	  if (__glibc_unlikely (n >= MAXHOSTNAMELEN))
+	  if (strlen (tbuf) >= MAXHOSTNAMELEN)
 	    {
 	      ++had_error;
 	      continue;
 	    }
-	  tname = bp;
-	  bp = __mempcpy (bp, tbuf, n);	/* Cannot overflow.  */
-	  linebuflen -= n;
+	  current_name = NULL;
 	  continue;
 	}
 
@@ -880,7 +862,8 @@  getanswer_r (struct resolv_context *ctx,
       switch (type)
 	{
 	case T_PTR:
-	  if (__glibc_unlikely (strcasecmp (tname, bp) != 0))
+	  if (__glibc_unlikely (strcasecmp (tname, current_name)
+				!= 0))
 	    {
 	      cp += n;
 	      continue;			/* XXX - had_error++ ? */
@@ -893,9 +876,14 @@  getanswer_r (struct resolv_context *ctx,
 	      ++had_error;
 	      break;
 	    }
-	  if (__ns_name_ntop (packtmp, bp, linebuflen) < 0)
+
+	  if (__ns_name_ntop (packtmp,
+			      alloc_buffer_next (buffer, char),
+			      alloc_buffer_size (buffer)) < 0)
 	    goto too_small;
-	  if (__glibc_unlikely (res_hnok (bp) == 0))
+	  current_name = alloc_buffer_next (buffer, char);
+
+	  if (__glibc_unlikely (res_hnok (current_name) == 0))
 	    {
 	      ++had_error;
 	      break;
@@ -904,12 +892,14 @@  getanswer_r (struct resolv_context *ctx,
 	      *ttlp = ttl;
 	  /* bind would put multiple PTR records as aliases, but we don't do
 	     that.  */
-	  result->h_name = bp;
+	  result->h_name = alloc_buffer_next (buffer, char);
+	  alloc_buffer_alloc_bytes (buffer, strlen (result->h_name) + 1);
 	  *h_errnop = NETDB_SUCCESS;
 	  return NSS_STATUS_SUCCESS;
 	case T_A:
 	case T_AAAA:
-	  if (__glibc_unlikely (strcasecmp (result->h_name, bp) != 0))
+	  if (__glibc_unlikely (strcasecmp (result->h_name, current_name)
+				!= 0))
 	    {
 	      cp += n;
 	      continue;			/* XXX - had_error++ ? */
@@ -930,29 +920,26 @@  getanswer_r (struct resolv_context *ctx,
 	    }
 	  if (!haveanswer)
 	    {
-	      int nn;
-
 	      /* We compose a single hostent out of the entire chain of
 	         entries, so the TTL of the hostent is essentially the lowest
 		 TTL in the chain.  */
 	      if (ttlp != NULL && ttl < *ttlp)
 		*ttlp = ttl;
 	      if (canonp != NULL)
-		*canonp = bp;
-	      result->h_name = bp;
-	      nn = strlen (bp) + 1;	/* for the \0 */
-	      bp += nn;
-	      linebuflen -= nn;
+		*canonp = alloc_buffer_next (buffer, char);
+	      result->h_name = alloc_buffer_next (buffer, char);
+	      alloc_buffer_alloc_bytes (buffer, strlen (result->h_name) + 1);
 	    }
 
-	  linebuflen -= sizeof (align) - ((u_long) bp % sizeof (align));
-	  bp += sizeof (align) - ((u_long) bp % sizeof (align));
-
-	  if (__glibc_unlikely (n > linebuflen))
-	    goto too_small;
-	  bp = __mempcpy (*hap++ = bp, cp, n);
-	  cp += n;
-	  linebuflen -= n;
+	  {
+	    void *addr = __alloc_buffer_alloc (buffer, n,
+					       __alignof (struct in_addr));
+	    if (addr == NULL)
+	      goto too_small;
+	    memcpy (addr, cp, n);
+	    *hap++ = addr;
+	    cp += n;
+	  }
 	  break;
 	default:
 	  abort ();
@@ -972,22 +959,19 @@  getanswer_r (struct resolv_context *ctx,
        */
       if (haveanswer > 1 && qtype == T_A
 	  && __resolv_context_sort_count (ctx) > 0)
-	addrsort (ctx, host_data->h_addr_ptrs, haveanswer);
+	addrsort (ctx, result->h_addr_list, haveanswer);
 
       if (result->h_name == NULL)
 	{
-	  n = strlen (qname) + 1;	/* For the \0.  */
-	  if (n > linebuflen)
+	  result->h_name = alloc_buffer_copy_string (buffer, qname);
+	  if (result->h_name == NULL)
 	    goto too_small;
-	  if (n >= MAXHOSTNAMELEN)
+	  if (strlen (qname) + 1 >= MAXHOSTNAMELEN)
 	    goto no_recovery;
-	  result->h_name = bp;
-	  bp = __mempcpy (bp, qname, n);	/* Cannot overflow.  */
-	  linebuflen -= n;
 	}
 
       if (have_to_map)
-	if (map_v4v6_hostent (result, &bp, &linebuflen))
+	if (map_v4v6_hostent (result, buffer))
 	  goto too_small;
       *h_errnop = NETDB_SUCCESS;
       return NSS_STATUS_SUCCESS;
@@ -998,7 +982,7 @@  getanswer_r (struct resolv_context *ctx,
   /* Special case here: if the resolver sent a result but it only
      contains a CNAME while we are looking for a T_A or T_AAAA record,
      we fail with NOTFOUND instead of TRYAGAIN.  */
-  return ((qtype == T_A || qtype == T_AAAA) && ap != host_data->aliases
+  return ((qtype == T_A || qtype == T_AAAA) && ap != result->h_aliases
 	   ? NSS_STATUS_NOTFOUND : NSS_STATUS_TRYAGAIN);
 }