]>
Commit | Line | Data |
---|---|---|
bb330e25 AF |
1 | commit af37a8a3496327a6e5617a2c76f17aa1e8db835e |
2 | Author: Siddhesh Poyarekar <siddhesh@redhat.com> | |
3 | Date: Mon Jan 27 11:32:44 2014 +0530 | |
4 | ||
5 | Avoid undefined behaviour in netgroupcache | |
6 | ||
7 | Using a buffer after it has been reallocated is undefined behaviour, | |
8 | so get offsets of the triplets in the old buffer before reallocating | |
9 | it. | |
10 | ||
11 | commit 5d41dadf31bc8a2f9c34c40d52a442d3794e405c | |
12 | Author: Siddhesh Poyarekar <siddhesh@redhat.com> | |
13 | Date: Fri Jan 24 13:51:15 2014 +0530 | |
14 | ||
15 | Adjust pointers to triplets in netgroup query data (BZ #16474) | |
16 | ||
17 | The _nss_*_getnetgrent_r query populates the netgroup results in the | |
18 | allocated buffer and then sets the result triplet to point to strings | |
19 | in the buffer. This is a problem when the buffer is reallocated since | |
20 | the pointers to the triplet strings are no longer valid. The pointers | |
21 | need to be adjusted so that they now point to strings in the | |
22 | reallocated buffer. | |
23 | ||
24 | commit 980cb5180e1b71224a57ca52b995c959b7148c09 | |
25 | Author: Siddhesh Poyarekar <siddhesh@redhat.com> | |
26 | Date: Thu Jan 16 10:20:22 2014 +0530 | |
27 | ||
28 | Don't use alloca in addgetnetgrentX (BZ #16453) | |
29 | ||
30 | addgetnetgrentX has a buffer which is grown as per the needs of the | |
31 | requested size either by using alloca or by falling back to malloc if | |
32 | the size is larger than 1K. There are two problems with the alloca | |
33 | bits: firstly, it doesn't really extend the buffer since it does not | |
34 | use the return value of the extend_alloca macro, which is the location | |
35 | of the reallocated buffer. Due to this the buffer does not actually | |
36 | extend itself and hence a subsequent write may overwrite stuff on the | |
37 | stack. | |
38 | ||
39 | The second problem is more subtle - the buffer growth on the stack is | |
40 | discontinuous due to block scope local variables. Combine that with | |
41 | the fact that unlike realloc, extend_alloca does not copy over old | |
42 | content and you have a situation where the buffer just has garbage in | |
43 | the space where it should have had data. | |
44 | ||
45 | This could have been fixed by adding code to copy over old data | |
46 | whenever we call extend_alloca, but it seems unnecessarily | |
47 | complicated. This code is not exactly a performance hotspot (it's | |
48 | called when there is a cache miss, so factors like network lookup or | |
49 | file reads will dominate over memory allocation/reallocation), so this | |
50 | premature optimization is unnecessary. | |
51 | ||
52 | Thanks Brad Hubbard <bhubbard@redhat.com> for his help with debugging | |
53 | the problem. | |
54 | ||
55 | diff -pruN glibc-2.12-2-gc4ccff1/nscd/netgroupcache.c glibc-2.12-2-gc4ccff1.patched/nscd/netgroupcache.c | |
56 | --- glibc-2.12-2-gc4ccff1/nscd/netgroupcache.c 2014-04-09 12:13:58.618582111 +0530 | |
57 | +++ glibc-2.12-2-gc4ccff1.patched/nscd/netgroupcache.c 2014-04-09 12:07:21.486598665 +0530 | |
58 | @@ -93,7 +93,6 @@ addgetnetgrentX (struct database_dyn *db | |
59 | size_t buffilled = sizeof (*dataset); | |
60 | char *buffer = NULL; | |
61 | size_t nentries = 0; | |
62 | - bool use_malloc = false; | |
63 | size_t group_len = strlen (key) + 1; | |
64 | union | |
65 | { | |
66 | @@ -138,7 +137,7 @@ addgetnetgrentX (struct database_dyn *db | |
67 | } | |
68 | ||
69 | memset (&data, '\0', sizeof (data)); | |
70 | - buffer = alloca (buflen); | |
71 | + buffer = xmalloc (buflen); | |
72 | first_needed.elem.next = &first_needed.elem; | |
73 | memcpy (first_needed.elem.name, key, group_len); | |
74 | data.needed_groups = &first_needed.elem; | |
75 | @@ -218,21 +217,24 @@ addgetnetgrentX (struct database_dyn *db | |
76 | ||
77 | if (buflen - req->key_len - bufused < needed) | |
78 | { | |
79 | - size_t newsize = MAX (2 * buflen, | |
80 | - buflen + 2 * needed); | |
81 | - if (use_malloc || newsize > 1024 * 1024) | |
82 | - { | |
83 | - buflen = newsize; | |
84 | - char *newbuf = xrealloc (use_malloc | |
85 | - ? buffer | |
86 | - : NULL, | |
87 | - buflen); | |
88 | - | |
89 | - buffer = newbuf; | |
90 | - use_malloc = true; | |
91 | - } | |
92 | - else | |
93 | - extend_alloca (buffer, buflen, newsize); | |
94 | + buflen += MAX (buflen, 2 * needed); | |
95 | + /* Save offset in the old buffer. We don't | |
96 | + bother with the NULL check here since | |
97 | + we'll do that later anyway. */ | |
98 | + size_t nhostdiff = nhost - buffer; | |
99 | + size_t nuserdiff = nuser - buffer; | |
100 | + size_t ndomaindiff = ndomain - buffer; | |
101 | + | |
102 | + char *newbuf = xrealloc (buffer, buflen); | |
103 | + /* Fix up the triplet pointers into the new | |
104 | + buffer. */ | |
105 | + nhost = (nhost ? newbuf + nhostdiff | |
106 | + : NULL); | |
107 | + nuser = (nuser ? newbuf + nuserdiff | |
108 | + : NULL); | |
109 | + ndomain = (ndomain ? newbuf + ndomaindiff | |
110 | + : NULL); | |
111 | + buffer = newbuf; | |
112 | } | |
113 | ||
114 | nhost = memcpy (buffer + bufused, | |
115 | @@ -299,18 +301,8 @@ addgetnetgrentX (struct database_dyn *db | |
116 | } | |
117 | else if (status == NSS_STATUS_UNAVAIL && e == ERANGE) | |
118 | { | |
119 | - size_t newsize = 2 * buflen; | |
120 | - if (use_malloc || newsize > 1024 * 1024) | |
121 | - { | |
122 | - buflen = newsize; | |
123 | - char *newbuf = xrealloc (use_malloc | |
124 | - ? buffer : NULL, buflen); | |
125 | - | |
126 | - buffer = newbuf; | |
127 | - use_malloc = true; | |
128 | - } | |
129 | - else | |
130 | - extend_alloca (buffer, buflen, newsize); | |
131 | + buflen *= 2; | |
132 | + buffer = xrealloc (buffer, buflen); | |
133 | } | |
134 | } | |
135 | ||
136 | @@ -446,8 +438,7 @@ addgetnetgrentX (struct database_dyn *db | |
137 | } | |
138 | ||
139 | out: | |
140 | - if (use_malloc) | |
141 | - free (buffer); | |
142 | + free (buffer); | |
143 | ||
144 | *resultp = dataset; | |
145 |