You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jan Kaluža <jk...@redhat.com> on 2014/11/11 14:32:20 UTC
PR 53435, r101624, mod_ssl: error strings can't be loaded again once?
Hi,
latest comment in PR 53435 shows that memory leak in mod_ssl which
happens during graceful restarts can be caused by r101624. Since this
commit is 11 years old, I wanted to ask people here, if following is
still true with current OpenSSL:
> @@ -255,7 +255,11 @@ static apr_status_t ssl_cleanup_pre_config(void *data)
> #endif
> #endif
> ERR_remove_state(0);
> - ERR_free_strings();
> +
> + /* Don't call ERR_free_strings here; ERR_load_*_strings only
> + * actually load the error strings once per process due to static
> + * variable abuse in OpenSSL. */
> +
> /*
> * TODO: determine somewhere we can safely shove out diagnostics
> * (when enabled) at this late stage in the game:
Last comment in PR 53435 showed that leaks disappeared after reverting
this patch and it does not seem to break anything here.
Regards,
Jan Kaluza
Re: PR 53435, r101624, mod_ssl: error strings can't be loaded again
once?
Posted by Jan Kaluža <jk...@redhat.com>.
On 11/12/2014 07:16 AM, Kaspar Brand wrote:
> On 12.11.2014 03:28, Dr Stephen Henson wrote:
>> I just checked the sources and this was fixed in OpenSSL 0.9.7m just over 7
>> years ago...
>
> For 0.9.8, it was fixed with 0.9.8e:
>
> https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=900f7a87760d1053127976480efcd71371787d6e
>
> I.e., given that for 2.4.x, we still "support" 0.9.8a (and for 2.2.x,
> even 0.9.6 something), the ERR_free_strings() call should probably be
> wrapped by a suitable "#if (OPENSSL_VERSION_NUMBER >= ...".
Thanks for the info. I will do the patch and commit it to trunk and
propose for 2.4.x.
> Kaspar
>
Regards,
Jan Kaluza
Re: PR 53435, r101624, mod_ssl: error strings can't be loaded again
once?
Posted by Kaspar Brand <ht...@velox.ch>.
On 12.11.2014 03:28, Dr Stephen Henson wrote:
> I just checked the sources and this was fixed in OpenSSL 0.9.7m just over 7
> years ago...
For 0.9.8, it was fixed with 0.9.8e:
https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=900f7a87760d1053127976480efcd71371787d6e
I.e., given that for 2.4.x, we still "support" 0.9.8a (and for 2.2.x,
even 0.9.6 something), the ERR_free_strings() call should probably be
wrapped by a suitable "#if (OPENSSL_VERSION_NUMBER >= ...".
Kaspar
Re: PR 53435, r101624, mod_ssl: error strings can't be loaded again
once?
Posted by Dr Stephen Henson <sh...@opensslfoundation.com>.
On 11/11/2014 13:32, Jan Kaluža wrote:
> Hi,
>
> latest comment in PR 53435 shows that memory leak in mod_ssl which happens
> during graceful restarts can be caused by r101624. Since this commit is 11 years
> old, I wanted to ask people here, if following is still true with current OpenSSL:
>
>> @@ -255,7 +255,11 @@ static apr_status_t ssl_cleanup_pre_config(void *data)
>> #endif
>> #endif
>> ERR_remove_state(0);
>> - ERR_free_strings();
>> +
>> + /* Don't call ERR_free_strings here; ERR_load_*_strings only
>> + * actually load the error strings once per process due to static
>> + * variable abuse in OpenSSL. */
>> +
>> /*
>> * TODO: determine somewhere we can safely shove out diagnostics
>> * (when enabled) at this late stage in the game:
>
> Last comment in PR 53435 showed that leaks disappeared after reverting this
> patch and it does not seem to break anything here.
>
I just checked the sources and this was fixed in OpenSSL 0.9.7m just over 7
years ago...
Steve.
--
Dr Stephen Henson. OpenSSL Software Foundation, Inc.
1829 Mount Ephraim Road
Adamstown, MD 21710
+1 877-673-6775
shenson@opensslfoundation.com
Re: PR 53435, r101624, mod_ssl: error strings can't be loaded again
once?
Posted by Jan Kaluža <jk...@redhat.com>.
On 11/11/2014 02:32 PM, Jan Kaluža wrote:
> Hi,
>
> latest comment in PR 53435 shows that memory leak in mod_ssl which
> happens during graceful restarts can be caused by r101624. Since this
> commit is 11 years old, I wanted to ask people here, if following is
> still true with current OpenSSL:
Hi,
little follow-up here, we have memory leak on reload in
ENGINE_load_builtin_engines which is caused by
<http://svn.apache.org/r654119>. Again, this is 6 years old commit which
may or may not be true. Does anyone know if it's still valid?
> + /* 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
> + * versions >= 0.9.8f, COMP_CTX cleanups would not be run, which
> + * could result in a per-connection memory leak (!). */
> +
Regards,
Jan Kaluza
>> @@ -255,7 +255,11 @@ static apr_status_t ssl_cleanup_pre_config(void
>> *data)
>> #endif
>> #endif
>> ERR_remove_state(0);
>> - ERR_free_strings();
>> +
>> + /* Don't call ERR_free_strings here; ERR_load_*_strings only
>> + * actually load the error strings once per process due to static
>> + * variable abuse in OpenSSL. */
>> +
>> /*
>> * TODO: determine somewhere we can safely shove out diagnostics
>> * (when enabled) at this late stage in the game:
>
> Last comment in PR 53435 showed that leaks disappeared after reverting
> this patch and it does not seem to break anything here.
>
> Regards,
> Jan Kaluza