Skip to content

Commit a84181c

Browse files
authored
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.
1 parent 08115d2 commit a84181c

File tree

6 files changed

+127
-32
lines changed

6 files changed

+127
-32
lines changed

Include/internal/pycore_code.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,13 @@ extern void _PyLineTable_InitAddressRange(
274274
/** API for traversing the line number table. */
275275
PyAPI_FUNC(int) _PyLineTable_NextAddressRange(PyCodeAddressRange *range);
276276
extern int _PyLineTable_PreviousAddressRange(PyCodeAddressRange *range);
277-
// This is used in dump_frame() in traceback.c without an attached tstate.
278-
extern int _PyCode_Addr2LineNoTstate(PyCodeObject *co, int addr);
277+
278+
// Similar to PyCode_Addr2Line(), but return -1 if the code object is invalid
279+
// and can be called without an attached tstate. Used by dump_frame() in
280+
// Python/traceback.c. The function uses heuristics to detect freed memory,
281+
// it's not 100% reliable.
282+
extern int _PyCode_SafeAddr2Line(PyCodeObject *co, int addr);
283+
279284

280285
/** API for executors */
281286
extern void _PyCode_Clear_Executors(PyCodeObject *code);

Include/internal/pycore_interpframe.h

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,36 @@ static inline PyCodeObject *_PyFrame_GetCode(_PyInterpreterFrame *f) {
2424
return (PyCodeObject *)executable;
2525
}
2626

27+
// Similar to _PyFrame_GetCode(), but return NULL if the frame is invalid or
28+
// freed. Used by dump_frame() in Python/traceback.c. The function uses
29+
// heuristics to detect freed memory, it's not 100% reliable.
30+
static inline PyCodeObject*
31+
_PyFrame_SafeGetCode(_PyInterpreterFrame *f)
32+
{
33+
// globals and builtins may be NULL on a legit frame, but it's unlikely.
34+
// It's more likely that it's a sign of an invalid frame.
35+
if (f->f_globals == NULL || f->f_builtins == NULL) {
36+
return NULL;
37+
}
38+
39+
if (PyStackRef_IsNull(f->f_executable)) {
40+
return NULL;
41+
}
42+
void *ptr;
43+
memcpy(&ptr, &f->f_executable, sizeof(f->f_executable));
44+
if (_PyMem_IsPtrFreed(ptr)) {
45+
return NULL;
46+
}
47+
PyObject *executable = PyStackRef_AsPyObjectBorrow(f->f_executable);
48+
if (_PyObject_IsFreed(executable)) {
49+
return NULL;
50+
}
51+
if (!PyCode_Check(executable)) {
52+
return NULL;
53+
}
54+
return (PyCodeObject *)executable;
55+
}
56+
2757
static inline _Py_CODEUNIT *
2858
_PyFrame_GetBytecode(_PyInterpreterFrame *f)
2959
{
@@ -37,6 +67,31 @@ _PyFrame_GetBytecode(_PyInterpreterFrame *f)
3767
#endif
3868
}
3969

70+
// Similar to PyUnstable_InterpreterFrame_GetLasti(), but return NULL if the
71+
// frame is invalid or freed. Used by dump_frame() in Python/traceback.c. The
72+
// function uses heuristics to detect freed memory, it's not 100% reliable.
73+
static inline int
74+
_PyFrame_SafeGetLasti(struct _PyInterpreterFrame *f)
75+
{
76+
// Code based on _PyFrame_GetBytecode() but replace _PyFrame_GetCode()
77+
// with _PyFrame_SafeGetCode().
78+
PyCodeObject *co = _PyFrame_SafeGetCode(f);
79+
if (co == NULL) {
80+
return -1;
81+
}
82+
83+
_Py_CODEUNIT *bytecode;
84+
#ifdef Py_GIL_DISABLED
85+
_PyCodeArray *tlbc = _PyCode_GetTLBCArray(co);
86+
assert(f->tlbc_index >= 0 && f->tlbc_index < tlbc->size);
87+
bytecode = (_Py_CODEUNIT *)tlbc->entries[f->tlbc_index];
88+
#else
89+
bytecode = _PyCode_CODE(co);
90+
#endif
91+
92+
return (int)(f->instr_ptr - bytecode) * sizeof(_Py_CODEUNIT);
93+
}
94+
4095
static inline PyFunctionObject *_PyFrame_GetFunction(_PyInterpreterFrame *f) {
4196
PyObject *func = PyStackRef_AsPyObjectBorrow(f->f_funcobj);
4297
assert(PyFunction_Check(func));

Include/internal/pycore_pymem.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,17 @@ static inline int _PyMem_IsPtrFreed(const void *ptr)
5454
{
5555
uintptr_t value = (uintptr_t)ptr;
5656
#if SIZEOF_VOID_P == 8
57-
return (value == 0
57+
return (value <= 0xff // NULL, 0x1, 0x2, ..., 0xff
5858
|| value == (uintptr_t)0xCDCDCDCDCDCDCDCD
5959
|| value == (uintptr_t)0xDDDDDDDDDDDDDDDD
60-
|| value == (uintptr_t)0xFDFDFDFDFDFDFDFD);
60+
|| value == (uintptr_t)0xFDFDFDFDFDFDFDFD
61+
|| value >= (uintptr_t)0xFFFFFFFFFFFFFF00); // -0xff, ..., -2, -1
6162
#elif SIZEOF_VOID_P == 4
62-
return (value == 0
63+
return (value <= 0xff
6364
|| value == (uintptr_t)0xCDCDCDCD
6465
|| value == (uintptr_t)0xDDDDDDDD
65-
|| value == (uintptr_t)0xFDFDFDFD);
66+
|| value == (uintptr_t)0xFDFDFDFD
67+
|| value >= (uintptr_t)0xFFFFFF00);
6668
#else
6769
# error "unknown pointer size"
6870
#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: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,8 +1005,8 @@ PyCode_NewEmpty(const char *filename, const char *funcname, int firstlineno)
10051005
* source location tracking (co_lines/co_positions)
10061006
******************/
10071007

1008-
int
1009-
_PyCode_Addr2LineNoTstate(PyCodeObject *co, int addrq)
1008+
static int
1009+
_PyCode_Addr2Line(PyCodeObject *co, int addrq)
10101010
{
10111011
if (addrq < 0) {
10121012
return co->co_firstlineno;
@@ -1020,12 +1020,29 @@ _PyCode_Addr2LineNoTstate(PyCodeObject *co, int addrq)
10201020
return _PyCode_CheckLineNumber(addrq, &bounds);
10211021
}
10221022

1023+
int
1024+
_PyCode_SafeAddr2Line(PyCodeObject *co, int addrq)
1025+
{
1026+
if (addrq < 0) {
1027+
return co->co_firstlineno;
1028+
}
1029+
if (co->_co_monitoring && co->_co_monitoring->lines) {
1030+
return _Py_Instrumentation_GetLine(co, addrq/sizeof(_Py_CODEUNIT));
1031+
}
1032+
if (!(addrq >= 0 && addrq < _PyCode_NBYTES(co))) {
1033+
return -1;
1034+
}
1035+
PyCodeAddressRange bounds;
1036+
_PyCode_InitAddressRange(co, &bounds);
1037+
return _PyCode_CheckLineNumber(addrq, &bounds);
1038+
}
1039+
10231040
int
10241041
PyCode_Addr2Line(PyCodeObject *co, int addrq)
10251042
{
10261043
int lineno;
10271044
Py_BEGIN_CRITICAL_SECTION(co);
1028-
lineno = _PyCode_Addr2LineNoTstate(co, addrq);
1045+
lineno = _PyCode_Addr2Line(co, addrq);
10291046
Py_END_CRITICAL_SECTION();
10301047
return lineno;
10311048
}

Python/traceback.c

Lines changed: 37 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,44 +1028,61 @@ _Py_DumpWideString(int fd, wchar_t *str)
10281028

10291029
/* Write a frame into the file fd: "File "xxx", line xxx in xxx".
10301030
1031-
This function is signal safe. */
1031+
This function is signal safe.
10321032
1033-
static void
1033+
Return 0 on success. Return -1 if the frame is invalid. */
1034+
1035+
static int
10341036
dump_frame(int fd, _PyInterpreterFrame *frame)
10351037
{
1036-
assert(frame->owner < FRAME_OWNED_BY_INTERPRETER);
1038+
if (frame->owner == FRAME_OWNED_BY_INTERPRETER) {
1039+
/* Ignore trampoline frame */
1040+
return 0;
1041+
}
10371042

1038-
PyCodeObject *code =_PyFrame_GetCode(frame);
1043+
PyCodeObject *code = _PyFrame_SafeGetCode(frame);
1044+
if (code == NULL) {
1045+
return -1;
1046+
}
1047+
1048+
int res = 0;
10391049
PUTS(fd, " File ");
10401050
if (code->co_filename != NULL
10411051
&& PyUnicode_Check(code->co_filename))
10421052
{
10431053
PUTS(fd, "\"");
10441054
_Py_DumpASCII(fd, code->co_filename);
10451055
PUTS(fd, "\"");
1046-
} else {
1056+
}
1057+
else {
10471058
PUTS(fd, "???");
1059+
res = -1;
10481060
}
1049-
int lasti = PyUnstable_InterpreterFrame_GetLasti(frame);
1050-
int lineno = _PyCode_Addr2LineNoTstate(code, lasti);
1061+
10511062
PUTS(fd, ", line ");
1063+
int lasti = _PyFrame_SafeGetLasti(frame);
1064+
int lineno = -1;
1065+
if (lasti >= 0) {
1066+
lineno = _PyCode_SafeAddr2Line(code, lasti);
1067+
}
10521068
if (lineno >= 0) {
10531069
_Py_DumpDecimal(fd, (size_t)lineno);
10541070
}
10551071
else {
10561072
PUTS(fd, "???");
1073+
res = -1;
10571074
}
1058-
PUTS(fd, " in ");
10591075

1060-
if (code->co_name != NULL
1061-
&& PyUnicode_Check(code->co_name)) {
1076+
PUTS(fd, " in ");
1077+
if (code->co_name != NULL && PyUnicode_Check(code->co_name)) {
10621078
_Py_DumpASCII(fd, code->co_name);
10631079
}
10641080
else {
10651081
PUTS(fd, "???");
1082+
res = -1;
10661083
}
1067-
10681084
PUTS(fd, "\n");
1085+
return res;
10691086
}
10701087

10711088
static int
@@ -1108,17 +1125,6 @@ dump_traceback(int fd, PyThreadState *tstate, int write_header)
11081125

11091126
unsigned int depth = 0;
11101127
while (1) {
1111-
if (frame->owner == FRAME_OWNED_BY_INTERPRETER) {
1112-
/* Trampoline frame */
1113-
frame = frame->previous;
1114-
if (frame == NULL) {
1115-
break;
1116-
}
1117-
1118-
/* Can't have more than one shim frame in a row */
1119-
assert(frame->owner != FRAME_OWNED_BY_INTERPRETER);
1120-
}
1121-
11221128
if (MAX_FRAME_DEPTH <= depth) {
11231129
if (MAX_FRAME_DEPTH < depth) {
11241130
PUTS(fd, "plus ");
@@ -1128,7 +1134,15 @@ dump_traceback(int fd, PyThreadState *tstate, int write_header)
11281134
break;
11291135
}
11301136

1131-
dump_frame(fd, frame);
1137+
if (_PyMem_IsPtrFreed(frame)) {
1138+
PUTS(fd, " <freed frame>\n");
1139+
break;
1140+
}
1141+
if (dump_frame(fd, frame) < 0) {
1142+
PUTS(fd, " <invalid frame>\n");
1143+
break;
1144+
}
1145+
11321146
frame = frame->previous;
11331147
if (frame == NULL) {
11341148
break;

0 commit comments

Comments
 (0)