Fix memory leak of ext_host_name in ssl_ext structure #49
Conversation
This file contains 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
Constant text strings actually take up SRAM space on the ESP8266 because the .RODATA segment must be copied to RAM at startup since FLASH isn't byte-accessible. Move the constant format strings in a printf completely into FLASH and add a wrapper to copy it into a local stack-allocated space when needed, freeing up about 3100 bytes of RAM for use. This doesn't make FLASH usage any higher, either, since those strings were already being stored there (but never used after the power-on startup code). Minor edits required in some of the output/debug/tracing functions, but no logic changed.
ssl_ext has a hostname which is strdup()'d (i.e. allocated) but wasn't freed in the ssl_ext_free() routine. This lead to the leaking of that strdup()'d pointer on every SSL close/reopen cycle. Patch simply checks if there's a string and frees it in the ssl_ext_free routine. Also adds a sanity check in set_ext_hostname() to not try and free() the old string if there is not one present.
|
Oops, this one accidentally included the other commit I'd sent in. I'll submit a new PR later tonight from a clean branch to avoid it pulling in bc27f4b . Looks like I'll need to destroy my existing repo and re-fork, but it shouldn't be a big deal. |
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.
@Jeroen88 found a memory leak in axtls where the new ext_host_name pointer was leaking on every SSL reconnection, eventually eating up all RAM and crashing the chip (see esp8266/Arduino#3428 ).
This patch just checks if we actually set a host name (strdup()d on the heap), and if so just frees it on the ssl_ext_free call. This should eliminate the leakage.