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

SystemError from calling __iter__ on a released memoryview #126341

Closed
devdanzin opened this issue Nov 3, 2024 · 15 comments
Closed

SystemError from calling __iter__ on a released memoryview #126341

devdanzin opened this issue Nov 3, 2024 · 15 comments
Assignees
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes easy interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@devdanzin
Copy link
Contributor

devdanzin commented Nov 3, 2024

Crash report

What happened?

The code below causes a SystemError on all tested Python versions in Windows and Linux. In a no-gil debug build of main, besides the expected abort, sometimes a segfault occurs.

from threading import Thread

av = memoryview(b"something")
av.release()

alive = [
    Thread(target=av.release, args=('/bin/sh')),
    Thread(target=av.__iter__, args=())
]

for obj in alive:
    try:
        print('START', obj)
        obj.start()
    except Exception:
        pass

Common abort:

Fatal Python error: _Py_CheckFunctionResult: a function returned a result with an exception set
Python runtime state: initialized
ValueError: operation forbidden on released memoryview object

The above exception was the direct cause of the following exception:

SystemError: <method-wrapper '__iter__' of memoryview object at 0x200006a6050> returned a result with an exception set
Aborted

Rare segfault:

Exception in thread Thread-1 (release):
Fatal Python error: _Py_CheckFunctionResult: a function returned a result with an exception set
Python runtime state: initialized
ValueError: operation forbidden on released memoryview object

The above exception was the direct cause of the following exception:

SystemError: <method-wrapper '__iter__' of memoryview object at 0x200006a6050> returned a result with an exception set

Current thread 0x00007f750b5d0640 (most recent call first):
  File "/home/danzin/projects/mycpython/Lib/threading.py", line 992 in run
  File "/home/danzin/projects/mycpython/Lib/threading.py", line 1041 in _bootstrap_inner
  File "/home/danzin/projects/mycpython/Lib/threading.py", line 1012 in _bootstrap

Thread 0x00007f750bdd5640 (most recent call first):
  File "<frozen importlib._bootstrap_external>", line 142 in _path_split
Python/traceback.c:979:18: runtime error: member access within misaligned address 0x000000000001 for type 'struct _PyInterpreterFrame', which requires 8 byte alignment
0x000000000001: note: pointer points here
<memory cannot be printed>
Segmentation fault

The backtrace for the segfault is:

0x00005555564d6e71 in _PyFrame_GetCode (f=0x7ffff6bbcca8)
    at ./Include/internal/pycore_frame.h:83
#1  PyUnstable_InterpreterFrame_GetLine (
    frame=frame@entry=0x7ffff6bbcca8) at Python/frame.c:146
#2  0x00005555565ffb1e in dump_frame (fd=fd@entry=2,
    frame=frame@entry=0x7ffff6bbcca8) at Python/traceback.c:905
#3  0x00005555565fffcc in dump_traceback (fd=fd@entry=2,
    tstate=tstate@entry=0x555557130d20,
    write_header=write_header@entry=0)
    at Python/traceback.c:974
#4  0x00005555566001e0 in _Py_DumpTracebackThreads (
    fd=fd@entry=2, interp=<optimized out>,
    interp@entry=0x555557052280 <_PyRuntime+129152>,
    current_tstate=current_tstate@entry=0x5555571351d0)
    at Python/traceback.c:1090
#5  0x00005555565888f1 in _Py_FatalError_DumpTracebacks (
    fd=fd@entry=2,
    interp=interp@entry=0x555557052280 <_PyRuntime+129152>,
    tstate=tstate@entry=0x5555571351d0)
    at Python/pylifecycle.c:2937
#6  0x000055555659b12b in fatal_error (fd=2,
    header=header@entry=1,
    prefix=prefix@entry=0x555556811220 <__func__.23> "_Py_CheckFunctionResult",
    msg=msg@entry=0x555556810c80 "a function returned a result with an exception set", status=status@entry=-1)
    at Python/pylifecycle.c:3245
#7  0x000055555659b2ca in _Py_FatalErrorFunc (
    func=func@entry=0x555556811220 <__func__.23> "_Py_CheckFunctionResult",
    msg=msg@entry=0x555556810c80 "a function returned a result with an exception set") at Python/pylifecycle.c:3286
#8  0x0000555555ee822c in _Py_CheckFunctionResult (
    tstate=tstate@entry=0x5555571351d0,result=result@entry=0x20004040130, where=where@entry=0x0)
    at Objects/call.c:65

Found using fusil by @vstinner.

CPython versions tested on:

3.11, 3.12, 3.13, CPython main branch

Operating systems tested on:

Linux, Windows

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

No response

Linked PRs

@devdanzin devdanzin added the type-crash A hard crash of the interpreter, possibly with a core dump label Nov 3, 2024
@ZeroIntensity ZeroIntensity added 3.13 bugs and security fixes topic-free-threading 3.14 new features, bugs and security fixes labels Nov 3, 2024
@ZeroIntensity
Copy link
Member

IIUC, it's undecided right now about what to do with method thread safety--most methods on objects aren't thread-safe, so you should be using a lock.

@devdanzin devdanzin changed the title SystemError from accessing a memoryview from two threads SystemError from calling __iter__ on a released memoryview Nov 3, 2024
@devdanzin
Copy link
Contributor Author

It seems to be independent of threads, my bad for not reducing further. The code below reliably causes the SystemError for me, but I couldn't get a segfault with it so far.

av = memoryview(b"something")
av.release()
av.__iter__()

@ZeroIntensity
Copy link
Member

I'm assuming the segfault is a result of missing thread safety on memoryview objects, but the SystemError is definitely a bug. Thank you for the report!

@ZeroIntensity
Copy link
Member

@picnixz Considering your recent ventures with memoryview objects, would you like to take this one? If not, I can submit a PR.

@sobolevn
Copy link
Member

sobolevn commented Nov 3, 2024

I would expect to see something like:

    CHECK_RELEASED(obj);

as the fix.

@picnixz
Copy link
Contributor

picnixz commented Nov 3, 2024

@picnixz Considering your recent ventures with memoryview objects, would you like to take this one? If not, I can submit a PR.

Please do so! I'm working on HACL* HMAC now and won't be available for the next few days. (I'll also fix a simple curses issue in a few hours (DONE)).

@picnixz picnixz added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Nov 3, 2024
@ZeroIntensity
Copy link
Member

Yeah, @sobolevn is right. I'll let someone newer have a chance to contribute since it's a super easy fix :)

index d4672e8198..25634f997a 100644
--- a/Objects/memoryobject.c
+++ b/Objects/memoryobject.c
@@ -3356,6 +3356,7 @@ memory_iter(PyObject *seq)
         PyErr_BadInternalCall();
         return NULL;
     }
+    CHECK_RELEASED(seq);
     PyMemoryViewObject *obj = (PyMemoryViewObject *)seq;
     int ndims = obj->view.ndim;
     if (ndims == 0) {

@devdanzin
Copy link
Contributor Author

Is the segfault something to worry about in other SystemError situations, even if it requires threading shenanigans? Should there also be a protection for that?

@ZeroIntensity
Copy link
Member

Let's deal with the segfault separately. I'm assuming that it's a data race, so we just need to add @critical_section to AC.

@ritvikpasham
Copy link
Contributor

Can I be assigned to this issue? I'm an undergraduate student looking to contribute to my first open source project.

@ZeroIntensity
Copy link
Member

@ritvikpasham Go for it :)

@ZeroIntensity
Copy link
Member

@ritvikpasham Are you still working on this?

@picnixz picnixz added the 3.12 bugs and security fixes label Nov 9, 2024
@ritvikpasham
Copy link
Contributor

Yeah I am!

@vstinner
Copy link
Member

@ritvikpasham:

Yeah I am!

Do you need help to write the fix? The fix was provided as a patch in #126341 (comment).

@ritvikpasham
Copy link
Contributor

Sorry for the delay. I'll finish it in the next hour.

sobolevn added a commit that referenced this issue Nov 13, 2024
…26759)

Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Peter Bierma <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: sobolevn <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Nov 13, 2024
…w` (pythonGH-126759)

(cherry picked from commit a12690e)

Co-authored-by: Ritvik Pasham <[email protected]>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Peter Bierma <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: sobolevn <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Nov 13, 2024
…w` (pythonGH-126759)

(cherry picked from commit a12690e)

Co-authored-by: Ritvik Pasham <[email protected]>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Peter Bierma <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: sobolevn <[email protected]>
Eclips4 pushed a commit that referenced this issue Nov 13, 2024
…ew` (GH-126759) (#126779)

gh-126341: add release check to `__iter__` method of `memoryview` (GH-126759)
(cherry picked from commit a12690e)

Co-authored-by: Ritvik Pasham <[email protected]>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Peter Bierma <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: sobolevn <[email protected]>
Eclips4 pushed a commit that referenced this issue Nov 13, 2024
…ew` (GH-126759) (#126778)

gh-126341: add release check to `__iter__` method of `memoryview` (GH-126759)
(cherry picked from commit a12690e)

Co-authored-by: Ritvik Pasham <[email protected]>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Peter Bierma <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: sobolevn <[email protected]>
@Eclips4 Eclips4 closed this as completed Nov 13, 2024
picnixz pushed a commit to picnixz/cpython that referenced this issue Dec 8, 2024
…w` (python#126759)

Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Peter Bierma <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: sobolevn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes easy interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Development

No branches or pull requests

7 participants