Fix dangling pointers from temporary c_str() and cleanup safety#31
Merged
Fix dangling pointers from temporary c_str() and cleanup safety#31
Conversation
… safety Calling .c_str() on a temporary std::string passed to a variadic function (ESP_LOG*, TEST_ASSERT_EQUAL_STRING) is undefined behavior — the temporary may be destroyed before the function reads the pointer. Store the string in a local variable first. Also: null handles after deletion in cleanup methods (_delete, _stop_and_delete) and narrow scoped_lock in i2c master_device to avoid holding the bus lock during error logging.
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
1 participant
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
.c_str()on temporarystd::stringvalues (fromto_string(),std::format(),.message()) and passing the pointer to variadic functions (ESP_LOG*,TEST_ASSERT_EQUAL_STRING) is UB — the temporary can be destroyed before the function reads the pointer. Fixed by storing the string in a local variable first._delete()/_stop_and_delete()methods in event_loop, i2c master_bus, i2c master_device, task, and timer now set_handle = nullptrafter freeing, preventing double-free on repeated calls.master_device::_try_transmit()/_try_receive()now release the bus lock before callingmap_xfer_error(), which doesESP_LOGDwith string temporaries.Across 27 files in 16 components.