python / cpython

The Python programming language
https://www.python.org
Other
62.39k stars 29.96k forks source link

Segfault when trying to use PyRun_SimpleString() with some imports #124160

Open luk1337 opened 2 days ago

luk1337 commented 2 days ago

Crash report

What happened?

I hit the segfault when doing the following thing:

$ docker run -ti fedora:41 bash
# dnf -y install gcc python-devel
# echo '#include <Python.h>

int main() {
    Py_Initialize();
    PyThreadState_Swap(Py_NewInterpreter());
    PyRun_SimpleString("import readline");
}' > test.c
# gcc test.c -I/usr/include/python3.13 -lpython3.13
# ./a.out 
Segmentation fault (core dumped)
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7a8c3cb in reload_singlephase_extension (tstate=tstate@entry=0x7ffff7e5a850, cached=cached@entry=0x0, 
    info=info@entry=0x7fffffff8c90) at /usr/src/debug/python3.13-3.13.0~rc2-1.fc41.x86_64/Python/import.c:1763
1763        PyModuleDef *def = cached->def;

The same code doesn't crash on 3.12.

CPython versions tested on:

3.13

Operating systems tested on:

Linux

Output from running 'python -VV' on the command line:

Python 3.13.0rc2 (main, Sep 7 2024, 00:00:00) [GCC 14.2.1 20240801 (Red Hat 14.2.1-1)]

Linked PRs

ZeroIntensity commented 1 day ago

I wasn't able to get an embedded interpreter working with requests, so I couldn't confirm the patch. (Embedding a locally compiled interpreter is still pretty painful.)

Though, I did try this directly on CPython (via _testcapi), and I wasn't able to reproduce it. It's odd that this only occurs when embedding.

(@encukou, for now, this needs topic-C-API and 3.13. This might be subinterpreter-related, but I don't know -- I'll look futher into it tomorrow.)

luk1337 commented 1 day ago

I wasn't able to get an embedded interpreter working with requests, so I couldn't confirm the patch. (Embedding a locally compiled interpreter is still pretty painful.)

Though, I did try this directly on CPython (via _testcapi), and I wasn't able to reproduce it. It's odd that this only occurs when embedding.

(@encukou, for now, this needs topic-C-API and 3.13. This might be subinterpreter-related, but I don't know -- I'll look futher into it tomorrow.)

Just fyi, you need both brotli and requests, it won't die w/o both of them installed. Unless importing brotl alone can trigger this crash too, but I haven't tried that myself.

luk1337 commented 1 day ago

I wasn't able to get an embedded interpreter working with requests, so I couldn't confirm the patch. (Embedding a locally compiled interpreter is still pretty painful.) Though, I did try this directly on CPython (via _testcapi), and I wasn't able to reproduce it. It's odd that this only occurs when embedding. (@encukou, for now, this needs topic-C-API and 3.13. This might be subinterpreter-related, but I don't know -- I'll look futher into it tomorrow.)

Just fyi, you need both brotli and requests, it won't die w/o both of them installed. Unless importing brotl alone can trigger this crash too, but I haven't tried that myself.

It does, I updated the op.

ZeroIntensity commented 1 day ago

Oh, I only tried this with requests, not brotli. Has this been reported to them yet? It could be something specific to their initialization function, rather than CPython.

With that being said, it doesn't look like brotli is doing anything complex -- it's just a simple single-phase init, as far as I can tell. But if this applied to all single-phase init extensions under subinterpreters, I would hope that it would have been reported much earlier in 3.13's development.

I wonder if something else is going wrong here, because you'll lose some errors by omitting NULL checks. Does PyThreadState_Swap return anything iffy? What about Py_NewInterpreter?

luk1337 commented 1 day ago

Oh, I only tried this with requests, not brotli. Has this been reported to them yet? It could be something specific to their initialization function, rather than CPython.

With that being said, it doesn't look like brotli is doing anything complex -- it's just a simple single-phase init, as far as I can tell. But if this applied to all single-phase init extensions under subinterpreters, I would hope that it would have been reported much earlier in 3.13's development.

This happens with some other modules too, e.g.

(gdb) bt
#0  0x00007f14bca8c3cb in reload_singlephase_extension (tstate=tstate@entry=0x7f14bc28d850, 
    cached=cached@entry=0x0, info=info@entry=0x7ffdaa112e00)
    at /usr/src/debug/python3.13-3.13.0~rc2-1.fc41.x86_64/Python/import.c:1763
#1  0x00007f14bc82d42e in import_run_extension (tstate=tstate@entry=0x7f14bc28d850, 
    p0=p0@entry=0x7f14ba909330 <PyInit__rust>, info=info@entry=0x7ffdaa112e00, 
    spec=spec@entry=<ModuleSpec(name='cryptography.hazmat.bindings._rust', loader=<ExtensionFileLoader(name='cryptography.hazmat.bindings._rust', path='/usr/lib64/python3.13/site-packages/cryptography/hazmat/bindings/_rust.abi3.so') at remote 0x7f14bac5c910>, origin='/usr/lib64/python3.13/site-packages/cryptography/hazmat/bindings/_rust.abi3.so', loader_state=None, submodule_search_locations=None, _uninitialized_submodules=[], _set_fileattr=True, _cached=None) at remote 0x7f14bac547d0>, modules=<optimized out>)
    at /usr/src/debug/python3.13-3.13.0~rc2-1.fc41.x86_64/Python/import.c:2099

so I assumed this is a regression in CPython and that module maintainers shouldn't have to apply workarounds to fix 3.13 support.

I wonder if something else is going wrong here, because you'll lose some errors by omitting NULL checks. Does PyThreadState_Swap return anything iffy? What about Py_NewInterpreter?

[root@377a2e8f31b5 /]# cat test.c 
#include <Python.h>

int main() {
    Py_Initialize();
    PyThreadState *a = Py_NewInterpreter();
    printf("a=%p\n", a);
    PyThreadState *b = PyThreadState_Swap(a);
    printf("b=%p\n", b);
    PyRun_SimpleFile(fopen("test.py", "r"), "test.py");
}
[root@377a2e8f31b5 /]# ./a.out 
a=0x7ffb3c7ab850
b=0x7ffb3c7ab850
Segmentation fault (core dumped)
ZeroIntensity commented 1 day ago

This happens with some other modules too, e.g.

Well that's not good :(

Are there any modules in the stdlib that this occurs with? (That makes it much easier to track down, so you don't have to deal with virtual environments when testing.)

Also, I'm assuming that the PyThreadState_Swap call is necessary for the reproducer. That runs the imports in a subinterpreter, rather than the main interpreter -- not all third party modules support that. Does this occur without that? (If not, that gives some good insight into what's going on.)

vstinner commented 1 day ago

cc @ericsnowcurrently

luk1337 commented 1 day ago

This happens with some other modules too, e.g.

Well that's not good :(

Are there any modules in the stdlib that this occurs with? (That makes it much easier to track down, so you don't have to deal with virtual environments when testing.)

Also, I'm assuming that the PyThreadState_Swap call is necessary for the reproducer. That runs the imports in a subinterpreter, rather than the main interpreter -- not all third party modules support that. Does this occur without that? (If not, that gives some good insight into what's going on.)

No idea about stdlib also it doesn't happen w/o PyThreadState_Swap(). BTW I uploaded a simple PR that fixes it, if you didn't notice it yet ^^

ZeroIntensity commented 1 day ago

I saw, I just don't know whether it's the correct fix because I can't reproduce it yet.

No idea about stdlib also it doesn't happen w/o PyThreadState_Swap()

Good to know, that means this is a subinterpreter problem. As far as I can see, brotli indicates that it supports subinterpreters via a module size of 0, but they are using global variables, so that might be incorrect on their end.

luca020400 commented 1 day ago

Wrote a quick script to traverse the whole support std modules and there's a few hits that make it crash

import sys
import importlib

for mod in sys.stdlib_module_names:
    try:
        print(mod)
        importlib.import_module(mod)
    except:
        pass

so far readline, rlcompleter, doctest, _tracemalloc make it crash on my machine like brotli.

Since the list isn't sorted you may be able to hit a few more entries I missed :)


Update to handle exceptions since some modules are listed but not loadable (like turtle)

ZeroIntensity commented 1 day ago

Thanks! I don't have a chance to try and reproduce this at the moment, but I think the problem is with using a module state with single-phase init under a subinterpreter. The fix by @luk1337 makes sense to me.

luk1337 commented 1 day ago

Wrote a quick script to traverse the whole support std modules and there's a few hits that make it crash

import sys
import importlib

for mod in sys.stdlib_module_names:
    try:
        print(mod)
        importlib.import_module(mod)
    except:
        pass

so far readline, rlcompleter, doctest, _tracemalloc make it crash on my machine like brotli.

Since the list isn't sorted you may be able to hit a few more entries I missed :)

Update to handle exceptions since some modules are listed but not loadable (like turtle)

thx, I updated OP with readline instead of brotli

ericsnowcurrently commented 1 day ago

FWIW, the fix in gh-124164 is correct. It should have been using main_tstate.

ZeroIntensity commented 22 hours ago

Alas, a reproducer that does not require C code:

import _interpreters

interp = _interpreters.create()
_interpreters.run_string(interp, "import readline")