From 43882c7c4e1ec9ccd4ce32eb6a4c5a3f47c270b7 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 5 Nov 2025 18:39:28 +0100 Subject: [PATCH] [3.13] gh-140815: Fix faulthandler for invalid/freed frame (#140921) (#140985) gh-140815: Fix faulthandler for invalid/freed frame (#140921) faulthandler now detects if a frame or a code object is invalid or freed. Add helper functions: * _PyCode_SafeAddr2Line() * _PyFrame_SafeGetCode() * _PyFrame_SafeGetLasti() _PyMem_IsPtrFreed() now detects pointers in [-0xff, 0xff] range as freed. (cherry picked from commit a84181c31bfc45a1d6bcb1296bd298ad612c54d0) --- Include/internal/pycore_code.h | 7 +++ Include/internal/pycore_frame.h | 40 +++++++++++++ Include/internal/pycore_pymem.h | 10 ++-- ...-11-02-19-23-32.gh-issue-140815.McEG-T.rst | 2 + Objects/codeobject.c | 17 ++++++ Python/traceback.c | 58 ++++++++++++------- 6 files changed, 108 insertions(+), 26 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2025-11-02-19-23-32.gh-issue-140815.McEG-T.rst diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h index 1fb8cc473c6e..efb97dd871fb 100644 --- a/Include/internal/pycore_code.h +++ b/Include/internal/pycore_code.h @@ -308,6 +308,13 @@ extern void _PyLineTable_InitAddressRange( extern int _PyLineTable_NextAddressRange(PyCodeAddressRange *range); extern int _PyLineTable_PreviousAddressRange(PyCodeAddressRange *range); +// Similar to PyCode_Addr2Line(), but return -1 if the code object is invalid +// and can be called without an attached tstate. Used by dump_frame() in +// Python/traceback.c. The function uses heuristics to detect freed memory, +// it's not 100% reliable. +extern int _PyCode_SafeAddr2Line(PyCodeObject *co, int addr); + + /** API for executors */ extern void _PyCode_Clear_Executors(PyCodeObject *code); diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index af181e3760d2..a6bd971ea0c5 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -11,6 +11,7 @@ extern "C" { #include #include // offsetof() #include "pycore_code.h" // STATS +#include "pycore_pymem.h" // _PyMem_IsPtrFreed() /* See Objects/frame_layout.md for an explanation of the frame stack * including explanation of the PyFrameObject and _PyInterpreterFrame @@ -82,6 +83,29 @@ static inline PyCodeObject *_PyFrame_GetCode(_PyInterpreterFrame *f) { return (PyCodeObject *)f->f_executable; } +// Similar to _PyFrame_GetCode(), but return NULL if the frame is invalid or +// freed. Used by dump_frame() in Python/traceback.c. The function uses +// heuristics to detect freed memory, it's not 100% reliable. +static inline PyCodeObject* +_PyFrame_SafeGetCode(_PyInterpreterFrame *f) +{ + // globals and builtins may be NULL on a legit frame, but it's unlikely. + // It's more likely that it's a sign of an invalid frame. + if (f->f_globals == NULL || f->f_builtins == NULL) { + return NULL; + } + + PyObject *executable = f->f_executable; + // Reimplement _PyObject_IsFreed() to avoid pycore_object.h dependency + if (_PyMem_IsPtrFreed(executable) || _PyMem_IsPtrFreed(Py_TYPE(executable))) { + return NULL; + } + if (!PyCode_Check(executable)) { + return NULL; + } + return (PyCodeObject *)executable; +} + static inline PyObject **_PyFrame_Stackbase(_PyInterpreterFrame *f) { return f->localsplus + _PyFrame_GetCode(f)->co_nlocalsplus; } @@ -126,6 +150,22 @@ static inline void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame * dest->previous = NULL; } +// Similar to PyUnstable_InterpreterFrame_GetLasti(), but return NULL if the +// frame is invalid or freed. Used by dump_frame() in Python/traceback.c. The +// function uses heuristics to detect freed memory, it's not 100% reliable. +static inline int +_PyFrame_SafeGetLasti(struct _PyInterpreterFrame *f) +{ + // Code based on _PyFrame_GetBytecode() but replace _PyFrame_GetCode() + // with _PyFrame_SafeGetCode(). + PyCodeObject *co = _PyFrame_SafeGetCode(f); + if (co == NULL) { + return -1; + } + + return (int)(f->instr_ptr - _PyCode_CODE(co)) * sizeof(_Py_CODEUNIT); +} + /* Consumes reference to func and locals. Does not initialize frame->previous, which happens when frame is linked into the frame stack. diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h index e9593dbff1cc..79c43a0768b7 100644 --- a/Include/internal/pycore_pymem.h +++ b/Include/internal/pycore_pymem.h @@ -90,15 +90,17 @@ static inline int _PyMem_IsPtrFreed(const void *ptr) { uintptr_t value = (uintptr_t)ptr; #if SIZEOF_VOID_P == 8 - return (value == 0 + return (value <= 0xff // NULL, 0x1, 0x2, ..., 0xff || value == (uintptr_t)0xCDCDCDCDCDCDCDCD || value == (uintptr_t)0xDDDDDDDDDDDDDDDD - || value == (uintptr_t)0xFDFDFDFDFDFDFDFD); + || value == (uintptr_t)0xFDFDFDFDFDFDFDFD + || value >= (uintptr_t)0xFFFFFFFFFFFFFF00); // -0xff, ..., -2, -1 #elif SIZEOF_VOID_P == 4 - return (value == 0 + return (value <= 0xff || value == (uintptr_t)0xCDCDCDCD || value == (uintptr_t)0xDDDDDDDD - || value == (uintptr_t)0xFDFDFDFD); + || value == (uintptr_t)0xFDFDFDFD + || value >= (uintptr_t)0xFFFFFF00); #else # error "unknown pointer size" #endif diff --git a/Misc/NEWS.d/next/Library/2025-11-02-19-23-32.gh-issue-140815.McEG-T.rst b/Misc/NEWS.d/next/Library/2025-11-02-19-23-32.gh-issue-140815.McEG-T.rst new file mode 100644 index 000000000000..18c4d3836efe --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-11-02-19-23-32.gh-issue-140815.McEG-T.rst @@ -0,0 +1,2 @@ +:mod:`faulthandler` now detects if a frame or a code object is invalid or +freed. Patch by Victor Stinner. diff --git a/Objects/codeobject.c b/Objects/codeobject.c index bdf6ee95a8b3..a361cb4957c0 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -997,6 +997,23 @@ PyCode_Addr2Line(PyCodeObject *co, int addrq) return _PyCode_CheckLineNumber(addrq, &bounds); } +int +_PyCode_SafeAddr2Line(PyCodeObject *co, int addrq) +{ + if (addrq < 0) { + return co->co_firstlineno; + } + if (co->_co_monitoring && co->_co_monitoring->lines) { + return _Py_Instrumentation_GetLine(co, addrq/sizeof(_Py_CODEUNIT)); + } + if (!(addrq >= 0 && addrq < _PyCode_NBYTES(co))) { + return -1; + } + PyCodeAddressRange bounds; + _PyCode_InitAddressRange(co, &bounds); + return _PyCode_CheckLineNumber(addrq, &bounds); +} + void _PyLineTable_InitAddressRange(const char *linetable, Py_ssize_t length, int firstlineno, PyCodeAddressRange *range) { diff --git a/Python/traceback.c b/Python/traceback.c index f8195c9ce9c0..7e7739682794 100644 --- a/Python/traceback.c +++ b/Python/traceback.c @@ -894,14 +894,24 @@ done: /* Write a frame into the file fd: "File "xxx", line xxx in xxx". - This function is signal safe. */ + This function is signal safe. -static void + Return 0 on success. Return -1 if the frame is invalid. */ + +static int dump_frame(int fd, _PyInterpreterFrame *frame) { - assert(frame->owner != FRAME_OWNED_BY_CSTACK); + if (frame->owner == FRAME_OWNED_BY_CSTACK) { + /* Ignore trampoline frame */ + return 0; + } - PyCodeObject *code =_PyFrame_GetCode(frame); + PyCodeObject *code = _PyFrame_SafeGetCode(frame); + if (code == NULL) { + return -1; + } + + int res = 0; PUTS(fd, " File "); if (code->co_filename != NULL && PyUnicode_Check(code->co_filename)) @@ -909,29 +919,36 @@ dump_frame(int fd, _PyInterpreterFrame *frame) PUTS(fd, "\""); _Py_DumpASCII(fd, code->co_filename); PUTS(fd, "\""); - } else { + } + else { PUTS(fd, "???"); + res = -1; } - int lineno = PyUnstable_InterpreterFrame_GetLine(frame); PUTS(fd, ", line "); + int lasti = _PyFrame_SafeGetLasti(frame); + int lineno = -1; + if (lasti >= 0) { + lineno = _PyCode_SafeAddr2Line(code, lasti); + } if (lineno >= 0) { _Py_DumpDecimal(fd, (size_t)lineno); } else { PUTS(fd, "???"); + res = -1; } - PUTS(fd, " in "); - if (code->co_name != NULL - && PyUnicode_Check(code->co_name)) { + PUTS(fd, " in "); + if (code->co_name != NULL && PyUnicode_Check(code->co_name)) { _Py_DumpASCII(fd, code->co_name); } else { PUTS(fd, "???"); + res = -1; } - PUTS(fd, "\n"); + return res; } static int @@ -974,17 +991,6 @@ dump_traceback(int fd, PyThreadState *tstate, int write_header) unsigned int depth = 0; while (1) { - if (frame->owner == FRAME_OWNED_BY_CSTACK) { - /* Trampoline frame */ - frame = frame->previous; - if (frame == NULL) { - break; - } - - /* Can't have more than one shim frame in a row */ - assert(frame->owner != FRAME_OWNED_BY_CSTACK); - } - if (MAX_FRAME_DEPTH <= depth) { if (MAX_FRAME_DEPTH < depth) { PUTS(fd, "plus "); @@ -994,7 +1000,15 @@ dump_traceback(int fd, PyThreadState *tstate, int write_header) break; } - dump_frame(fd, frame); + if (_PyMem_IsPtrFreed(frame)) { + PUTS(fd, " \n"); + break; + } + if (dump_frame(fd, frame) < 0) { + PUTS(fd, " \n"); + break; + } + frame = frame->previous; if (frame == NULL) { break; -- 2.47.3