[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)

diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h
index 1fb8cc473c6e98b942d1a083f9fd96cb40f98004..efb97dd871fb4820a99e76e963f5d1966f86d323 100644 (file)
--- 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 af181e3760d231c626df1ec36b966c276483dc7d..a6bd971ea0c550fdac3b7ea8b16a5e9419c37c15 100644 (file)
--- a/Include/internal/pycore_frame.h
+++ b/Include/internal/pycore_frame.h
@@ -11,6 +11,7 @@ extern "C" {
 #include <stdbool.h>
 #include <stddef.h>               // 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 e9593dbff1cc60aeb2ac5ee59a04f41b8a7fee5c..79c43a0768b7051923a5278d32f01a51eb27af4c 100644 (file)
--- 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 (file)
index 0000000..18c4d38
--- /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 bdf6ee95a8b3a317d185e7efaf068ad055b4d3d8..a361cb4957c0bdd795e57eb48dc05fa8f1bf5e64 100644 (file)
--- 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 f8195c9ce9c090d3efcdb793f0485e01073e6a24..7e7739682794ac4581e74a87dbcf06a5158bc40b 100644 (file)
--- 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, "  <freed frame>\n");
+            break;
+        }
+        if (dump_frame(fd, frame) < 0) {
+            PUTS(fd, "  <invalid frame>\n");
+            break;
+        }
+
         frame = frame->previous;
         if (frame == NULL) {
             break;