aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSiddhesh Poyarekar <siddhesh@redhat.com>2014-01-16 10:20:22 +0530
committerSiddhesh Poyarekar <siddhesh@redhat.com>2014-01-16 10:21:52 +0530
commit980cb5180e1b71224a57ca52b995c959b7148c09 (patch)
tree132d22c6a5d1ea9993dcc3bf721f089b40857e49
parent2393fc0119fa291ff01b7b912dda2069257c8600 (diff)
downloadglibc-980cb5180e1b71224a57ca52b995c959b7148c09.tar
glibc-980cb5180e1b71224a57ca52b995c959b7148c09.tar.gz
glibc-980cb5180e1b71224a57ca52b995c959b7148c09.tar.bz2
glibc-980cb5180e1b71224a57ca52b995c959b7148c09.zip
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.
-rw-r--r--ChangeLog5
-rw-r--r--NEWS2
-rw-r--r--nscd/netgroupcache.c37
3 files changed, 12 insertions, 32 deletions
diff --git a/ChangeLog b/ChangeLog
index 703b0ee00f..a61bfde918 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2013-01-16 Siddhesh Poyarekar <siddhesh@redhat.com>
+
+ [BZ #16453]
+ * nscd/netgroupcache.c (addgetnetgrentX): Don't use alloca.
+
2014-01-15 Adhemerval Zanella <azanella@linux.vnet.ibm.com>
* sysdeps/powerpc/sotruss-lib.c: New file: sotruss-lib.so
diff --git a/NEWS b/NEWS
index f406522882..248f2c30ad 100644
--- a/NEWS
+++ b/NEWS
@@ -25,7 +25,7 @@ Version 2.19
16151, 16153, 16167, 16172, 16195, 16214, 16245, 16271, 16274, 16283,
16289, 16293, 16314, 16316, 16330, 16337, 16338, 16356, 16365, 16366,
16369, 16372, 16375, 16379, 16384, 16385, 16386, 16387, 16390, 16394,
- 16400, 16407, 16408, 16414.
+ 16400, 16407, 16408, 16414, 16453.
* Slovenian translations for glibc messages have been contributed by the
Translation Project's Slovenian team of translators.
diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c
index 9fc16640ae..58234b1492 100644
--- a/nscd/netgroupcache.c
+++ b/nscd/netgroupcache.c
@@ -141,7 +141,6 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
size_t buffilled = sizeof (*dataset);
char *buffer = NULL;
size_t nentries = 0;
- bool use_malloc = false;
size_t group_len = strlen (key) + 1;
union
{
@@ -159,7 +158,7 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
}
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;
@@ -241,21 +240,8 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
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);
+ buffer = xrealloc (buffer, buflen);
}
nhost = memcpy (buffer + bufused,
@@ -322,18 +308,8 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
}
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);
}
}
@@ -478,8 +454,7 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
}
out:
- if (use_malloc)
- free (buffer);
+ free (buffer);
*resultp = dataset;