diff options
author | Richard Levitte <levitte@openssl.org> | 2017-12-12 02:05:38 +0100 |
---|---|---|
committer | Richard Levitte <levitte@openssl.org> | 2017-12-12 18:03:46 +0100 |
commit | 2717f2b7eb845594271a94969767af2c4521e004 (patch) | |
tree | ddd19a234c2497488b7d56b17323a208abd6715b /crypto/err | |
parent | 0aa0e13a6a367dc27bfa59bd2ab1e90645c3158b (diff) |
Fix leak in ERR_get_state() when OPENSSL_init_crypto() isn't called yet
If OPENSSL_init_crypto() hasn't been called yet when ERR_get_state()
is called, it need to be called early, so the base initialization is
done. On some platforms (those who support DSO functionality and
don't define OPENSSL_USE_NODELETE), that includes a call of
ERR_set_mark(), which calls this function again.
Furthermore, we know that ossl_init_thread_start(), which is called
later in ERR_get_state(), calls OPENSSL_init_crypto(0, NULL), except
that's too late.
Here's what happens without an early call of OPENSSL_init_crypto():
=> ERR_get_state():
=> CRYPTO_THREAD_get_local():
<= NULL;
# no state is found, so it gets allocated.
=> ossl_init_thread_start():
=> OPENSSL_init_crypto():
# Here, base_inited is set to 1
# before ERR_set_mark() call
=> ERR_set_mark():
=> ERR_get_state():
=> CRYPTO_THREAD_get_local():
<= NULL;
# no state is found, so it gets allocated!!!!!
=> ossl_init_thread_start():
=> OPENSSL_init_crypto():
# base_inited is 1,
# so no more init to be done
<= 1
<=
=> CRYPTO_thread_set_local():
<=
<=
<=
<= 1
<=
=> CRYPTO_thread_set_local() # previous value removed!
<=
Result: double allocation, and we have a leak.
By calling the base OPENSSL_init_crypto() early, we get this instead:
=> ERR_get_state():
=> OPENSSL_init_crypto():
# Here, base_inited is set to 1
# before ERR_set_mark() call
=> ERR_set_mark():
=> ERR_get_state():
=> OPENSSL_init_crypto():
# base_inited is 1,
# so no more init to be done
<= 1
=> CRYPTO_THREAD_get_local():
<= NULL;
# no state is found, so it gets allocated
# let's assume we got 0xDEADBEEF
=> ossl_init_thread_start():
=> OPENSSL_init_crypto():
# base_inited is 1,
# so no more init to be done
<= 1
<= 1
=> CRYPTO_thread_set_local():
<=
<=
<=
<= 1
=> CRYPTO_THREAD_get_local():
<= 0xDEADBEEF
<= 0xDEADBEEF
Result: no leak.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4913)
(cherry picked from commit aef84bb4efbddfd95d042f3f5f1d362ed7d4faeb)
Diffstat (limited to 'crypto/err')
-rw-r--r-- | crypto/err/err.c | 8 |
1 files changed, 8 insertions, 0 deletions
diff --git a/crypto/err/err.c b/crypto/err/err.c index 84eabc1772..c4399285fe 100644 --- a/crypto/err/err.c +++ b/crypto/err/err.c @@ -658,6 +658,14 @@ ERR_STATE *ERR_get_state(void) if (!RUN_ONCE(&err_init, err_do_init)) return NULL; + /* + * If base OPENSSL_init_crypto() hasn't been called yet, be sure to call + * it now to avoid state to be doubly allocated and thereby leak memory. + * Needed on any platform that doesn't define OPENSSL_USE_NODELETE. + */ + if (!OPENSSL_init_crypto(0, NULL)) + return NULL; + state = CRYPTO_THREAD_get_local(&err_thread_local); if (state == NULL) { |