This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Fix use-after-free crash in glfw embedder #44358
Merged
Conversation
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
- Use after free due to use of c_str() from temp std::string variables.
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
cbracken
approved these changes
Aug 7, 2023
Member
cbracken
left a comment
cbracken
left a comment
There was a problem hiding this comment.
Ouch. Nicely spotted. Thanks for catching and fixing this.
lgtm.
cbracken
changed the title
cbracken
changed the title
Contributor
|
test-exempt: would need ASAN but that's not currently affordable on our CI. |
cbracken
added
the
autosubmit
Contributor
|
auto label is removed for flutter/engine/44358, due to This PR has not met approval requirements for merging. You are not a member of flutter-hackers and need 1 more review(s) in order to merge this PR.
|
auto-submit
bot
removed
the
autosubmit
cbracken
added
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Aug 8, 2023
fluttermirroringbot
pushed a commit
to flutter/flutter
that referenced
this pull request
Aug 8, 2023
…132108) flutter/engine@c271092...9c83d90 2023-08-07 [email protected] Roll Dart SDK from 0816d590a220 to f664f4b9c50d (1 revision) (flutter/engine#44462) 2023-08-07 [email protected] [Impeller] Flutter GPU: Add GpuContext. (flutter/engine#44359) 2023-08-07 [email protected] Fix use-after-free crash in glfw embedder (flutter/engine#44358) 2023-08-07 [email protected] Roll Skia from 9fbd7296de9a to d1ada6624536 (1 revision) (flutter/engine#44447) 2023-08-07 [email protected] Revert clang back to 6d667d4b261e81f325756fdfd5bb43b3b3d2451d (flutter/engine#44442) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
14 tasks
gaaclarke
pushed a commit
to gaaclarke/engine
that referenced
this pull request
Aug 30, 2023
When setting up the paths to assets and ICU data, we were previously returning the underlying char* data in a temporary string returned by `std::filesystem::path::string()`. Since the return value of `string()` is never stored, there's no guarantee that the underlying data pointer is still valid later in the function. By storing the string in a local whose lifetime exceeds that of the running engine instance, we guarantee the string isn't freed until after the Flutter app terminates. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
When setting up the paths to assets and ICU data, we were previously returning the underlying char* data in a temporary string returned by
std::filesystem::path::string(). Since the return value ofstring()is never stored, there's no guarantee that the underlying data pointer is still valid later in the function. By storing the string in a local whose lifetime exceeds that of the running engine instance, we guarantee the string isn't freed until after the Flutter app terminates.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.