On Fri, Aug 3, 2018 at 11:46 AM, Ruediger Pluem <[email protected]> wrote:
>> + ap_init_rng(ap_pglobal);
>
> With APR trunk used this now causes httpd to SEGFAULT in EVP_cleanup
> when it stops in case mod_ssl is loaded. This is because mod_ssl
> stored data in Openssl data structures that points to it (likely
> static data in mod_ssl), but it gets unloaded due to the pconf pool
> cleanup before the crypto_lib_cleanup runs EVP_cleanup as it is a
> cleanup on the parent pool ap_pglobal.
Ouch, ISTM that mod_ssl should cleanup what it owns after itself.
Any idea which static data (or code/callbacks) in mod_ssl are still pointed to?
> One approach that made this go away was the following patch to APR:
>
> Index: srclib/apr/crypto/apr_crypto.c
> ===================================================================
> --- srclib/apr/crypto/apr_crypto.c (revision 1837332)
> +++ srclib/apr/crypto/apr_crypto.c (working copy)
> @@ -534,8 +534,7 @@
> lib->pool = pool;
> apr_hash_set(active_libs, lib->name, APR_HASH_KEY_STRING, lib);
> if (apr_pool_parent_get(pool)) {
> - apr_pool_cleanup_register(pool, lib, crypto_lib_cleanup,
> - apr_pool_cleanup_null);
> + apr_pool_pre_cleanup_register(pool, lib, crypto_lib_cleanup);
> }
> }
> else {
>
> So using a pre_cleanup instead of a cleanup. But I guess this would only fix
> this specific use case. I guess we have
> a larger problem lurking around that a shared object can put some data in
> Openssl that points to it (e.g. static data in
> the shared object) and that it is gone by the time the pool cleanup runs. In
> this case we are lucky that a child pool
> causes the shared object to be unloaded and hence the pre_cleanup works here.
> But IMHO we don't need to have this
> connection always.
It looks fragile yes :/
I'd prefer each module to cleanup after itself in openssl, if possible...
Regards,
Yann.
