-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
py/parse: Zero dangling parse tree pointer to fix possible leak. #14136
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.
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
py/parse: Zero dangling parse tree pointer to fix possible leak. #14136
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #14136 +/- ##
=======================================
Coverage 98.39% 98.39%
=======================================
Files 161 161
Lines 21199 21200 +1
=======================================
+ Hits 20859 20860 +1
Misses 340 340 ☔ View full report in Codecov by Sentry. |
Code size report:
|
I did a quick audit of other functions calling
I think the only way around this reliably would be a |
It's not even "at" same address I realise, it only has to overlap the dangling pointer address. |
This fixes a bug where a random Python object may become un-garbage-collectable until an enclosing Python file (compiled on device) finishes executing. Details: The mp_parse_tree_t structure is stored on the stack in top-level functions such as parse_compile_execute() in pyexec.c (and others). Although it quickly falls out of scope in these functions, it is usually still in the current stack frame when the compiled code executes. (Compiler dependent, but usually it's one stack push per function.) This means if any Python object happens to allocate at the same address as the (freed) root parse tree chunk, it's un-garbage-collectable as there's a (dangling) pointer up the stack referencing this same address. As reported by @GitHubsSilverBullet here: https://github.com/orgs/micropython/discussions/14116#discussioncomment-8837214 This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
a6973ef
to
71044a4
Compare
Thank you!
Maybe... |
That wouldn't have helped this case if it were written as: m_free_and_null(&chunk); because |
@dpgeorge good point, so yeah maybe it's OK to do it piecemeal if/when these things come up. |
This fixes a bug where a random Python object may become un-garbage-collectable until an enclosing Python file (compiled on device) finishes executing:
Details
The
mp_parse_tree_t
structure is stored on the stack in top-level functions such as parse_compile_execute() in pyexec.c (and others).Although it quickly falls out of scope in these functions, it is usually still in the current stack frame when the compiled code executes. (Compiler dependent, but usually it's one stack push per function.)
This means if any Python object happens to allocate at the same address as the (freed) root parse tree chunk, it's un-garbage-collectable as there's a (dangling) pointer up the stack referencing this same address.
As reported by @GitHubsSilverBullet here:
https://github.com/orgs/micropython/discussions/14116#discussioncomment-8837214
This work was funded through GitHub Sponsors.