Skip to content

[3.12] gh-122695: Fix double-free when using gc.get_referents with a freed _asyncio.FutureIter #122834

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

[3.12] gh-122695: Fix double-free when using gc.get_referents with a freed _asyncio.FutureIter #122834

wants to merge 3 commits into from

Conversation

ZeroIntensity
Copy link
Member

@ZeroIntensity ZeroIntensity commented Aug 8, 2024 •
edited
Loading

CC @vstinner, @douglas-raillard-arm

This is a (somewhat hacky) way to do it while keeping items in the freelist returned by gc.get_referents. The ideal solution would be to remove the traversal of the freelist all together, but that might be a breaking change (depending on how lisa relied on it).

@bedevere-app bedevere-app bot mentioned this pull request Aug 8, 2024
Closed
vstinner
vstinner requested changes Aug 8, 2024
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is wrong. A freelist is not storing objects but just raw memory.

module_traverse() should be modified to not visit the free list.

@bedevere-app
Copy link

bedevere-app bot commented Aug 8, 2024

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@ZeroIntensity ZeroIntensity requested a review from vstinner August 8, 2024 18:30
@ZeroIntensity
Copy link
Member Author

(Hit the wrong button when copy-pasting the <gh-issue-number> shindig, my bad!)

@colesbury
Copy link
Contributor

Do you want this in 3.13 as well?

@ZeroIntensity
Copy link
Member Author

Do you want this in 3.13 as well?

Nope, this issue only occurs on 3.12.

@colesbury
Copy link
Contributor

3.13 has the same underlying bug: module_traverse visits the free list, and the test case you added hangs (forever?) on 3.13.

@ZeroIntensity
Copy link
Member Author

It's now just a removal of the traversal, as @vstinner suggested.

@ZeroIntensity
Copy link
Member Author

3.13 has the same underlying bug: module_traverse visits the free list, and the test case you added hangs (forever?) on 3.13.

It does? I thought we tried this on 3.13 already. I guess this does go in 3.13 then.

@colesbury
Copy link
Contributor

I thought we tried this on 3.13 already

I think someone confused 3.13 with the main branch (future 3.14 release)

@ZeroIntensity
Copy link
Member Author

I thought we tried this on 3.13 already

I think someone confused 3.13 with the main branch (future 3.14 release)

Will make a PR for 3.13

ZeroIntensity added a commit to ZeroIntensity/cpython that referenced this pull request Aug 8, 2024
@ZeroIntensity
Backport python#122834 for 3.13
3c81475
@colesbury colesbury mentioned this pull request Aug 8, 2024
Merged
@vstinner
Copy link
Member

vstinner commented Aug 9, 2024

The issue should be fixed in the 3.13 branch and then backported automatically to 3.12. I close this PR in favor of #122837

@vstinner vstinner closed this Aug 9, 2024
kumaraditya303 pushed a commit that referenced this pull request Aug 9, 2024
@ZeroIntensity
[3.13] gh-122695: Fix double-free when using gc.get_referents with …
e8fb088 
…a freed `_asyncio.FutureIter` (#122837)

* Backport #122834 for 3.13
@vstinner vstinner mentioned this pull request Aug 9, 2024
Merged
vstinner pushed a commit to vstinner/cpython that referenced this pull request Aug 9, 2024
@ZeroIntensity @vstinner
[3.13] pythongh-122695: Fix double-free when using gc.get_referents…
5d77c37 
… with a freed `_asyncio.FutureIter` (python#122837)

* Backport python#122834 for 3.13

(cherry picked from commit e8fb088)
vstinner added a commit that referenced this pull request Aug 9, 2024
@vstinner @ZeroIntensity
[3.12] gh-122695: Fix double-free when using gc.get_referents with …
c57a33d 
…a freed `_asyncio.FutureIter` (#122837) (#122859)

[3.13] gh-122695: Fix double-free when using `gc.get_referents` with a freed `_asyncio.FutureIter` (#122837)

* Backport #122834 for 3.13

(cherry picked from commit e8fb088)

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
@ZeroIntensity ZeroIntensity deleted the gc-segfault-fix branch January 11, 2025 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner

@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner

@gvanrossum gvanrossum Awaiting requested review from gvanrossum gvanrossum is a code owner

@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303 kumaraditya303 is a code owner

@willingc willingc Awaiting requested review from willingc willingc is a code owner

@vstinner vstinner Awaiting requested review from vstinner

Assignees
No one assigned
Labels
awaiting changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ZeroIntensity @colesbury @vstinner