From 8cab4e9bc73a66b64aae179db86493fd28c39b64 Mon Sep 17 00:00:00 2001 From: Emilia Kasper Date: Sat, 12 Mar 2016 20:46:13 +0100 Subject: Fix memory leak in library deinit ENGINE_cleanup calls CRYPTO_free_ex_data and therefore, CRYPTO_cleanup_all_ex_data - which cleans up the method pointers - must run after ENGINE_cleanup. Additionally, don't needlessly initialize the EX_CALLBACKS stack during e.g. CRYPTO_free_ex_data. The only time this is actually needed is when reserving the first ex data index. Specifically, since sk_num returns -1 on NULL input, the rest of the code already handles a NULL method stack correctly. Reviewed-by: Rich Salz --- crypto/ex_data.c | 23 ++++++++++++----------- crypto/init.c | 13 ++++++++----- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/crypto/ex_data.c b/crypto/ex_data.c index de734d30aa..4af0a9d5b3 100644 --- a/crypto/ex_data.c +++ b/crypto/ex_data.c @@ -161,17 +161,6 @@ static EX_CALLBACKS *get_and_lock(int class_index) ip = &ex_data[class_index]; CRYPTO_THREAD_write_lock(ex_data_lock); - if (ip->meth == NULL) { - ip->meth = sk_EX_CALLBACK_new_null(); - /* We push an initial value on the stack because the SSL - * "app_data" routines use ex_data index zero. See RT 3710. */ - if (ip->meth == NULL - || !sk_EX_CALLBACK_push(ip->meth, NULL)) { - CRYPTOerr(CRYPTO_F_GET_AND_LOCK, ERR_R_MALLOC_FAILURE); - CRYPTO_THREAD_unlock(ex_data_lock); - return NULL; - } - } return ip; } @@ -255,6 +244,18 @@ int CRYPTO_get_ex_new_index(int class_index, long argl, void *argp, if (ip == NULL) return -1; + + if (ip->meth == NULL) { + ip->meth = sk_EX_CALLBACK_new_null(); + /* We push an initial value on the stack because the SSL + * "app_data" routines use ex_data index zero. See RT 3710. */ + if (ip->meth == NULL + || !sk_EX_CALLBACK_push(ip->meth, NULL)) { + CRYPTOerr(CRYPTO_F_GET_AND_LOCK, ERR_R_MALLOC_FAILURE); + goto err; + } + } + a = (EX_CALLBACK *)OPENSSL_malloc(sizeof(*a)); if (a == NULL) { CRYPTOerr(CRYPTO_F_CRYPTO_GET_EX_NEW_INDEX, ERR_R_MALLOC_FAILURE); diff --git a/crypto/init.c b/crypto/init.c index 1fa5e89420..d50d7f19e8 100644 --- a/crypto/init.c +++ b/crypto/init.c @@ -474,12 +474,17 @@ void OPENSSL_cleanup(void) "RAND_cleanup()\n"); #endif - CRYPTO_cleanup_all_ex_data(); - EVP_cleanup(); - CONF_modules_free(); +/* + * Note that cleanup order is important. + * For example, ENGINEs use CRYPTO_EX_DATA and therefore, must be cleaned up + * before the ex data handlers are wiped in CRYPTO_cleanup_all_ex_data(). + */ #ifndef OPENSSL_NO_ENGINE ENGINE_cleanup(); #endif + CRYPTO_cleanup_all_ex_data(); + EVP_cleanup(); + CONF_modules_free(); RAND_cleanup(); base_inited = 0; } @@ -628,5 +633,3 @@ int OPENSSL_atexit(void (*handler)(void)) return 1; } - - -- cgit v1.2.3