Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit cea3697

Browse filesBrowse files
brandtbuchermpage
authored andcommitted
pythonGH-97002: Prevent _PyInterpreterFrames from backing more than one PyFrameObject (pythonGH-97996)
1 parent 078e3ed commit cea3697
Copy full SHA for cea3697

File tree

3 files changed

+91
-6
lines changed
Filter options

3 files changed

+91
-6
lines changed

‎Lib/test/test_frame.py

Copy file name to clipboardExpand all lines: Lib/test/test_frame.py
+65Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import gc
12
import re
23
import sys
34
import textwrap
@@ -261,5 +262,69 @@ def gen():
261262
""")
262263
assert_python_ok("-c", code)
263264

265+
@support.cpython_only
266+
def test_sneaky_frame_object(self):
267+
268+
def trace(frame, event, arg):
269+
"""
270+
Don't actually do anything, just force a frame object to be created.
271+
"""
272+
273+
def callback(phase, info):
274+
"""
275+
Yo dawg, I heard you like frames, so I'm allocating a frame while
276+
you're allocating a frame, so you can have a frame while you have a
277+
frame!
278+
"""
279+
nonlocal sneaky_frame_object
280+
sneaky_frame_object = sys._getframe().f_back
281+
# We're done here:
282+
gc.callbacks.remove(callback)
283+
284+
def f():
285+
while True:
286+
yield
287+
288+
old_threshold = gc.get_threshold()
289+
old_callbacks = gc.callbacks[:]
290+
old_enabled = gc.isenabled()
291+
old_trace = sys.gettrace()
292+
try:
293+
# Stop the GC for a second while we set things up:
294+
gc.disable()
295+
# Create a paused generator:
296+
g = f()
297+
next(g)
298+
# Move all objects to the oldest generation, and tell the GC to run
299+
# on the *very next* allocation:
300+
gc.collect()
301+
gc.set_threshold(1, 0, 0)
302+
# Okay, so here's the nightmare scenario:
303+
# - We're tracing the resumption of a generator, which creates a new
304+
# frame object.
305+
# - The allocation of this frame object triggers a collection
306+
# *before* the frame object is actually created.
307+
# - During the collection, we request the exact same frame object.
308+
# This test does it with a GC callback, but in real code it would
309+
# likely be a trace function, weakref callback, or finalizer.
310+
# - The collection finishes, and the original frame object is
311+
# created. We now have two frame objects fighting over ownership
312+
# of the same interpreter frame!
313+
sys.settrace(trace)
314+
gc.callbacks.append(callback)
315+
sneaky_frame_object = None
316+
gc.enable()
317+
next(g)
318+
# g.gi_frame should be the the frame object from the callback (the
319+
# one that was *requested* second, but *created* first):
320+
self.assertIs(g.gi_frame, sneaky_frame_object)
321+
finally:
322+
gc.set_threshold(*old_threshold)
323+
gc.callbacks[:] = old_callbacks
324+
sys.settrace(old_trace)
325+
if old_enabled:
326+
gc.enable()
327+
328+
264329
if __name__ == "__main__":
265330
unittest.main()
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix an issue where several frame objects could be backed by the same
2+
interpreter frame, possibly leading to corrupted memory and hard crashes of
3+
the interpreter.

‎Python/frame.c

Copy file name to clipboardExpand all lines: Python/frame.c
+23-6Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,31 @@ _PyFrame_MakeAndSetFrameObject(_PyInterpreterFrame *frame)
3737
Py_XDECREF(error_type);
3838
Py_XDECREF(error_value);
3939
Py_XDECREF(error_traceback);
40+
return NULL;
4041
}
41-
else {
42-
assert(frame->owner != FRAME_OWNED_BY_FRAME_OBJECT);
43-
assert(frame->owner != FRAME_CLEARED);
44-
f->f_frame = frame;
45-
frame->frame_obj = f;
46-
PyErr_Restore(error_type, error_value, error_traceback);
42+
PyErr_Restore(error_type, error_value, error_traceback);
43+
if (frame->frame_obj) {
44+
// GH-97002: How did we get into this horrible situation? Most likely,
45+
// allocating f triggered a GC collection, which ran some code that
46+
// *also* created the same frame... while we were in the middle of
47+
// creating it! See test_sneaky_frame_object in test_frame.py for a
48+
// concrete example.
49+
//
50+
// Regardless, just throw f away and use that frame instead, since it's
51+
// already been exposed to user code. It's actually a bit tricky to do
52+
// this, since we aren't backed by a real _PyInterpreterFrame anymore.
53+
// Just pretend that we have an owned, cleared frame so frame_dealloc
54+
// doesn't make the situation worse:
55+
f->f_frame = (_PyInterpreterFrame *)f->_f_frame_data;
56+
f->f_frame->owner = FRAME_CLEARED;
57+
f->f_frame->frame_obj = f;
58+
Py_DECREF(f);
59+
return frame->frame_obj;
4760
}
61+
assert(frame->owner != FRAME_OWNED_BY_FRAME_OBJECT);
62+
assert(frame->owner != FRAME_CLEARED);
63+
f->f_frame = frame;
64+
frame->frame_obj = f;
4865
return f;
4966
}
5067

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.