Skip to content
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

GH-97002: Prevent _PyInterpreterFrames from backing more than one PyFrameObject #97996

Merged
merged 4 commits into from
Oct 6, 2022

Conversation

brandtbucher
Copy link
Member

@brandtbucher brandtbucher commented Oct 6, 2022

There is a nasty situation where a GC collection during PyFrameObject allocation could create multiple PyFrameObjects for the same _PyInterpreterFrame, which breaks our ownership rules and creates all sorts of opportunities for accessing corrupted memory.

If this happens, we just keep the PyFrameObject that has already been exposed to user code, and throw away the duplicate.

@brandtbucher brandtbucher added sprint interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump needs backport to 3.11 only security fixes labels Oct 6, 2022
@brandtbucher brandtbucher self-assigned this Oct 6, 2022
@markshannon markshannon merged commit 21a2d9f into python:main Oct 6, 2022
@miss-islington
Copy link
Contributor

Thanks @brandtbucher for the PR, and @markshannon for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Oct 6, 2022
@bedevere-bot
Copy link

GH-98002 is a backport of this pull request to the 3.11 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 6, 2022
… one `PyFrameObject` (pythonGH-97996)

(cherry picked from commit 21a2d9f)

Co-authored-by: Brandt Bucher <brandtbucher@microsoft.com>
carljm added a commit to carljm/cpython that referenced this pull request Oct 8, 2022
* main:
  pythonGH-97002: Prevent `_PyInterpreterFrame`s from backing more than one `PyFrameObject` (pythonGH-97996)
  pythongh-97973: Return all necessary information from the tokenizer (pythonGH-97984)
  fixes pythongh-96078: os.sched_yield release the GIL while calling sched_yield(2). (pythongh-97965)
  pythongh-65961: Do not rely solely on `__cached__` (pythonGH-97990)
  pythongh-97850: Remove the open issues section from the import reference (python#97935)
  Docs: pin sphinx-lint (pythonGH-97992)
  pythongh-94590: add signatures to operator itemgetter, attrgetter, methodcaller (python#94591)
  Add Pynche's move to the What's new in 3.11 (python#97974)
  pythongh-97781: Apply changes from importlib_metadata 5. (pythonGH-97785)
  pythongh-86482: Document assignment expression need for ()s (python#23291)
  pythongh-97943: PyFunction_GetAnnotations should return a borrowed reference. (python#97949)
mpage pushed a commit to mpage/cpython that referenced this pull request Oct 11, 2022
// Just pretend that we have an owned, cleared frame so frame_dealloc
// doesn't make the situation worse:
f->f_frame = (_PyInterpreterFrame *)f->_f_frame_data;
f->f_frame->owner = FRAME_CLEARED;
Copy link
Contributor

@andersk andersk Jun 26, 2023

Choose a reason for hiding this comment

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

FRAME_CLEARED is a value of enum _framestate. Shouldn’t ->owner be a value of enum _frameowner instead?

GH-106156

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) sprint type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants