Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Finish memory leak #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions neverbleed.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

@omasanori omasanori Sep 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you make the change in neverbleed_init also for !NEVERBLEED_OPAQUE_RSA_METHOD, this declaration should be moved out of #ifdef.

#endif

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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

#endif

struct expbuf_t buf = {NULL};
size_t ret;

Expand Down Expand Up @@ -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);
Copy link
Contributor

@omasanori omasanori Sep 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


RSA_meth_set1_name(rsa_method, "privsep RSA method");
RSA_meth_set_priv_enc(rsa_method, priv_enc_proxy);
Expand Down