From 557b05c7a5334de5da3dc94c108c0121f10b9191 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Tue, 1 Aug 2023 02:32:18 -0700 Subject: [PATCH] gh-106092: Fix use-after-free crash in frame_dealloc (#106875) It was possible for the trashcan to delay the deallocation of a PyFrameObject until after its corresponding _PyInterpreterFrame has already been freed. So frame_dealloc needs to avoid dereferencing the f_frame pointer unless it first checks that the pointer still points to the interpreter frame within the frame object. Signed-off-by: Anders Kaseorg --- .../2023-07-18-16-13-51.gh-issue-106092.bObgRM.rst | 2 ++ Objects/frameobject.c | 13 +++++++------ 2 files changed, 9 insertions(+), 6 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-07-18-16-13-51.gh-issue-106092.bObgRM.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-07-18-16-13-51.gh-issue-106092.bObgRM.rst b/Misc/NEWS.d/next/Core and Builtins/2023-07-18-16-13-51.gh-issue-106092.bObgRM.rst new file mode 100644 index 00000000000000..7fb5b45c763e45 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-07-18-16-13-51.gh-issue-106092.bObgRM.rst @@ -0,0 +1,2 @@ +Fix a segmentation fault caused by a use-after-free bug in ``frame_dealloc`` +when the trashcan delays the deallocation of a ``PyFrameObject``. diff --git a/Objects/frameobject.c b/Objects/frameobject.c index cc9ac4b42f479a..17571535048e23 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -879,9 +879,6 @@ frame_dealloc(PyFrameObject *f) /* It is the responsibility of the owning generator/coroutine * to have cleared the generator pointer */ - assert(f->f_frame->owner != FRAME_OWNED_BY_GENERATOR || - _PyFrame_GetGenerator(f->f_frame)->gi_frame_state == FRAME_CLEARED); - if (_PyObject_GC_IS_TRACKED(f)) { _PyObject_GC_UNTRACK(f); } @@ -889,10 +886,14 @@ frame_dealloc(PyFrameObject *f) Py_TRASHCAN_BEGIN(f, frame_dealloc); PyObject *co = NULL; + /* GH-106092: If f->f_frame was on the stack and we reached the maximum + * nesting depth for deallocations, the trashcan may have delayed this + * deallocation until after f->f_frame is freed. Avoid dereferencing + * f->f_frame unless we know it still points to valid memory. */ + _PyInterpreterFrame *frame = (_PyInterpreterFrame *)f->_f_frame_data; + /* Kill all local variables including specials, if we own them */ - if (f->f_frame->owner == FRAME_OWNED_BY_FRAME_OBJECT) { - assert(f->f_frame == (_PyInterpreterFrame *)f->_f_frame_data); - _PyInterpreterFrame *frame = (_PyInterpreterFrame *)f->_f_frame_data; + if (f->f_frame == frame && frame->owner == FRAME_OWNED_BY_FRAME_OBJECT) { /* Don't clear code object until the end */ co = frame->f_executable; frame->f_executable = NULL;