[PATCH] nscd: Use struct scratch_buffer, not extend_alloca in most caches [BZ #18023]

[PATCH] nscd: Use struct scratch_buffer, not extend_alloca in most caches [BZ #18023]

[Florian Weimer]

This replaces the ERANGE retry loops with loops which have heap
fallback.  Heap allocation might actually be required for extremely
large NSS results.

2018-06-25  Florian Weimer  <fweimer@redhat.com>

	[BZ #18023]
	* nscd/grpcache.c (addgrbyX): Use struct scratch_buffer instead
	of extend_alloca.
	* nscd/hstcache.c (addhstbyX): Likewise.
	* nscd/pwdcache.c (addpwbyX): Likewise.
	* nscd/servicescache.c (addservbyX): Likewise.

diff --git a/nscd/grpcache.c b/nscd/grpcache.c
index c01aeb1f1d..b4c8ea9c56 100644
--- a/nscd/grpcache.c
+++ b/nscd/grpcache.c
@@ -16,7 +16,6 @@
    You should have received a copy of the GNU General Public License
    along with this program; if not, see <http://www.gnu.org/licenses/>.  */
 
-#include <alloca.h>
 #include <assert.h>
 #include <errno.h>
 #include <error.h>
@@ -32,6 +31,7 @@
 #include <sys/mman.h>
 #include <sys/socket.h>
 #include <stackinfo.h>
+#include <scratch_buffer.h>
 
 #include "nscd.h"
 #include "dbg_log.h"
@@ -417,12 +417,12 @@ addgrbyX (struct database_dyn *db, int fd, request_header *req,
      look again in the table whether the dataset is now available.  We
      simply insert it.  It does not matter if it is in there twice.  The
      pruning function only will look at the timestamp.  */
-  size_t buflen = 1024;
-  char *buffer = (char *) alloca (buflen);
+
   struct group resultbuf;
   struct group *grp;
-  bool use_malloc = false;
   int errval = 0;
+  struct scratch_buffer tmpbuf;
+  scratch_buffer_init (&tmpbuf);
 
   if (__glibc_unlikely (debug_level > 0))
     {
@@ -432,43 +432,24 @@ addgrbyX (struct database_dyn *db, int fd, request_header *req,
 	dbg_log (_("Reloading \"%s\" in group cache!"), keystr);
     }
 
-  while (lookup (req->type, key, &resultbuf, buffer, buflen, &grp) != 0
+  while (lookup (req->type, key, &resultbuf,
+		 tmpbuf.data, tmpbuf.length, &grp) != 0
 	 && (errval = errno) == ERANGE)
-    {
-      errno = 0;
-
-      if (__glibc_unlikely (buflen > 32768))
-	{
-	  char *old_buffer = buffer;
-	  buflen *= 2;
-	  buffer = (char *) realloc (use_malloc ? buffer : NULL, buflen);
-	  if (buffer == NULL)
-	    {
-	      /* We ran out of memory.  We cannot do anything but
-		 sending a negative response.  In reality this should
-		 never happen.  */
-	      grp = NULL;
-	      buffer = old_buffer;
-
-	      /* We set the error to indicate this is (possibly) a
-		 temporary error and that it does not mean the entry
-		 is not available at all.  */
-	      errval = EAGAIN;
-	      break;
-	    }
-	  use_malloc = true;
-	}
-      else
-	/* Allocate a new buffer on the stack.  If possible combine it
-	   with the previously allocated buffer.  */
-	buffer = (char *) extend_alloca (buffer, buflen, 2 * buflen);
-    }
+    if (!scratch_buffer_grow (&tmpbuf))
+      {
+	/* We ran out of memory.  We cannot do anything but sending a
+	   negative response.  In reality this should never
+	   happen.  */
+	grp = NULL;
+	/* We set the error to indicate this is (possibly) a temporary
+	   error and that it does not mean the entry is not available
+	   at all.  */
+	errval = EAGAIN;
+	break;
+      }
 
   time_t timeout = cache_addgr (db, fd, req, keystr, grp, uid, he, dh, errval);
-
-  if (use_malloc)
-    free (buffer);
-
+  scratch_buffer_free (&tmpbuf);
   return timeout;
 }
 
diff --git a/nscd/hstcache.c b/nscd/hstcache.c
index 6ef0c653ea..5597e13ec1 100644
--- a/nscd/hstcache.c
+++ b/nscd/hstcache.c
@@ -34,6 +34,7 @@
 #include <arpa/nameser.h>
 #include <sys/mman.h>
 #include <stackinfo.h>
+#include <scratch_buffer.h>
 
 #include "nscd.h"
 #include "dbg_log.h"
@@ -432,11 +433,8 @@ addhstbyX (struct database_dyn *db, int fd, request_header *req,
      look again in the table whether the dataset is now available.  We
      simply insert it.  It does not matter if it is in there twice.  The
      pruning function only will look at the timestamp.  */
-  int buflen = 1024;
-  char *buffer = (char *) alloca (buflen);
   struct hostent resultbuf;
   struct hostent *hst;
-  bool use_malloc = false;
   int errval = 0;
   int32_t ttl = INT32_MAX;
 
@@ -456,46 +454,30 @@ addhstbyX (struct database_dyn *db, int fd, request_header *req,
 	dbg_log (_("Reloading \"%s\" in hosts cache!"), (char *) str);
     }
 
-  while (lookup (req->type, key, &resultbuf, buffer, buflen, &hst, &ttl) != 0
+  struct scratch_buffer tmpbuf;
+  scratch_buffer_init (&tmpbuf);
+
+  while (lookup (req->type, key, &resultbuf,
+		 tmpbuf.data, tmpbuf.length, &hst, &ttl) != 0
 	 && h_errno == NETDB_INTERNAL
 	 && (errval = errno) == ERANGE)
-    {
-      errno = 0;
-
-      if (__glibc_unlikely (buflen > 32768))
-	{
-	  char *old_buffer = buffer;
-	  buflen *= 2;
-	  buffer = (char *) realloc (use_malloc ? buffer : NULL, buflen);
-	  if (buffer == NULL)
-	    {
-	      /* We ran out of memory.  We cannot do anything but
-		 sending a negative response.  In reality this should
-		 never happen.  */
-	      hst = NULL;
-	      buffer = old_buffer;
-
-	      /* We set the error to indicate this is (possibly) a
-		 temporary error and that it does not mean the entry
-		 is not available at all.  */
-	      h_errno = TRY_AGAIN;
-	      errval = EAGAIN;
-	      break;
-	    }
-	  use_malloc = true;
-	}
-      else
-	/* Allocate a new buffer on the stack.  If possible combine it
-	   with the previously allocated buffer.  */
-	buffer = (char *) extend_alloca (buffer, buflen, 2 * buflen);
-    }
+    if (!scratch_buffer_grow (&tmpbuf))
+      {
+	/* We ran out of memory.  We cannot do anything but sending a
+	   negative response.  In reality this should never
+	   happen.  */
+	hst = NULL;
+	/* We set the error to indicate this is (possibly) a temporary
+	   error and that it does not mean the entry is not
+	   available at all.  */
+	h_errno = TRY_AGAIN;
+	errval = EAGAIN;
+	break;
+      }
 
   time_t timeout = cache_addhst (db, fd, req, key, hst, uid, he, dh,
 				 h_errno == TRY_AGAIN ? errval : 0, ttl);
-
-  if (use_malloc)
-    free (buffer);
-
+  scratch_buffer_free (&tmpbuf);
   return timeout;
 }
 
diff --git a/nscd/pwdcache.c b/nscd/pwdcache.c
index 997d7c08fc..092c79222a 100644
--- a/nscd/pwdcache.c
+++ b/nscd/pwdcache.c
@@ -16,7 +16,6 @@
    You should have received a copy of the GNU General Public License
    along with this program; if not, see <http://www.gnu.org/licenses/>.  */
 
-#include <alloca.h>
 #include <assert.h>
 #include <errno.h>
 #include <error.h>
@@ -32,6 +31,7 @@
 #include <sys/mman.h>
 #include <sys/socket.h>
 #include <stackinfo.h>
+#include <scratch_buffer.h>
 
 #include "nscd.h"
 #include "dbg_log.h"
@@ -395,12 +395,11 @@ addpwbyX (struct database_dyn *db, int fd, request_header *req,
      look again in the table whether the dataset is now available.  We
      simply insert it.  It does not matter if it is in there twice.  The
      pruning function only will look at the timestamp.  */
-  size_t buflen = 1024;
-  char *buffer = (char *) alloca (buflen);
   struct passwd resultbuf;
   struct passwd *pwd;
-  bool use_malloc = false;
   int errval = 0;
+  struct scratch_buffer tmpbuf;
+  scratch_buffer_init (&tmpbuf);
 
   if (__glibc_unlikely (debug_level > 0))
     {
@@ -410,45 +409,26 @@ addpwbyX (struct database_dyn *db, int fd, request_header *req,
 	dbg_log (_("Reloading \"%s\" in password cache!"), keystr);
     }
 
-  while (lookup (req->type, key, &resultbuf, buffer, buflen, &pwd) != 0
+  while (lookup (req->type, key, &resultbuf,
+		 tmpbuf.data, tmpbuf.length, &pwd) != 0
 	 && (errval = errno) == ERANGE)
-    {
-      errno = 0;
-
-      if (__glibc_unlikely (buflen > 32768))
-	{
-	  char *old_buffer = buffer;
-	  buflen *= 2;
-	  buffer = (char *) realloc (use_malloc ? buffer : NULL, buflen);
-	  if (buffer == NULL)
-	    {
-	      /* We ran out of memory.  We cannot do anything but
-		 sending a negative response.  In reality this should
-		 never happen.  */
-	      pwd = NULL;
-	      buffer = old_buffer;
-
-	      /* We set the error to indicate this is (possibly) a
-		 temporary error and that it does not mean the entry
-		 is not available at all.  */
-	      errval = EAGAIN;
-	      break;
-	    }
-	  use_malloc = true;
-	}
-      else
-	/* Allocate a new buffer on the stack.  If possible combine it
-	   with the previously allocated buffer.  */
-	buffer = (char *) extend_alloca (buffer, buflen, 2 * buflen);
-    }
+    if (!scratch_buffer_grow (&tmpbuf))
+      {
+	/* We ran out of memory.  We cannot do anything but sending a
+	   negative response.  In reality this should never
+	   happen.  */
+	pwd = NULL;
+	/* We set the error to indicate this is (possibly) a temporary
+	   error and that it does not mean the entry is not available
+	   at all.  */
+	errval = EAGAIN;
+	break;
+      }
 
   /* Add the entry to the cache.  */
   time_t timeout = cache_addpw (db, fd, req, keystr, pwd, c_uid, he, dh,
 				errval);
-
-  if (use_malloc)
-    free (buffer);
-
+  scratch_buffer_free (&tmpbuf);
   return timeout;
 }
 
diff --git a/nscd/servicescache.c b/nscd/servicescache.c
index 187207f7ea..f71c1a608a 100644
--- a/nscd/servicescache.c
+++ b/nscd/servicescache.c
@@ -16,7 +16,6 @@
    You should have received a copy of the GNU General Public License
    along with this program; if not, see <http://www.gnu.org/licenses/>.  */
 
-#include <alloca.h>
 #include <assert.h>
 #include <errno.h>
 #include <libintl.h>
@@ -25,6 +24,7 @@
 #include <stdint.h>
 #include <sys/mman.h>
 #include <kernel-features.h>
+#include <scratch_buffer.h>
 
 #include "nscd.h"
 #include "dbg_log.h"
@@ -346,12 +346,11 @@ addservbyX (struct database_dyn *db, int fd, request_header *req,
      look again in the table whether the dataset is now available.  We
      simply insert it.  It does not matter if it is in there twice.  The
      pruning function only will look at the timestamp.  */
-  size_t buflen = 1024;
-  char *buffer = (char *) alloca (buflen);
   struct servent resultbuf;
   struct servent *serv;
-  bool use_malloc = false;
   int errval = 0;
+  struct scratch_buffer tmpbuf;
+  scratch_buffer_init (&tmpbuf);
 
   if (__glibc_unlikely (debug_level > 0))
     {
@@ -361,43 +360,24 @@ addservbyX (struct database_dyn *db, int fd, request_header *req,
 	dbg_log (_("Reloading \"%s\" in services cache!"), key);
     }
 
-  while (lookup (req->type, key, &resultbuf, buffer, buflen, &serv) != 0
+  while (lookup (req->type, key, &resultbuf,
+		 tmpbuf.data, tmpbuf.length, &serv) != 0
 	 && (errval = errno) == ERANGE)
-    {
-      errno = 0;
-
-      if (__glibc_unlikely (buflen > 32768))
-	{
-	  char *old_buffer = buffer;
-	  buflen *= 2;
-	  buffer = (char *) realloc (use_malloc ? buffer : NULL, buflen);
-	  if (buffer == NULL)
-	    {
-	      /* We ran out of memory.  We cannot do anything but
-		 sending a negative response.  In reality this should
-		 never happen.  */
-	      serv = NULL;
-	      buffer = old_buffer;
-
-	      /* We set the error to indicate this is (possibly) a
-		 temporary error and that it does not mean the entry
-		 is not available at all.  */
-	      errval = EAGAIN;
-	      break;
-	    }
-	  use_malloc = true;
-	}
-      else
-	/* Allocate a new buffer on the stack.  If possible combine it
-	   with the previously allocated buffer.  */
-	buffer = (char *) extend_alloca (buffer, buflen, 2 * buflen);
-    }
+    if (!scratch_buffer_grow (&tmpbuf))
+      {
+	/* We ran out of memory.  We cannot do anything but sending a
+	   negative response.  In reality this should never
+	   happen.  */
+	serv = NULL;
+	/* We set the error to indicate this is (possibly) a temporary
+	   error and that it does not mean the entry is not available
+	   at all.  */
+	errval = EAGAIN;
+	break;
+      }
 
   time_t timeout = cache_addserv (db, fd, req, key, serv, uid, he, dh, errval);
-
-  if (use_malloc)
-    free (buffer);
-
+  scratch_buffer_free (&tmpbuf);
   return timeout;
 }
 

Re: [PATCH] nscd: Use struct scratch_buffer, not extend_alloca in most caches [BZ #18023]

[Andreas Schwab]

On Jun 25 2018, fweimer@redhat.com (Florian Weimer) wrote:

> 	[BZ #18023]
> 	* nscd/grpcache.c (addgrbyX): Use struct scratch_buffer instead
> 	of extend_alloca.
> 	* nscd/hstcache.c (addhstbyX): Likewise.
> 	* nscd/pwdcache.c (addpwbyX): Likewise.
> 	* nscd/servicescache.c (addservbyX): Likewise.

Ok.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."