Skip to content

Commit 1aee44b

Browse files
committed
pythongh-140815: Fix faulthandler for invalid/freed frame (python#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 a84181c)
1 parent e750796 commit 1aee44b

File tree

6 files changed

+108
-26
lines changed

6 files changed

+108
-26
lines changed

Include/internal/pycore_code.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,13 @@ extern void _PyLineTable_InitAddressRange(
308308
extern int _PyLineTable_NextAddressRange(PyCodeAddressRange *range);
309309
extern int _PyLineTable_PreviousAddressRange(PyCodeAddressRange *range);
310310

311+
// Similar to PyCode_Addr2Line(), but return -1 if the code object is invalid
312+
// and can be called without an attached tstate. Used by dump_frame() in
313+
// Python/traceback.c. The function uses heuristics to detect freed memory,
314+
// it's not 100% reliable.
315+
extern int _PyCode_SafeAddr2Line(PyCodeObject *co, int addr);
316+
317+
311318
/** API for executors */
312319
extern void _PyCode_Clear_Executors(PyCodeObject *code);
313320

Include/internal/pycore_frame.h

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ extern "C" {
1111
#include <stdbool.h>
1212
#include <stddef.h> // offsetof()
1313
#include "pycore_code.h" // STATS
14+
#include "pycore_pymem.h" // _PyMem_IsPtrFreed()
1415

1516
/* See Objects/frame_layout.md for an explanation of the frame stack
1617
* including explanation of the PyFrameObject and _PyInterpreterFrame
@@ -82,6 +83,29 @@ static inline PyCodeObject *_PyFrame_GetCode(_PyInterpreterFrame *f) {
8283
return (PyCodeObject *)f->f_executable;
8384
}
8485

86+
// Similar to _PyFrame_GetCode(), but return NULL if the frame is invalid or
87+
// freed. Used by dump_frame() in Python/traceback.c. The function uses
88+
// heuristics to detect freed memory, it's not 100% reliable.
89+
static inline PyCodeObject*
90+
_PyFrame_SafeGetCode(_PyInterpreterFrame *f)
91+
{
92+
// globals and builtins may be NULL on a legit frame, but it's unlikely.
93+
// It's more likely that it's a sign of an invalid frame.
94+
if (f->f_globals == NULL || f->f_builtins == NULL) {
95+
return NULL;
96+
}
97+
98+
PyObject *executable = f->f_executable;
99+
// Reimplement _PyObject_IsFreed() to avoid pycore_object.h dependency
100+
if (_PyMem_IsPtrFreed(executable) || _PyMem_IsPtrFreed(Py_TYPE(executable))) {
101+
return NULL;
102+
}
103+
if (!PyCode_Check(executable)) {
104+
return NULL;
105+
}
106+
return (PyCodeObject *)executable;
107+
}
108+
85109
static inline PyObject **_PyFrame_Stackbase(_PyInterpreterFrame *f) {
86110
return f->localsplus + _PyFrame_GetCode(f)->co_nlocalsplus;
87111
}
@@ -126,6 +150,22 @@ static inline void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame *
126150
dest->previous = NULL;
127151
}
128152

153+
// Similar to PyUnstable_InterpreterFrame_GetLasti(), but return NULL if the
154+
// frame is invalid or freed. Used by dump_frame() in Python/traceback.c. The
155+
// function uses heuristics to detect freed memory, it's not 100% reliable.
156+
static inline int
157+
_PyFrame_SafeGetLasti(struct _PyInterpreterFrame *f)
158+
{
159+
// Code based on _PyFrame_GetBytecode() but replace _PyFrame_GetCode()
160+
// with _PyFrame_SafeGetCode().
161+
PyCodeObject *co = _PyFrame_SafeGetCode(f);
162+
if (co == NULL) {
163+
return -1;
164+
}
165+
166+
return (int)(f->instr_ptr - _PyCode_CODE(co)) * sizeof(_Py_CODEUNIT);
167+
}
168+
129169
/* Consumes reference to func and locals.
130170
Does not initialize frame->previous, which happens
131171
when frame is linked into the frame stack.

Include/internal/pycore_pymem.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,15 +90,17 @@ static inline int _PyMem_IsPtrFreed(const void *ptr)
9090
{
9191
uintptr_t value = (uintptr_t)ptr;
9292
#if SIZEOF_VOID_P == 8
93-
return (value == 0
93+
return (value <= 0xff // NULL, 0x1, 0x2, ..., 0xff
9494
|| value == (uintptr_t)0xCDCDCDCDCDCDCDCD
9595
|| value == (uintptr_t)0xDDDDDDDDDDDDDDDD
96-
|| value == (uintptr_t)0xFDFDFDFDFDFDFDFD);
96+
|| value == (uintptr_t)0xFDFDFDFDFDFDFDFD
97+
|| value >= (uintptr_t)0xFFFFFFFFFFFFFF00); // -0xff, ..., -2, -1
9798
#elif SIZEOF_VOID_P == 4
98-
return (value == 0
99+
return (value <= 0xff
99100
|| value == (uintptr_t)0xCDCDCDCD
100101
|| value == (uintptr_t)0xDDDDDDDD
101-
|| value == (uintptr_t)0xFDFDFDFD);
102+
|| value == (uintptr_t)0xFDFDFDFD
103+
|| value >= (uintptr_t)0xFFFFFF00);
102104
#else
103105
# error "unknown pointer size"
104106
#endif
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
:mod:`faulthandler` now detects if a frame or a code object is invalid or
2+
freed. Patch by Victor Stinner.

Objects/codeobject.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -997,6 +997,23 @@ PyCode_Addr2Line(PyCodeObject *co, int addrq)
997997
return _PyCode_CheckLineNumber(addrq, &bounds);
998998
}
999999

1000+
int
1001+
_PyCode_SafeAddr2Line(PyCodeObject *co, int addrq)
1002+
{
1003+
if (addrq < 0) {
1004+
return co->co_firstlineno;
1005+
}
1006+
if (co->_co_monitoring && co->_co_monitoring->lines) {
1007+
return _Py_Instrumentation_GetLine(co, addrq/sizeof(_Py_CODEUNIT));
1008+
}
1009+
if (!(addrq >= 0 && addrq < _PyCode_NBYTES(co))) {
1010+
return -1;
1011+
}
1012+
PyCodeAddressRange bounds;
1013+
_PyCode_InitAddressRange(co, &bounds);
1014+
return _PyCode_CheckLineNumber(addrq, &bounds);
1015+
}
1016+
10001017
void
10011018
_PyLineTable_InitAddressRange(const char *linetable, Py_ssize_t length, int firstlineno, PyCodeAddressRange *range)
10021019
{

Python/traceback.c

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -894,44 +894,61 @@ _Py_DumpASCII(int fd, PyObject *text)
894894

895895
/* Write a frame into the file fd: "File "xxx", line xxx in xxx".
896896
897-
This function is signal safe. */
897+
This function is signal safe.
898898
899-
static void
899+
Return 0 on success. Return -1 if the frame is invalid. */
900+
901+
static int
900902
dump_frame(int fd, _PyInterpreterFrame *frame)
901903
{
902-
assert(frame->owner != FRAME_OWNED_BY_CSTACK);
904+
if (frame->owner == FRAME_OWNED_BY_CSTACK) {
905+
/* Ignore trampoline frame */
906+
return 0;
907+
}
903908

904-
PyCodeObject *code =_PyFrame_GetCode(frame);
909+
PyCodeObject *code = _PyFrame_SafeGetCode(frame);
910+
if (code == NULL) {
911+
return -1;
912+
}
913+
914+
int res = 0;
905915
PUTS(fd, " File ");
906916
if (code->co_filename != NULL
907917
&& PyUnicode_Check(code->co_filename))
908918
{
909919
PUTS(fd, "\"");
910920
_Py_DumpASCII(fd, code->co_filename);
911921
PUTS(fd, "\"");
912-
} else {
922+
}
923+
else {
913924
PUTS(fd, "???");
925+
res = -1;
914926
}
915927

916-
int lineno = PyUnstable_InterpreterFrame_GetLine(frame);
917928
PUTS(fd, ", line ");
929+
int lasti = _PyFrame_SafeGetLasti(frame);
930+
int lineno = -1;
931+
if (lasti >= 0) {
932+
lineno = _PyCode_SafeAddr2Line(code, lasti);
933+
}
918934
if (lineno >= 0) {
919935
_Py_DumpDecimal(fd, (size_t)lineno);
920936
}
921937
else {
922938
PUTS(fd, "???");
939+
res = -1;
923940
}
924-
PUTS(fd, " in ");
925941

926-
if (code->co_name != NULL
927-
&& PyUnicode_Check(code->co_name)) {
942+
PUTS(fd, " in ");
943+
if (code->co_name != NULL && PyUnicode_Check(code->co_name)) {
928944
_Py_DumpASCII(fd, code->co_name);
929945
}
930946
else {
931947
PUTS(fd, "???");
948+
res = -1;
932949
}
933-
934950
PUTS(fd, "\n");
951+
return res;
935952
}
936953

937954
static int
@@ -974,17 +991,6 @@ dump_traceback(int fd, PyThreadState *tstate, int write_header)
974991

975992
unsigned int depth = 0;
976993
while (1) {
977-
if (frame->owner == FRAME_OWNED_BY_CSTACK) {
978-
/* Trampoline frame */
979-
frame = frame->previous;
980-
if (frame == NULL) {
981-
break;
982-
}
983-
984-
/* Can't have more than one shim frame in a row */
985-
assert(frame->owner != FRAME_OWNED_BY_CSTACK);
986-
}
987-
988994
if (MAX_FRAME_DEPTH <= depth) {
989995
if (MAX_FRAME_DEPTH < depth) {
990996
PUTS(fd, "plus ");
@@ -994,7 +1000,15 @@ dump_traceback(int fd, PyThreadState *tstate, int write_header)
9941000
break;
9951001
}
9961002

997-
dump_frame(fd, frame);
1003+
if (_PyMem_IsPtrFreed(frame)) {
1004+
PUTS(fd, " <freed frame>\n");
1005+
break;
1006+
}
1007+
if (dump_frame(fd, frame) < 0) {
1008+
PUTS(fd, " <invalid frame>\n");
1009+
break;
1010+
}
1011+
9981012
frame = frame->previous;
9991013
if (frame == NULL) {
10001014
break;

0 commit comments

Comments
 (0)