Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(226)

Side by Side Diff: patches.chromium/0006-fix_lhash_iteration.patch

Issue 57833003: Fix lhash implementation to avoid unwanted resizes during iteration. (Closed) Base URL: https://chromium.googlesource.com/chromium/deps/openssl@master
Patch Set: Update to latest upstream version of the patch + update README.Chromium Created 7 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « openssl/ssl/ssl_sess.c ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
(Empty)
1 diff -burN android-openssl.org/openssl.config android-openssl/openssl.config
2 --- android-openssl.org/openssl.config 2013-11-05 10:26:23.008411507 +0100
3 +++ android-openssl/openssl.config 2013-11-05 10:28:13.409668262 +0100
4 @@ -997,6 +997,7 @@
5 fix_clang_build.patch \
6 x509_hash_name_algorithm_change.patch \
7 reduce_client_hello_size.patch \
8 +fix_lhash_iteration.patch \
9 "
10
11 OPENSSL_PATCHES_progs_SOURCES="\
12 @@ -1060,3 +1061,13 @@
13 OPENSSL_PATCHES_reduce_client_hello_size_SOURCES="\
14 ssl/t1_lib.c \
15 "
16 +
17 +OPENSSL_PATCHES_fix_lhash_iteration_SOURCES="\
18 +crypto/conf/conf_api.c
19 +crypto/lhash/lhash.c
20 +crypto/lhash/lhash.h
21 +crypto/objects/o_names.c
22 +crypto/objects/obj_dat.c
23 +include/openssl/lhash.h
24 +ssl/ssl_sess.c
25 +"
26 diff -burN android-openssl.org/patches/fix_lhash_iteration.patch android-openssl /patches/fix_lhash_iteration.patch
27 --- android-openssl.org/patches/fix_lhash_iteration.patch 1970-01-01 01:00 :00.000000000 +0100
28 +++ android-openssl/patches/fix_lhash_iteration.patch 2013-11-05 10:26:40.6786 12658 +0100
29 @@ -0,0 +1,291 @@
30 +diff --git a/crypto/conf/conf_api.c b/crypto/conf/conf_api.c
31 +index f5fcbb9..9a37bd4 100644
32 +--- a/crypto/conf/conf_api.c
33 ++++ b/crypto/conf/conf_api.c
34 +@@ -225,9 +225,6 @@ void _CONF_free_data(CONF *conf)
35 + {
36 + if (conf == NULL || conf->data == NULL) return;
37 +
38 +- lh_CONF_VALUE_down_load(conf->data)=0; /* evil thing to make
39 +- * sure the 'OPENSSL_free()' works as
40 +- * expected */
41 + lh_CONF_VALUE_doall_arg(conf->data,
42 + LHASH_DOALL_ARG_FN(value_free_hash),
43 + LHASH_OF(CONF_VALUE), conf->data);
44 +diff --git a/crypto/lhash/lhash.c b/crypto/lhash/lhash.c
45 +index 47f7480..e5b3cdf 100644
46 +--- a/crypto/lhash/lhash.c
47 ++++ b/crypto/lhash/lhash.c
48 +@@ -94,6 +94,7 @@
49 + *
50 + * 1.0 eay - First version
51 + */
52 ++#include <limits.h>
53 + #include <stdio.h>
54 + #include <string.h>
55 + #include <stdlib.h>
56 +@@ -107,6 +108,113 @@ const char lh_version[]="lhash" OPENSSL_VERSION_PTEXT;
57 + #define UP_LOAD (2*LH_LOAD_MULT) /* load times 256 (default 2) */
58 + #define DOWN_LOAD (LH_LOAD_MULT) /* load times 256 (default 1) */
59 +
60 ++/* Maximum number of nodes to guarantee the load computations don't overflow * /
61 ++#define MAX_LOAD_ITEMS (UINT_MAX / LH_LOAD_MULT)
62 ++
63 ++/* The field 'iteration_state' is used to hold data to ensure that a hash
64 ++ * table is not resized during an 'insert' or 'delete' operation performed
65 ++ * within a lh_doall/lh_doall_arg call.
66 ++ *
67 ++ * Conceptually, this records two things:
68 ++ *
69 ++ * - A 'depth' count, which is incremented at the start of lh_doall*,
70 ++ * and decremented just before it returns.
71 ++ *
72 ++ * - A 'mutated' boolean flag, which is set in lh_insert() or lh_delete()
73 ++ * when the operation is performed with a non-0 depth.
74 ++ *
75 ++ * The following are helper macros to handle this state in a more explicit
76 ++ * way.
77 ++ */
78 ++
79 ++/* Reset the iteration state to its defaults. */
80 ++#define LH_ITERATION_RESET(lh) do { \
81 ++ (lh)->iteration_state = 0; \
82 ++ } while (0)
83 ++
84 ++/* Returns 1 if the hash table is currently being iterated on, 0 otherwise. */
85 ++#define LH_ITERATION_IS_ACTIVE(lh) ((lh)->iteration_state >= 2)
86 ++
87 ++/* Increment iteration depth. This should always be followed by a paired call
88 ++ * to LH_ITERATION_DECREMENT_DEPTH(). */
89 ++#define LH_ITERATION_INCREMENT_DEPTH(lh) do { \
90 ++ (lh)->iteration_state += 2; \
91 ++ } while (0)
92 ++
93 ++/* Decrement iteration depth. This should always be called after a paired call
94 ++ * to LH_ITERATION_INCREMENT_DEPTH(). */
95 ++#define LH_ITERATION_DECREMENT_DEPTH(lh) do { \
96 ++ (lh)->iteration_state -= 2; \
97 ++ } while (0)
98 ++
99 ++/* Return 1 if the iteration 'mutated' flag is set, 0 otherwise. */
100 ++#define LH_ITERATION_IS_MUTATED(lh) (((lh)->iteration_state & 1) != 0)
101 ++
102 ++/* Set the iteration 'mutated' flag to 1. LH_ITERATION_RESET() to reset it. */
103 ++#define LH_ITERATION_SET_MUTATED(lh) do { \
104 ++ (lh)->iteration_state |= 1; \
105 ++ } while (0)
106 ++
107 ++/* This macro returns 1 if the hash table should be expanded due to its curren t
108 ++ * load, or 0 otherwise. The exact comparison to be performed is expressed by
109 ++ * the mathematical expression (where '//' denotes division over real numbers) :
110 ++ *
111 ++ * (num_items // num_nodes) >= (up_load // LOAD_MULT) or
112 ++ * (num_items * LOAD_MULT // num_nodes) >= up_load.
113 ++ *
114 ++ * Given that the C language operator '/' implements integer division, i.e:
115 ++ * a // b == (a / b) + epsilon (with 0 <= epsilon < 1, for positive a & b )
116 ++ *
117 ++ * This can be rewritten as:
118 ++ * (num_items * LOAD_MULT / num_nodes) + epsilon >= up_load
119 ++ * (num_items * LOAD_MULT / num_nodes) - up_load >= - epsilon
120 ++ *
121 ++ * Let's call 'A' the left-hand side of the equation above, it is an integer
122 ++ * and:
123 ++ * - If A >= 0, the expression is true for any value of epsilon.
124 ++ * - If A <= -1, the expression is also true for any value of epsilon.
125 ++ *
126 ++ * In other words, this is equivalent to 'A >= 0', or:
127 ++ * (num_items * LOAD_MULT / num_nodes) >= up_load
128 ++ */
129 ++#define LH_SHOULD_EXPAND(lh) \
130 ++ ((lh)->num_items < MAX_LOAD_ITEMS && \
131 ++ (((lh)->num_items*LH_LOAD_MULT/(lh)->num_nodes) >= (lh)->up_load))
132 ++
133 ++/* This macro returns 1 if the hash table should be contracted due to its
134 ++ * current load, or 0 otherwise. Abbreviated computations are:
135 ++ *
136 ++ * (num_items // num_nodes) <= (down_load // LOAD_MULT)
137 ++ * (num_items * LOAD_MULT // num_nodes) <= down_load
138 ++ * (num_items * LOAD_MULT / num_nodes) + epsilon <= down_load
139 ++ * (num_items * LOAD_MULT / num_nodes) - down_load <= -epsilon
140 ++ *
141 ++ * Let's call 'B' the left-hand side of the equation above:
142 ++ * - If B <= -1, the expression is true for any value of epsilon.
143 ++ * - If B >= 1, the expression is false for any value of epsilon.
144 ++ * - If B == 0, the expression is true for 'epsilon == 0', and false
145 ++ * otherwise, which is problematic.
146 ++ *
147 ++ * To work around this problem, while keeping the code simple, just change
148 ++ * the initial expression to use a strict inequality, i.e.:
149 ++ *
150 ++ * (num_items // num_nodes) < (down_load // LOAD_MULT)
151 ++ *
152 ++ * Which leads to:
153 ++ * (num_items * LOAD_MULT / num_nodes) - down_load < -epsilon
154 ++ *
155 ++ * Then:
156 ++ * - If 'B <= -1', the expression is true for any value of epsilon.
157 ++ * - If 'B' >= 0, the expression is false for any value of epsilon,
158 ++ *
159 ++ * In other words, this is equivalent to 'B < 0', or:
160 ++ * (num_items * LOAD_MULT / num_nodes) < down_load
161 ++ */
162 ++#define LH_SHOULD_CONTRACT(lh) \
163 ++ (((lh)->num_nodes > MIN_NODES) && \
164 ++ ((lh)->num_items < MAX_LOAD_ITEMS && \
165 ++ ((lh)->num_items*LH_LOAD_MULT/(lh)->num_nodes) < (lh)->down_load))
166 ++
167 + static void expand(_LHASH *lh);
168 + static void contract(_LHASH *lh);
169 + static LHASH_NODE **getrn(_LHASH *lh, const void *data, unsigned long *rhash);
170 +@@ -147,6 +255,7 @@ _LHASH *lh_new(LHASH_HASH_FN_TYPE h, LHASH_COMP_FN_TYPE c)
171 + ret->num_hash_comps=0;
172 +
173 + ret->error=0;
174 ++ LH_ITERATION_RESET(ret);
175 + return(ret);
176 + err1:
177 + OPENSSL_free(ret);
178 +@@ -183,7 +292,10 @@ void *lh_insert(_LHASH *lh, void *data)
179 + void *ret;
180 +
181 + lh->error=0;
182 +- if (lh->up_load <= (lh->num_items*LH_LOAD_MULT/lh->num_nodes))
183 ++ /* Do not expand the array if the table is being iterated on. */
184 ++ if (LH_ITERATION_IS_ACTIVE(lh))
185 ++ LH_ITERATION_SET_MUTATED(lh);
186 ++ else if (LH_SHOULD_EXPAND(lh))
187 + expand(lh);
188 +
189 + rn=getrn(lh,data,&hash);
190 +@@ -238,8 +350,10 @@ void *lh_delete(_LHASH *lh, const void *data)
191 + }
192 +
193 + lh->num_items--;
194 +- if ((lh->num_nodes > MIN_NODES) &&
195 +- (lh->down_load >= (lh->num_items*LH_LOAD_MULT/lh->num_nodes)))
196 ++ /* Do not contract the array if the table is being iterated on. */
197 ++ if (LH_ITERATION_IS_ACTIVE(lh))
198 ++ LH_ITERATION_SET_MUTATED(lh);
199 ++ else if (LH_SHOULD_CONTRACT(lh))
200 + contract(lh);
201 +
202 + return(ret);
203 +@@ -276,6 +390,7 @@ static void doall_util_fn(_LHASH *lh, int use_arg, LHASH_DO ALL_FN_TYPE func,
204 + if (lh == NULL)
205 + return;
206 +
207 ++ LH_ITERATION_INCREMENT_DEPTH(lh);
208 + /* reverse the order so we search from 'top to bottom'
209 + * We were having memory leaks otherwise */
210 + for (i=lh->num_nodes-1; i>=0; i--)
211 +@@ -283,10 +398,7 @@ static void doall_util_fn(_LHASH *lh, int use_arg, LHASH_D OALL_FN_TYPE func,
212 + a=lh->b[i];
213 + while (a != NULL)
214 + {
215 +- /* 28/05/91 - eay - n added so items can be deleted
216 +- * via lh_doall */
217 +- /* 22/05/08 - ben - eh? since a is not passed,
218 +- * this should not be needed */
219 ++ /* note that 'a' can be deleted by the callback */
220 + n=a->next;
221 + if(use_arg)
222 + func_arg(a->data,arg);
223 +@@ -295,6 +407,19 @@ static void doall_util_fn(_LHASH *lh, int use_arg, LHASH_D OALL_FN_TYPE func,
224 + a=n;
225 + }
226 + }
227 ++
228 ++ LH_ITERATION_DECREMENT_DEPTH(lh);
229 ++ if (!LH_ITERATION_IS_ACTIVE(lh) && LH_ITERATION_IS_MUTATED(lh))
230 ++ {
231 ++ LH_ITERATION_RESET(lh);
232 ++ /* Resize the buckets array if necessary. Each expand() or
233 ++ * contract() call will double/halve the size of the array,
234 ++ * respectively, so call them in a loop. */
235 ++ while (LH_SHOULD_EXPAND(lh))
236 ++ expand(lh);
237 ++ while (LH_SHOULD_CONTRACT(lh))
238 ++ contract(lh);
239 ++ }
240 + }
241 +
242 + void lh_doall(_LHASH *lh, LHASH_DOALL_FN_TYPE func)
243 +diff --git a/crypto/lhash/lhash.h b/crypto/lhash/lhash.h
244 +index e7d8763..75386f2 100644
245 +--- a/crypto/lhash/lhash.h
246 ++++ b/crypto/lhash/lhash.h
247 +@@ -163,6 +163,7 @@ typedef struct lhash_st
248 + unsigned long num_hash_comps;
249 +
250 + int error;
251 ++ int iteration_state;
252 + } _LHASH; /* Do not use _LHASH directly, use LHASH_OF
253 + * and friends */
254 +
255 +diff --git a/crypto/objects/o_names.c b/crypto/objects/o_names.c
256 +index 4a548c2..4e18045 100644
257 +--- a/crypto/objects/o_names.c
258 ++++ b/crypto/objects/o_names.c
259 +@@ -350,13 +350,9 @@ static void name_funcs_free(NAME_FUNCS *ptr)
260 +
261 + void OBJ_NAME_cleanup(int type)
262 + {
263 +- unsigned long down_load;
264 +-
265 + if (names_lh == NULL) return;
266 +
267 + free_type=type;
268 +- down_load=lh_OBJ_NAME_down_load(names_lh);
269 +- lh_OBJ_NAME_down_load(names_lh)=0;
270 +
271 + lh_OBJ_NAME_doall(names_lh,LHASH_DOALL_FN(names_lh_free));
272 + if (type < 0)
273 +@@ -366,7 +362,5 @@ void OBJ_NAME_cleanup(int type)
274 + names_lh=NULL;
275 + name_funcs_stack = NULL;
276 + }
277 +- else
278 +- lh_OBJ_NAME_down_load(names_lh)=down_load;
279 + }
280 +
281 +diff --git a/crypto/objects/obj_dat.c b/crypto/objects/obj_dat.c
282 +index 8a342ba..ef0512a 100644
283 +--- a/crypto/objects/obj_dat.c
284 ++++ b/crypto/objects/obj_dat.c
285 +@@ -227,7 +227,6 @@ void OBJ_cleanup(void)
286 + return ;
287 + }
288 + if (added == NULL) return;
289 +- lh_ADDED_OBJ_down_load(added) = 0;
290 + lh_ADDED_OBJ_doall(added,LHASH_DOALL_FN(cleanup1)); /* zero counters */
291 + lh_ADDED_OBJ_doall(added,LHASH_DOALL_FN(cleanup2)); /* set counters */
292 + lh_ADDED_OBJ_doall(added,LHASH_DOALL_FN(cleanup3)); /* free objects */
293 +diff --git a/include/openssl/lhash.h b/include/openssl/lhash.h
294 +index e7d8763..75386f2 100644
295 +--- a/include/openssl/lhash.h
296 ++++ b/include/openssl/lhash.h
297 +@@ -163,6 +163,7 @@ typedef struct lhash_st
298 + unsigned long num_hash_comps;
299 +
300 + int error;
301 ++ int iteration_state;
302 + } _LHASH; /* Do not use _LHASH directly, use LHASH_OF
303 + * and friends */
304 +
305 +diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
306 +index 920b763..447a37e 100644
307 +--- a/ssl/ssl_sess.c
308 ++++ b/ssl/ssl_sess.c
309 +@@ -999,11 +999,8 @@ void SSL_CTX_flush_sessions(SSL_CTX *s, long t)
310 + if (tp.cache == NULL) return;
311 + tp.time=t;
312 + CRYPTO_w_lock(CRYPTO_LOCK_SSL_CTX);
313 +- i=CHECKED_LHASH_OF(SSL_SESSION, tp.cache)->down_load;
314 +- CHECKED_LHASH_OF(SSL_SESSION, tp.cache)->down_load=0;
315 + lh_SSL_SESSION_doall_arg(tp.cache, LHASH_DOALL_ARG_FN(timeout),
316 + TIMEOUT_PARAM, &tp);
317 +- CHECKED_LHASH_OF(SSL_SESSION, tp.cache)->down_load=i;
318 + CRYPTO_w_unlock(CRYPTO_LOCK_SSL_CTX);
319 + }
320 +
OLDNEW
« no previous file with comments | « openssl/ssl/ssl_sess.c ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698