[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: svn commit: r1198930 - in /httpd/httpd/trunk: include/mod_core.h server/core.c server/main.c

On Fri, Aug 3, 2018 at 11:46 AM, Ruediger Pluem <rpluem@xxxxxxxxxx> 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...