From 6b7b2abac75f969a86c537d64adf003378e24113 Mon Sep 17 00:00:00 2001
From: Florian Weimer <fweimer@redhat.com>
Date: Mon, 25 Jun 2018 16:05:46 +0200
Subject: nscd: Switch to struct scratch_buffer in adhstaiX [BZ #18023]

The pre-allocation of the three scratch buffers increased the initial
stack size somewhat, but if retries are needed, the previous version
used more stack space if extend_alloca could not merge allocations.
Lack of alloca accounting also means could be problematic with
extremely large NSS responses, too.

	[BZ #18023]
	* nscd/aicache.c (addhstaiX): Use struct scratch_buffer instead
	of extend_alloca.
---
 nscd/aicache.c | 79 +++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 45 insertions(+), 34 deletions(-)

(limited to 'nscd')

diff --git a/nscd/aicache.c b/nscd/aicache.c
index 2095edf..439e0ea 100644
--- a/nscd/aicache.c
+++ b/nscd/aicache.c
@@ -28,6 +28,7 @@
 #include <resolv/resolv-internal.h>
 #include <resolv/resolv_context.h>
 #include <resolv/res_use_inet6.h>
+#include <scratch_buffer.h>
 
 #include "dbg_log.h"
 #include "nscd.h"
@@ -108,10 +109,13 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req,
   if (ctx == NULL)
     no_more = 1;
 
-  size_t tmpbuf6len = 1024;
-  char *tmpbuf6 = alloca (tmpbuf6len);
-  size_t tmpbuf4len = 0;
-  char *tmpbuf4 = NULL;
+  struct scratch_buffer tmpbuf6;
+  scratch_buffer_init (&tmpbuf6);
+  struct scratch_buffer tmpbuf4;
+  scratch_buffer_init (&tmpbuf4);
+  struct scratch_buffer canonbuf;
+  scratch_buffer_init (&canonbuf);
+
   int32_t ttl = INT32_MAX;
   ssize_t total = 0;
   char *key_copy = NULL;
@@ -124,6 +128,7 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req,
       int status[2] = { NSS_STATUS_UNAVAIL, NSS_STATUS_UNAVAIL };
       int naddrs = 0;
       size_t addrslen = 0;
+
       char *canon = NULL;
       size_t canonlen;
 
@@ -138,12 +143,17 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req,
 	      at = &atmem;
 	      rc6 = 0;
 	      herrno = 0;
-	      status[1] = DL_CALL_FCT (fct4, (key, &at, tmpbuf6, tmpbuf6len,
+	      status[1] = DL_CALL_FCT (fct4, (key, &at,
+					      tmpbuf6.data, tmpbuf6.length,
 					      &rc6, &herrno, &ttl));
 	      if (rc6 != ERANGE || (herrno != NETDB_INTERNAL
 				    && herrno != TRY_AGAIN))
 		break;
-	      tmpbuf6 = extend_alloca (tmpbuf6, tmpbuf6len, 2 * tmpbuf6len);
+	      if (!scratch_buffer_grow (&tmpbuf6))
+		{
+		  rc6 = ENOMEM;
+		  break;
+		}
 	    }
 
 	  if (rc6 != 0 && herrno == NETDB_INTERNAL)
@@ -221,41 +231,38 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req,
 	  while (1)
 	    {
 	      rc6 = 0;
-	      status[0] = DL_CALL_FCT (fct, (key, AF_INET6, &th[0], tmpbuf6,
-					     tmpbuf6len, &rc6, &herrno, &ttl,
+	      status[0] = DL_CALL_FCT (fct, (key, AF_INET6, &th[0],
+					     tmpbuf6.data, tmpbuf6.length,
+					     &rc6, &herrno, &ttl,
 					     &canon));
 	      if (rc6 != ERANGE || herrno != NETDB_INTERNAL)
 		break;
-	      tmpbuf6 = extend_alloca (tmpbuf6, tmpbuf6len, 2 * tmpbuf6len);
+	      if (!scratch_buffer_grow (&tmpbuf6))
+		{
+		  rc6 = ENOMEM;
+		  break;
+		}
 	    }
 
 	  if (rc6 != 0 && herrno == NETDB_INTERNAL)
 	    goto out;
 
-	  /* If the IPv6 lookup has been successful do not use the
-	     buffer used in that lookup, use a new one.  */
-	  if (status[0] == NSS_STATUS_SUCCESS && rc6 == 0)
-	    {
-	      tmpbuf4len = 512;
-	      tmpbuf4 = alloca (tmpbuf4len);
-	    }
-	  else
-	    {
-	      tmpbuf4len = tmpbuf6len;
-	      tmpbuf4 = tmpbuf6;
-	    }
-
 	  /* Next collect IPv4 information.  */
 	  while (1)
 	    {
 	      rc4 = 0;
-	      status[1] = DL_CALL_FCT (fct, (key, AF_INET, &th[1], tmpbuf4,
-					     tmpbuf4len, &rc4, &herrno,
+	      status[1] = DL_CALL_FCT (fct, (key, AF_INET, &th[1],
+					     tmpbuf4.data, tmpbuf4.length,
+					     &rc4, &herrno,
 					     ttl == INT32_MAX ? &ttl : NULL,
 					     canon == NULL ? &canon : NULL));
 	      if (rc4 != ERANGE || herrno != NETDB_INTERNAL)
 		break;
-	      tmpbuf4 = extend_alloca (tmpbuf4, tmpbuf4len, 2 * tmpbuf4len);
+	      if (!scratch_buffer_grow (&tmpbuf4))
+		{
+		  rc4 = ENOMEM;
+		  break;
+		}
 	    }
 
 	  if (rc4 != 0 && herrno == NETDB_INTERNAL)
@@ -281,13 +288,11 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req,
 	      cfct = __nss_lookup_function (nip, "getcanonname_r");
 	      if (cfct != NULL)
 		{
-		  const size_t max_fqdn_len = 256;
-		  char *buf = alloca (max_fqdn_len);
 		  char *s;
 		  int rc;
 
-		  if (DL_CALL_FCT (cfct, (key, buf, max_fqdn_len, &s,
-					  &rc, &herrno))
+		  if (DL_CALL_FCT (cfct, (key, canonbuf.data, canonbuf.length,
+					  &s, &rc, &herrno))
 		      == NSS_STATUS_SUCCESS)
 		    canon = s;
 		  else
@@ -316,18 +321,20 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req,
 		      addrfamily = AF_INET6;
 		    }
 
-		  size_t tmpbuflen = 512;
-		  char *tmpbuf = alloca (tmpbuflen);
 		  int rc;
 		  while (1)
 		    {
 		      rc = __gethostbyaddr2_r (addr, addrlen, addrfamily,
-					       &hstent_mem, tmpbuf, tmpbuflen,
+					       &hstent_mem,
+					       canonbuf.data, canonbuf.length,
 					       &hstent, &herrno, NULL);
 		      if (rc != ERANGE || herrno != NETDB_INTERNAL)
 			break;
-		      tmpbuf = extend_alloca (tmpbuf, tmpbuflen,
-					      tmpbuflen * 2);
+		      if (!scratch_buffer_grow (&canonbuf))
+			{
+			  rc = ENOMEM;
+			  break;
+			}
 		    }
 
 		  if (rc == 0)
@@ -531,6 +538,10 @@ next_nip:
 	dh->usable = false;
     }
 
+  scratch_buffer_free (&tmpbuf6);
+  scratch_buffer_free (&tmpbuf4);
+  scratch_buffer_free (&canonbuf);
+
   return timeout;
 }
 
-- 
cgit v1.1