Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Segfault on frame.f_back when frame is created with PyFrame_New() #99110

Open
jpe opened this issue Nov 4, 2022 · 5 comments
Open

Segfault on frame.f_back when frame is created with PyFrame_New() #99110

jpe opened this issue Nov 4, 2022 · 5 comments
Assignees
@brandtbucher
Labels
type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@jpe
Copy link

jpe commented Nov 4, 2022 •
edited by bedevere-bot

Python segfaults when frame.f_back is accessed on a frame created with PyFrame_New() c api. Calling the PyFrame_GetBack() c api also segfaults, at least in debug builds and on win32 (it depends on the contents of uninitialized memory). Tested with 3.11.0 and git 3.11 branch as of Nov 4, 2022

Cause is that the ->previous field of the _PyInterpreterFrame is never set to NULL and when PyFrame_GetBack() runs, it tries to dereference the pointer value of ->previous and segfaults. A test case using ctypes is attached.

Adding a frame->previous = NULL; line to init_frame() in frameobject.c fixes this, though I don't know if it's the best place for it.

f_back_segfault.py.txt

Linked PRs
@jpe jpe added the type-crash A hard crash of the interpreter, possibly with a core dump label Nov 4, 2022
@byllyfish byllyfish mentioned this issue Nov 11, 2022
Open
@brandtbucher
Copy link
Member

brandtbucher commented Nov 18, 2022

Thanks, I'll take a closer look at this soon.

@brandtbucher brandtbucher self-assigned this Nov 18, 2022
@byllyfish
Copy link

byllyfish commented Dec 8, 2022

I am also seeing this issue when using the grpc extension which has an asyncio task written in cython. This crash is new to 3.11.0 and 3.11.1; there was no problem with 3.10.x.

Here's a valgrind stack trace from Python 3.11.1 on Linux. The program test.py is the same as f_back_segfault.py.txt mentioned above. (There are other stack traces in the linked byllyfish/finsy issue.). The address 0xcdcdcd... indicates the code tried to dereference an uninitialized pointer.

$ valgrind python -X dev test.py 
==75321== Memcheck, a memory error detector
==75321== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==75321== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==75321== Command: python -X dev test.py
==75321== 
<frame at 0x52bfe90, file '/home/bfish/code/finsy/test.py', line 21, code greet>
==75321== Invalid read of size 1
==75321==    at 0x2859F0: _PyFrame_IsIncomplete (pycore_frame.h:147)
==75321==    by 0x2859F0: PyFrame_GetBack (frameobject.c:1326)
==75321==    by 0x2859F0: frame_getback (frameobject.c:103)
==75321==    by 0x2BBFAE: _PyObject_GenericGetAttrWithDict (object.c:1278)
==75321==    by 0x2BB46A: PyObject_GetAttr (object.c:916)
==75321==    by 0x1F9B17: _PyEval_EvalFrameDefault (ceval.c:3464)
==75321==    by 0x36987E: _PyEval_EvalFrame (pycore_ceval.h:73)
==75321==    by 0x36987E: _PyEval_Vector (ceval.c:6435)
==75321==    by 0x36987E: PyEval_EvalCode (ceval.c:1154)
==75321==    by 0x3B90AB: run_eval_code_obj (pythonrun.c:1714)
==75321==    by 0x3B90AB: run_mod (pythonrun.c:1735)
==75321==    by 0x3BA801: pyrun_file (pythonrun.c:1630)
==75321==    by 0x3BA801: _PyRun_SimpleFileObject (pythonrun.c:440)
==75321==    by 0x3BADBE: _PyRun_AnyFileObject (pythonrun.c:79)
==75321==    by 0x3DCEF7: pymain_run_file_obj (main.c:360)
==75321==    by 0x3DCEF7: pymain_run_file (main.c:379)
==75321==    by 0x3DCEF7: pymain_run_python.constprop.0 (main.c:601)
==75321==    by 0x3DD5EF: Py_RunMain (main.c:680)
==75321==    by 0x3DD5EF: pymain_main (main.c:710)
==75321==    by 0x3DD5EF: Py_BytesMain (main.c:734)
==75321==    by 0x4973D8F: (below main) (libc_start_call_main.h:58)
==75321==  Address 0xcdcdcdcdcdcdce12 is not stack'd, malloc'd or (recently) free'd
==75321== 
Fatal Python error: Segmentation fault
... (rest elided)

frame->previous is not initialized in _PyFrame_InitializeSpecials(). Setting it to NULL there fixes the issues with grpc/cython and also jpe's original complaint.

diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h
index ecc8346..6e73e1f 100644
--- a/Include/internal/pycore_frame.h
+++ b/Include/internal/pycore_frame.h
@@ -107,6 +107,7 @@ _PyFrame_InitializeSpecials(
     frame->f_locals = Py_XNewRef(locals);
     frame->stacktop = nlocalsplus;
     frame->frame_obj = NULL;
+    frame->previous = NULL;
     frame->prev_instr = _PyCode_CODE(frame->f_code) - 1;
     frame->is_entry = false;
     frame->owner = FRAME_OWNED_BY_THREAD;

If frame->previous should be initialized somewhere else, I would still put a comment in _PyFrame_InitializeSpecials to explain why it wasn't initialized there, because it looked weird to me that it was missing. Thanks.

@jpe
Copy link
Author

jpe commented Dec 8, 2022

Would it help to submit a pull request for this? I didn't submit one initially because it's only one line and there's two places it could go.

@byllyfish
Copy link

byllyfish commented Dec 10, 2022

I will submit a pull request to the main branch (3.12) tomorrow. Thanks.

This was referenced Dec 12, 2022
Open
Closed
@illia-v
Copy link
Contributor

illia-v commented Dec 22, 2022

I am not sure how much this issue is related to #100126.
It looks like a new segfault was introduced in recent b72014c, it is related to frames too.

@brandtbucher brandtbucher mentioned this issue Dec 22, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@brandtbucher brandtbucher

Labels
type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@byllyfish @jpe @illia-v @brandtbucher