From f0eb1698291c00c60f76f8207b9fa9c079e75c12 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Wed, 14 Jan 2015 17:49:32 -0800 Subject: [PATCH] Free all error queues on shutdown. As feared, 2bca0988 did cause some leak checkers to get upset about the state_hash pointer getting cleared. This change makes err_shutdown free all the error queues to try and avoid this. Hopefully this doesn't upset TSAN in turn. BUG=448296 Change-Id: I827da63c793dcabc73168ece052cdcd3d3cc64e3 Reviewed-on: https://boringssl-review.googlesource.com/2890 Reviewed-by: David Benjamin Reviewed-by: Adam Langley --- crypto/err/err.c | 23 ++++++++++++++--------- crypto/err/err_impl.c | 3 ++- include/openssl/err.h | 2 +- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/crypto/err/err.c b/crypto/err/err.c index 8b8094f5..892b2ac8 100644 --- a/crypto/err/err.c +++ b/crypto/err/err.c @@ -296,10 +296,21 @@ void ERR_clear_error(void) { state->top = state->bottom = 0; } +static void err_state_free(ERR_STATE *state) { + unsigned i; + + for (i = 0; i < ERR_NUM_ERRORS; i++) { + err_clear(&state->errors[i]); + } + if (state->to_free) { + OPENSSL_free(state->to_free); + } + OPENSSL_free(state); +} + void ERR_remove_thread_state(const CRYPTO_THREADID *tid) { CRYPTO_THREADID current; ERR_STATE *state; - unsigned i; if (tid == NULL) { CRYPTO_THREADID_current(¤t); @@ -312,13 +323,7 @@ void ERR_remove_thread_state(const CRYPTO_THREADID *tid) { return; } - for (i = 0; i < ERR_NUM_ERRORS; i++) { - err_clear(&state->errors[i]); - } - if (state->to_free) { - OPENSSL_free(state->to_free); - } - OPENSSL_free(state); + err_state_free(state); } int ERR_get_next_error_library(void) { @@ -800,7 +805,7 @@ void ERR_load_crypto_strings(void) { err_load_strings(); } void ERR_free_strings(void) { err_fns_check(); - ERRFN(shutdown)(); + ERRFN(shutdown)(err_state_free); } void ERR_load_BIO_strings(void) {} diff --git a/crypto/err/err_impl.c b/crypto/err/err_impl.c index e448d65b..8cfcb46d 100644 --- a/crypto/err/err_impl.c +++ b/crypto/err/err_impl.c @@ -287,13 +287,14 @@ static ERR_STATE *err_release_state(const CRYPTO_THREADID *tid) { return state; } -static void err_shutdown(void) { +static void err_shutdown(void (*err_state_free_cb)(ERR_STATE*)) { CRYPTO_w_lock(CRYPTO_LOCK_ERR); if (error_hash) { lh_ERR_STRING_DATA_free(error_hash); error_hash = NULL; } if (state_hash) { + lh_ERR_STATE_doall(state_hash, err_state_free_cb); lh_ERR_STATE_free(state_hash); state_hash = NULL; } diff --git a/include/openssl/err.h b/include/openssl/err.h index 18a43ce7..c749659c 100644 --- a/include/openssl/err.h +++ b/include/openssl/err.h @@ -488,7 +488,7 @@ OPENSSL_EXPORT void ERR_load_strings(const ERR_STRING_DATA *str); /* ERR_FNS_st is a structure of function pointers that contains the actual * implementation of the error queue handling functions. */ struct ERR_FNS_st { - void (*shutdown)(void); + void (*shutdown)(void (*err_state_free_cb)(ERR_STATE*)); ERR_STRING_DATA *(*get_item)(uint32_t packed_error); ERR_STRING_DATA *(*set_item)(const ERR_STRING_DATA *); ERR_STRING_DATA *(*del_item)(uint32_t packed_error);