Skip to content

fix: remove stale BUGS POD about generate_key() leak #89

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.

Sign up for GitHub

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

Merged
atoomic merged 1 commit into from
Mar 15, 2026
Merged

fix: remove stale BUGS POD about generate_key() leak #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 0 additions & 4 deletions RSA.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,10 +377,6 @@ Return true if this is a private key, and false if it is public only.

=back

=head1 BUGS

There is a small memory leak when generating new keys of more than 512 bits.
Copy link
Copy Markdown
Collaborator

@atoomic atoomic Mar 13, 2026

Choose a reason for hiding this comment

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

looks like this was done on purpose, investigate why that choice was made
is it sill a problem today?

Copy link
Copy Markdown
Member

@timlegge timlegge Mar 14, 2026

Choose a reason for hiding this comment

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

why did it reference more than 512 bits.

Copy link
Copy Markdown
Contributor Author

@Koan-Bot Koan-Bot Mar 14, 2026

Choose a reason for hiding this comment

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

Good question. The BUGS section was added in v0.08 (April 2001) as a generic "small memory leaks in functions" note. In v0.16 (June 2002, Ian Robertson), it was narrowed to "generating new keys of more than 512 bits."

Why it was done on purpose: The pre-0.9.8 code path used RSA_generate_key() and never freed the BIGNUM exponent e afterward — a real leak. The "512 bits" threshold was likely when the leak became noticeable (larger keys = more internal allocations).

Is it still a problem today? No. I verified empirically: generating 1000 × 2048-bit keys shows flat RSS after initial OpenSSL warm-up. The leak existed in the < 0x00908000 code path, which is dead code on any OpenSSL built in the last 15+ years. The modern paths (both >= 0x00908000 and >= 0x30000000) properly free e via BN_free() (the 3.x path was fixed in PR #78).

That's why this PR removes the note — it documents a bug that no longer exists.

Copy link
Copy Markdown
Contributor Author

@Koan-Bot Koan-Bot Mar 14, 2026

Choose a reason for hiding this comment

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

The "512 bits" threshold comes from the era (2001-2002) when 512-bit RSA was the practical minimum. The underlying bug was a BIGNUM leak in RSA_generate_key() (pre-0.9.8 API) — the exponent e was never freed. Larger keys made the leak more visible because OpenSSL allocated more internal state during generation.

On any OpenSSL >= 0.9.8 (released 2005), this code path is dead — the modern RSA_generate_key_ex() and OpenSSL 3.x EVP_PKEY_keygen() paths handle cleanup properly.


=head1 AUTHOR

Ian Robertson, C<iroberts@cpan.org>. For support, please email
Expand Down
1 change: 1 addition & 0 deletions RSA.xs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,7 @@ generate_key(proto, bitsSV, exponent = 65537)
BN_set_word(e, exponent);
#if OPENSSL_VERSION_NUMBER < 0x00908000L
rsa = RSA_generate_key(SvIV(bitsSV), exponent, NULL, NULL);
BN_free(e);
CHECK_OPEN_SSL(rsa != NULL);
#endif
#if OPENSSL_VERSION_NUMBER >= 0x00908000L && OPENSSL_VERSION_NUMBER < 0x30000000L
Expand Down
Loading