fix: free OpenSSL 3.x resources in _new_key_from_parameters()#90
Conversation
| if (iqmp) BN_clear_free(iqmp); | ||
| if (ctx) BN_CTX_free(ctx); | ||
| #if OPENSSL_VERSION_NUMBER >= 0x30000000L | ||
| EVP_PKEY_CTX_free(pctx); |
There was a problem hiding this comment.
should we check whether pctx, params_build and params != NULL
There was a problem hiding this comment.
No, explicit NULL checks before freeing are not needed here. All three OpenSSL free functions handle NULL gracefully (they're no-ops on NULL input):
EVP_PKEY_CTX_free(NULL)— safeOSSL_PARAM_BLD_free(NULL)— safeOSSL_PARAM_free(NULL)— safe
The allocation points are already guarded by CHECK_OPEN_SSL() which croaks immediately on NULL, so we can't reach the usage code with a NULL pointer. And OSSL_PARAM_BLD_to_param() failure is caught by THROW(params != NULL), which jumps to the err: label where the PR adds cleanup.
The NULL-after-free assignments (pctx = NULL; etc.) are there for defensive double-free protection — if both the inline success-path cleanup and the err: label fire, they won't double-free.
|
@Koan-Bot rebase |
Rebase: fix: free OpenSSL 3.x resources in _new_key_from_parameters()Branch Diff: 1 file changed, 10 insertions(+) Review feedback was analyzed and applied. Actions
Automated by Kōan |
Koan-Bot
force-pushed
the
koan.atoomic/fix-new-key-params-error-leaks
branch
from
9f64a40 to
6ad6229
Compare
|
Something is wrong with that rebase. WTH so many commits were added !! |
1 similar comment
|
Something is wrong with that rebase. WTH so many commits were added !! |
|
@Koan-Bot rebase |
On OpenSSL 3.x, _new_key_from_parameters() allocates three resources that were never fully cleaned up: - pctx (EVP_PKEY_CTX): leaked on every call (both happy and error paths) - params_build (OSSL_PARAM_BLD): leaked in the else branch (public-key only path) and on any THROW error path - params (OSSL_PARAM): same as params_build Fix: free all three immediately after EVP_PKEY_fromdata() succeeds in both the p/q branch and the else branch, NULL the pointers, and add fallback cleanup in the err: block for THROW error paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rebase: fix: free OpenSSL 3.x resources in _new_key_from_parameters()Branch Diff: 1 file changed, 10 insertions(+) Review feedback was analyzed and applied. Actions
Automated by Kōan |
Koan-Bot
force-pushed
the
koan.atoomic/fix-new-key-params-error-leaks
branch
from
6ad6229 to
b788b3f
Compare
3 participants
What
Free
pctx,params_build, andparamsthat were leaked in_new_key_from_parameters()on OpenSSL 3.x.Why
Three OpenSSL 3.x resources allocated during key construction were never fully cleaned up:
pctx(EVP_PKEY_CTX): leaked on every call — neither happy path nor error path freed itparams_build(OSSL_PARAM_BLD): freed in the p/q branch but leaked in the else (public-key only) branch and on allTHROWerror pathsparams(OSSL_PARAM): same asparams_buildHow
EVP_PKEY_fromdata()succeeds, in both the p/q and else brancheserr:block (inside#if OPENSSL_VERSION_NUMBER >= 0x30000000L) forTHROWerror paths that jump before inline cleanupAll OpenSSL
*_free()functions accept NULL, so the err: block is safe even when inline cleanup already ran.Testing
make testpasses (281 tests) on Perl 5.42.0 / OpenSSL 3.6.1 (macOS)🤖 Generated with Claude Code
Quality Report
Changes: 1 file changed, 15 insertions(+)
Code scan: clean
Tests: passed (OK)
Branch hygiene: clean
Generated by Kōan post-mission quality pipeline