commit af37a8a3496327a6e5617a2c76f17aa1e8db835e

Author: Siddhesh Poyarekar <siddhesh@redhat.com>

Date:   Mon Jan 27 11:32:44 2014 +0530

    Avoid undefined behaviour in netgroupcache

    Using a buffer after it has been reallocated is undefined behaviour,

    so get offsets of the triplets in the old buffer before reallocating

    it.

commit 5d41dadf31bc8a2f9c34c40d52a442d3794e405c

Author: Siddhesh Poyarekar <siddhesh@redhat.com>

Date:   Fri Jan 24 13:51:15 2014 +0530

    Adjust pointers to triplets in netgroup query data (BZ #16474)

    The _nss_*_getnetgrent_r query populates the netgroup results in the

    allocated buffer and then sets the result triplet to point to strings

    in the buffer.  This is a problem when the buffer is reallocated since

    the pointers to the triplet strings are no longer valid.  The pointers

    need to be adjusted so that they now point to strings in the

    reallocated buffer.

commit 980cb5180e1b71224a57ca52b995c959b7148c09

Author: Siddhesh Poyarekar <siddhesh@redhat.com>

Date:   Thu Jan 16 10:20:22 2014 +0530

    Don't use alloca in addgetnetgrentX (BZ #16453)

    addgetnetgrentX has a buffer which is grown as per the needs of the

    requested size either by using alloca or by falling back to malloc if

    the size is larger than 1K.  There are two problems with the alloca

    bits: firstly, it doesn't really extend the buffer since it does not

    use the return value of the extend_alloca macro, which is the location

    of the reallocated buffer.  Due to this the buffer does not actually

    extend itself and hence a subsequent write may overwrite stuff on the

    stack.

    The second problem is more subtle - the buffer growth on the stack is

    discontinuous due to block scope local variables.  Combine that with

    the fact that unlike realloc, extend_alloca does not copy over old

    content and you have a situation where the buffer just has garbage in

    the space where it should have had data.

    This could have been fixed by adding code to copy over old data

    whenever we call extend_alloca, but it seems unnecessarily

    complicated.  This code is not exactly a performance hotspot (it's

    called when there is a cache miss, so factors like network lookup or

    file reads will dominate over memory allocation/reallocation), so this

    premature optimization is unnecessary.

    Thanks Brad Hubbard <bhubbard@redhat.com> for his help with debugging

    the problem.

diff -pruN glibc-2.17-c758a686/nscd/netgroupcache.c glibc-2.17-c758a686/nscd/netgroupcache.c

--- glibc-2.17-c758a686/nscd/netgroupcache.c	2014-04-09 12:13:58.618582111 +0530

+++ glibc-2.17-c758a686/nscd/netgroupcache.c	2014-04-09 12:07:21.486598665 +0530

@@ -93,7 +93,6 @@ addgetnetgrentX (struct database_dyn *db

   size_t buffilled = sizeof (*dataset);

   char *buffer = NULL;

   size_t nentries = 0;

-  bool use_malloc = false;

   size_t group_len = strlen (key) + 1;

   union

   {

@@ -138,7 +137,7 @@ addgetnetgrentX (struct database_dyn *db

     }

   memset (&data, '\0', sizeof (data));

-  buffer = alloca (buflen);

+  buffer = xmalloc (buflen);

   first_needed.elem.next = &first_needed.elem;

   memcpy (first_needed.elem.name, key, group_len);

   data.needed_groups = &first_needed.elem;

@@ -218,21 +217,24 @@ addgetnetgrentX (struct database_dyn *db

 				if (buflen - req->key_len - bufused < needed)

 				  {

-				    size_t newsize = MAX (2 * buflen,

-							  buflen + 2 * needed);

-				    if (use_malloc || newsize > 1024 * 1024)

-				      {

-					buflen = newsize;

-					char *newbuf = xrealloc (use_malloc

-								 ? buffer

-								 : NULL,

-								 buflen);

-

-					buffer = newbuf;

-					use_malloc = true;

-				      }

-				    else

-				      extend_alloca (buffer, buflen, newsize);

+				    buflen += MAX (buflen, 2 * needed);

+				    /* Save offset in the old buffer.  We don't

+				       bother with the NULL check here since

+				       we'll do that later anyway.  */

+				    size_t nhostdiff = nhost - buffer;

+				    size_t nuserdiff = nuser - buffer;

+				    size_t ndomaindiff = ndomain - buffer;

+

+				    char *newbuf = xrealloc (buffer, buflen);

+				    /* Fix up the triplet pointers into the new

+				       buffer.  */

+				    nhost = (nhost ? newbuf + nhostdiff

+					     : NULL);

+				    nuser = (nuser ? newbuf + nuserdiff

+					     : NULL);

+				    ndomain = (ndomain ? newbuf + ndomaindiff

+					       : NULL);

+				    buffer = newbuf;

 				  }

 				nhost = memcpy (buffer + bufused,

@@ -299,18 +301,8 @@ addgetnetgrentX (struct database_dyn *db

 		      }

 		    else if (status == NSS_STATUS_UNAVAIL && e == ERANGE)

 		      {

-			size_t newsize = 2 * buflen;

-			if (use_malloc || newsize > 1024 * 1024)

-			  {

-			    buflen = newsize;

-			    char *newbuf = xrealloc (use_malloc

-						     ? buffer : NULL, buflen);

-

-			    buffer = newbuf;

-			    use_malloc = true;

-			  }

-			else

-			  extend_alloca (buffer, buflen, newsize);

+			buflen *= 2;

+			buffer = xrealloc (buffer, buflen);

 		      }

 		  }

@@ -446,8 +438,7 @@ addgetnetgrentX (struct database_dyn *db

     }

  out:

-  if (use_malloc)

-    free (buffer);

+  free (buffer);

   *resultp = dataset;