Finish memory leak #27
Conversation
Previously BN_MONT_CTX objects were not freed. The default RSA method uses static function rsa_ossl_finish() to free them. This fix gets the reference to the function and call it from our finish function.
|
@kazuho Sure. |
| @@ -1431,6 +1439,7 @@ int neverbleed_init(neverbleed_t *nb, char *errbuf) | |||
| #ifdef NEVERBLEED_OPAQUE_RSA_METHOD | |||
| rsa_default_method = RSA_PKCS1_OpenSSL(); | |||
| rsa_method = RSA_meth_dup(rsa_default_method); | |||
| rsa_finish = RSA_meth_get_finish(rsa_method); | |||
There was a problem hiding this comment.
I may miss some context, but I don't know why this and other changes are only for newer OpenSSL since RSA_METHOD has int (*finish) (RSA *rsa); even before OpenSSL 1.1.0. If there is no specific reasons, I would suggest putting rsa_finish = rsa_default_method->finish; in #else -- #endif below for older versions.
There was a problem hiding this comment.
Note that, while getting the finish function pointer from the default method and doing from a clone of it is the same, the former might be better for consistency if you approve my comment.
| @@ -1229,13 +1229,21 @@ __attribute__((noreturn)) static void *daemon_close_notify_thread(void *_close_n | |||
| _exit(0); | |||
| } | |||
|
|
|||
| #ifdef NEVERBLEED_OPAQUE_RSA_METHOD | |||
| static int (*rsa_finish)(RSA *rsa); | |||
There was a problem hiding this comment.
If you make the change in neverbleed_init also for !NEVERBLEED_OPAQUE_RSA_METHOD, this declaration should be moved out of #ifdef.
| static int priv_rsa_finish(RSA *rsa) | ||
| { | ||
| struct st_neverbleed_rsa_exdata_t *exdata; | ||
| struct st_neverbleed_thread_data_t *thdata; | ||
|
|
||
| get_privsep_data(rsa, &exdata, &thdata); | ||
|
|
||
| #ifdef NEVERBLEED_OPAQUE_RSA_METHOD | ||
| rsa_finish(rsa); |
As reported in #26.
Differences from tatsuhiro-t@003de52 are:
exdatahas been omitted, as that is checked withinget_privsep_datarsa_finishhas been surrounded byNEVERBLEED_OPAQUE_RSA_METHOD@omasanori Would you mind reviewing this PR?