Fix memory leaks in RSAKeyPair #2210
Conversation
wintersteiger
force-pushed
the
cwinter_rsa_mem_leak
branch
from
8efe112 to
03cddb6
Compare
src/tls/rsa_key_pair.h
Outdated
| EVP_PKEY_encrypt(ctx, output.data(), &olen, input, input_size)); | ||
|
|
||
| output.resize(olen); | ||
| EVP_PKEY_CTX_free(ctx); |
There was a problem hiding this comment.
Unless I'm missing something, OPENSSL_CHECK1() can throw, so we're still leaking in all error cases. Can we wrap the ctx into a unique_ptr with a custom deleter instead?
There was a problem hiding this comment.
Sure, can do. Failures of OPENSSL_CHECK1 are pretty much fatal, so I didn't spend too much effort on cleanup, but I'll have another look around.
There was a problem hiding this comment.
We have a big catch around transaction logic though, in case anything gets thrown by user code. So these will get caught there, they won’t cause an abort. I’m happy for that to go in a follow up PR, but I would prefer if it was fixed.
src/tls/rsa_key_pair.h
Outdated
| ctx, output.data(), &olen, input.data(), input.size())); | ||
|
|
||
| output.resize(olen); | ||
| EVP_PKEY_CTX_free(ctx); |
|
cwinter_rsa_mem_leak@19184 aka 20210219.8 vs main ewma over 20 builds from 18786 to 19177 |
|
There is still a leak in the ca_cert test it seems: 57: ================================================================= |
achamayou
left a comment
achamayou
left a comment
There was a problem hiding this comment.
There's still a leak in the ca_certs_test it seems.
|
Yeah, that one's giving me headaches. I fixed a number of them yesterday, but I can't reproduce the ca_certs_test failure on my machine and the stack traces are useless. I'll dig deeper shortly. |
wintersteiger
force-pushed
the
cwinter_rsa_mem_leak
branch
from
d6514c4 to
599929d
Compare
3 participants
No description provided.