| 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 |
| |
| |
| @@ -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; |
| |