Open
Conversation
ocsp_user_agent Fix three leak categories in SSLConfig: - Clear BIO_FLAGS_MEM_RDONLY before BIO_free so internal BUF_MEM structures are properly released - Delete existing session_cache/origin_sess_cache before allocating new ones in initialize() - Free ssl_ocsp_user_agent before overwriting with new value
The destructor was empty, leaking all SSLOriginSession objects still held in the CountQueue and map when the cache was destroyed. Pop and delete every queued node before the map is implicitly destroyed.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets LSAN/ASAN-reported memory leaks in the TLS/SSL subsystem by ensuring cached session objects, config-owned strings, and OpenSSL BIO internals are properly released during teardown and reload.
Changes:
- Drain and delete queued origin TLS session cache nodes during
SSLOriginSessionCachedestruction. - Free/recreate
origin_sess_cacheon SSL config reload, and freessl_ocsp_user_agentbefore overwriting it. - Adjust BIO cleanup to clear
BIO_FLAGS_MEM_RDONLYbefore freeing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/iocore/net/SSLSessionCache.cc |
Implements destructor cleanup for origin session cache nodes/map. |
src/iocore/net/SSLConfig.cc |
Frees/recreates origin session cache on reload, frees OCSP UA string before overwrite, updates BIO cleanup. |
…e cache self-sizes on insert via the static origin_session_cache_size, so recreating it is unnecessary and unsafe (concurrent TLS handshakes hold bare pointers to the global). Allocate once; let entries age out naturally when the configured size shrinks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SSLOriginSessionCachedestructor — drain queued session nodes and clear the mapSSLConfigParams::initialize— delete oldorigin_sess_cachebefore re-allocating, free oldssl_ocsp_user_agentbefore overwriteBIO_FLAGS_MEM_RDONLYbeforeBIO_freeso internalBUF_MEMstructures are properly releasedFound via ASAN-enabled autest runs on Fedora 43.
Test plan
ENABLE_ASAN=ONand ran full autest suite — no new LSAN reports for these symbols