From 120f9ee886ca3fbaa3d66fef37c9c2d402ff8d67 Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Tue, 31 Jan 2017 23:37:41 +0000 Subject: [PATCH] mod_ssl: work around leaks on (graceful) restart. Tested with valgrind and --with-ssl shared/static. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1781187 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 2 ++ modules/ssl/mod_ssl.c | 52 +++++++++++++++++++++++++++++++-------- modules/ssl/ssl_private.h | 3 +++ modules/ssl/ssl_util.c | 34 ++++++++++++++++--------- 4 files changed, 70 insertions(+), 21 deletions(-) diff --git a/CHANGES b/CHANGES index 6166fa19e7..0e0806af7f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,8 @@ -*- coding: utf-8 -*- Changes with Apache 2.5.0 + *) mod_ssl: work around leaks on (graceful) restart. [Yann Ylavic] + *) When using mod_status with the Event MPM, report the number of requests associated with an active connection in the "ACC" field. Previously zero was always reported with this MPM. PR60647. [Eric Covener] diff --git a/modules/ssl/mod_ssl.c b/modules/ssl/mod_ssl.c index abef4b386d..b8f561b03a 100644 --- a/modules/ssl/mod_ssl.c +++ b/modules/ssl/mod_ssl.c @@ -30,6 +30,7 @@ #include "util_md5.h" #include "util_mutex.h" #include "ap_provider.h" +#include "http_config.h" #include "mod_proxy.h" /* for proxy_hook_section_post_config() */ @@ -39,6 +40,7 @@ #include int ssl_running_on_valgrind = 0; #endif +static int modssl_running_statically = 0; APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(ssl, SSL, int, pre_handshake, (conn_rec *c,SSL *ssl,int is_proxy), @@ -308,21 +310,40 @@ static const command_rec ssl_config_cmds[] = { /* * the various processing hooks */ +static int modssl_is_prelinked(void) +{ + apr_size_t i = 0; + const module *mod; + while ((mod = ap_prelinked_modules[i++])) { + if (strcmp(mod->name, "mod_ssl.c") == 0) { + return 1; + } + } + return 0; +} + static apr_status_t ssl_cleanup_pre_config(void *data) { /* * Try to kill the internals of the SSL library. */ - /* Corresponds to OPENSSL_load_builtin_modules(): - * XXX: borrowed from apps.h, but why not CONF_modules_free() - * which also invokes CONF_modules_finish()? - */ - CONF_modules_unload(1); +#ifdef HAVE_FIPS + FIPS_mode_set(0); +#endif + /* Corresponds to OBJ_create()s */ + OBJ_cleanup(); + /* Corresponds to OPENSSL_load_builtin_modules() */ + CONF_modules_free(); /* Corresponds to SSL_library_init: */ EVP_cleanup(); #if HAVE_ENGINE_LOAD_BUILTIN_ENGINES ENGINE_cleanup(); #endif +#if OPENSSL_VERSION_NUMBER >= 0x1000200fL + SSL_COMP_free_compression_methods(); +#endif + + /* Usually needed per thread, but this parent process is single-threaded */ #if OPENSSL_VERSION_NUMBER < 0x10100000L #if OPENSSL_VERSION_NUMBER >= 0x1000000fL ERR_remove_thread_state(NULL); @@ -338,11 +359,14 @@ static apr_status_t ssl_cleanup_pre_config(void *data) ERR_free_strings(); #endif - /* Also don't call CRYPTO_cleanup_all_ex_data here; any registered - * ex_data indices may have been cached in static variables in - * OpenSSL; removing them may cause havoc. Notably, with OpenSSL + /* Also don't call CRYPTO_cleanup_all_ex_data when linked statically here; + * any registered ex_data indices may have been cached in static variables + * in OpenSSL; removing them may cause havoc. Notably, with OpenSSL * versions >= 0.9.8f, COMP_CTX cleanups would not be run, which * could result in a per-connection memory leak (!). */ + if (!modssl_running_statically) { + CRYPTO_cleanup_all_ex_data(); + } /* * TODO: determine somewhere we can safely shove out diagnostics @@ -356,10 +380,15 @@ static int ssl_hook_pre_config(apr_pool_t *pconf, apr_pool_t *plog, apr_pool_t *ptemp) { - #if HAVE_VALGRIND - ssl_running_on_valgrind = RUNNING_ON_VALGRIND; + ssl_running_on_valgrind = RUNNING_ON_VALGRIND; #endif + modssl_running_statically = modssl_is_prelinked(); + + /* Some OpenSSL internals are allocated per-thread, make sure they + * are associated to the/our same thread-id until cleaned up. + */ + ssl_util_thread_id_setup(pconf); /* We must register the library in full, to ensure our configuration * code can successfully test the SSL environment. @@ -383,6 +412,9 @@ static int ssl_hook_pre_config(apr_pool_t *pconf, "SRVName otherName form"); } + /* Start w/o errors (e.g. OBJ_txt2nid() above) */ + ERR_clear_error(); + /* * Let us cleanup the ssl library when the module is unloaded */ diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h index 459b6b34c2..26a54bb6f7 100644 --- a/modules/ssl/ssl_private.h +++ b/modules/ssl/ssl_private.h @@ -920,9 +920,12 @@ void ssl_util_ppclose(server_rec *, apr_pool_t *, apr_file_t *); char *ssl_util_readfilter(server_rec *, apr_pool_t *, const char *, const char * const *); BOOL ssl_util_path_check(ssl_pathcheck_t, const char *, apr_pool_t *); +#if APR_HAS_THREADS #if OPENSSL_VERSION_NUMBER < 0x10100000L void ssl_util_thread_setup(apr_pool_t *); #endif +void ssl_util_thread_id_setup(apr_pool_t *); +#endif int ssl_init_ssl_connection(conn_rec *c, request_rec *r); BOOL ssl_util_vhost_matches(const char *servername, server_rec *s); diff --git a/modules/ssl/ssl_util.c b/modules/ssl/ssl_util.c index 052d23e822..9e4e719f4b 100644 --- a/modules/ssl/ssl_util.c +++ b/modules/ssl/ssl_util.c @@ -383,6 +383,12 @@ static void ssl_util_thr_id(CRYPTO_THREADID *id) #endif } +static apr_status_t ssl_util_thr_id_cleanup(void *old) +{ + CRYPTO_THREADID_set_callback(old); + return APR_SUCCESS; +} + #else static unsigned long ssl_util_thr_id(void) @@ -403,16 +409,17 @@ static unsigned long ssl_util_thr_id(void) #endif } +static apr_status_t ssl_util_thr_id_cleanup(void *old) +{ + CRYPTO_set_id_callback(old); + return APR_SUCCESS; +} + #endif static apr_status_t ssl_util_thread_cleanup(void *data) { CRYPTO_set_locking_callback(NULL); -#if OPENSSL_VERSION_NUMBER >= 0x10000000L - CRYPTO_THREADID_set_callback(NULL); -#else - CRYPTO_set_id_callback(NULL); -#endif CRYPTO_set_dynlock_create_callback(NULL); CRYPTO_set_dynlock_lock_callback(NULL); @@ -436,12 +443,6 @@ void ssl_util_thread_setup(apr_pool_t *p) apr_thread_mutex_create(&(lock_cs[i]), APR_THREAD_MUTEX_DEFAULT, p); } -#if OPENSSL_VERSION_NUMBER >= 0x10000000L - CRYPTO_THREADID_set_callback(ssl_util_thr_id); -#else - CRYPTO_set_id_callback(ssl_util_thr_id); -#endif - CRYPTO_set_locking_callback(ssl_util_thr_lock); /* Set up dynamic locking scaffolding for OpenSSL to use at its @@ -455,5 +456,16 @@ void ssl_util_thread_setup(apr_pool_t *p) apr_pool_cleanup_register(p, NULL, ssl_util_thread_cleanup, apr_pool_cleanup_null); } + +void ssl_util_thread_id_setup(apr_pool_t *p) +{ +#if OPENSSL_VERSION_NUMBER >= 0x10000000L + CRYPTO_THREADID_set_callback(ssl_util_thr_id); +#else + CRYPTO_set_id_callback(ssl_util_thr_id); +#endif + apr_pool_cleanup_register(p, NULL, ssl_util_thr_id_cleanup, + apr_pool_cleanup_null); +} #endif /* #if OPENSSL_VERSION_NUMBER < 0x10100000L */ #endif /* #if APR_HAS_THREADS */ -- 2.50.1