GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-160663: Fix UAF in `visit_generation` by johng · Pull Request #160667 · python/cpython

Conversation

@johng

@johng johng commented Oct 25, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Callback can free an object resulting in UAF if we have relied on the object for visiting the next item in the chain.

Fix is to use finalize_garbage approach of walking the object gc generation list directly maintaing and moving it into a tempoary list before calling callback

  • Issue: UAF in GC visit_generation() #160663

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@johng
Fix UAF in visit_generation
25692b4
Callback can free an object resulting in UAF if
we have relied on the object for visiting the next
item in the chain.

Fix is to use `finalize_garbage` approach of walking
the object gc generation list directly maintaing and
moving it into a tempoary list before calling callback
@bedevere-app bedevere-app Bot added the awaiting review label Oct 25, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Oct 25, 2026
UAF in GC visit_generation() #160663
Open
@StanFromIreland
StanFromIreland requested a review from sergey-miryanov October 25, 2026 15:47
sergey-miryanov
sergey-miryanov reviewed Oct 25, 2026
View reviewed changes
Comment thread Python/gc.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Modules/_testcapi/gc.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Python/gc.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@johng
johng requested a review from sergey-miryanov October 25, 2026 18:24
@johng
Revert added test.
3350121
Failure case is crashing the whole test suite, also may not be
reliable
sergey-miryanov
sergey-miryanov reviewed Oct 25, 2026
View reviewed changes
Comment thread Python/gc.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@johng
Update comment
ffeb81f
sergey-miryanov
sergey-miryanov approved these changes Oct 25, 2026
View reviewed changes

@sergey-miryanov sergey-miryanov left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks! LGTM.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

johng reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Oct 25, 2026
@sergey-miryanov

sergey-miryanov commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

@nascheme Could you please take a look?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sergey-miryanov
Merge branch 'main' into gc-visit-objects-uaf
3d02096
@nascheme

nascheme commented Oct 30, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

IMHO, this is a bug the debug mode behavior, should likely not be using PySys_WriteStderr and PySys_FormatStderr if they can lead to GIL drop. I'd say the fix is not to change visit_generation but to use a different method to output the debug info. If the GC has to be written in a way that assumes the GIL can be dropped, it makes things a lot more complex. So, I think fixing the way we output the debug info is a better design.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@johng

johng commented Oct 30, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@nascheme just to clarify this issue is slightly different in that it's a user callback that exposed the bug rather than being specifically related to the debug_prints. Although you can use the debug_prints to expose a similar gap .PyUnstable_GC_VisitObjects accepts an arbitrary and is helping to protect against "bad" things callers can do in the callbacks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nascheme

nascheme commented Oct 30, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

@nascheme just to clarify this issue is slightly different in that it's a user callback that exposed the bug

I would say we don't support callback doing that. It is quite limited what it is safe for the callback to. E.g. you can't run user level Python code from there and the APIs you are allowed to call is very limited. Basically the callback is meant for CPython to use, not for extensions to use.

Oh, I see PyUnstable_GC_VisitObjects exists. Oh dear.

encukou reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sergey-miryanov

sergey-miryanov commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

@nascheme IIUC we can run user level Python code from debug_cycle if we override stderr.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nascheme

nascheme commented Nov 4, 2026

Copy link
Copy Markdown
Member

Ping @pablogsal for his thoughts. Personally I don't think we want the INCREF/DECREF around the visit call and should document you are not allowed to DECREF things inside that function (among the other things you are not allowed to do).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sergey-miryanov sergey-miryanov sergey-miryanov approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@johng @sergey-miryanov @nascheme
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.