Skip to content

Possible NULL-pointer dereference in faulthandler_user #96652

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

Closed
ovchinnikov-v-a opened this issue Sep 7, 2022 · 6 comments
Closed

Possible NULL-pointer dereference in faulthandler_user #96652

ovchinnikov-v-a opened this issue Sep 7, 2022 · 6 comments
Assignees
Labels
3.12 only security fixes type-bug An unexpected behavior, bug, or error type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@ovchinnikov-v-a
Copy link

ovchinnikov-v-a commented Sep 7, 2022

Crash report

Segmentation fault occurs whenever test_socketserver suite takes more than 60 seconds on a system with HAVE_SIGACTION undefined.

To reproduce the issue:

  1. Replace #define HAVE_SIGACTION 1 with #undef HAVE_SIGACTION in pyconfig.h.
  2. Add an artificial 1 second delay on every iteration of the for loop in test_tcpserver_bind_leak (Lib/test/test_socketserver.py):
    def test_tcpserver_bind_leak(self):
        ...
        import time
        for i in range(1024):
            time.sleep(1)
            with self.assertRaises(OverflowError):
                ....
  1. Rebuild CPython with make.
  2. Run test_socketserver test suite with make test TESTOPTS="-j1 -v test_socketserver".

Once 60-seconds alarm set in SocketServerTest::setUp goes off, SIGALRM handler (i.e. faulthandler_user) gets called, prints Python's traceback (via faulthandler_dump_traceback) and crashes while calling user->previous that is NULL.

Error messages

test_tcpserver_bind_leak (test.test_socketserver.SocketServerTest.test_tcpserver_bind_leak) ... Current thread 0x00007fa77c504740 (most recent call first):
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/test_socketserver.py", line 294 in test_tcpserver_bind_leak
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/case.py", line 579 in _callTestMethod
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/case.py", line 623 in run
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/case.py", line 678 in __call__
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/suite.py", line 122 in run
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/suite.py", line 84 in __call__
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/suite.py", line 122 in run
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/suite.py", line 84 in __call__
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/suite.py", line 122 in run
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/suite.py", line 84 in __call__
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/runner.py", line 208 in run
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/support/__init__.py", line 1100 in _run_suite
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/support/__init__.py", line 1226 in run_unittest
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/runtest.py", line 281 in _test_module
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/runtest.py", line 317 in _runtest_inner2
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/runtest.py", line 360 in _runtest_inner
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/runtest.py", line 235 in _runtest
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/runtest.py", line 265 in runtest
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/runtest_mp.py", line 95 in run_tests_worker
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/main.py", line 722 in _main
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/main.py", line 701 in main
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/main.py", line 763 in main
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/regrtest.py", line 43 in _main
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/regrtest.py", line 47 in <module>
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/runpy.py", line 88 in _run_code
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/runpy.py", line 198 in _run_module_as_main
Fatal Python error: Segmentation fault

Current thread 0x00007f80ee8d0740 (most recent call first):
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/test_socketserver.py", line 294 in test_tcpserver_bind_leak
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/case.py", line 579 in _callTestMethod
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/case.py", line 623 in run
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/case.py", line 678 in __call__
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/suite.py", line 122 in run
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/suite.py", line 84 in __call__
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/suite.py", line 122 in run
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/suite.py", line 84 in __call__
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/suite.py", line 122 in run
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/suite.py", line 84 in __call__
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/unittest/runner.py", line 208 in run
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/support/__init__.py", line 1100 in _run_suite
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/support/__init__.py", line 1226 in run_unittest
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/runtest.py", line 281 in _test_module
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/runtest.py", line 317 in _runtest_inner2
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/runtest.py", line 360 in _runtest_inner
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/runtest.py", line 235 in _runtest
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/runtest.py", line 265 in runtest
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/main.py", line 347 in rerun_failed_tests
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/main.py", line 746 in _main
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/main.py", line 701 in main
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/libregrtest/main.py", line 763 in main
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/test/__main__.py", line 2 in <module>
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/runpy.py", line 88 in _run_code
  File "/home/vitalii/Projects/CPython/ovchinnikov-v-a-cpython/Lib/runpy.py", line 198 in _run_module_as_main

Extension modules: _testcapi (total: 1)
make: *** [Makefile:1828: test] Segmentation fault (core dumped)

Your environment

CPython built from sources: main dde15f5879 [origin/main] gh-94808: Improve coverage of _PyBytes_FormatEx (GH-95895)
Operating system and architecture: Linux 5.15.0-46-generic #49-Ubuntu SMP x86_64 x86_64 x86_64 GNU/Linux

@ovchinnikov-v-a ovchinnikov-v-a added the type-crash A hard crash of the interpreter, possibly with a core dump label Sep 7, 2022
@kumaraditya303 kumaraditya303 added type-bug An unexpected behavior, bug, or error 3.12 only security fixes labels Sep 7, 2022
ovchinnikov-v-a pushed a commit to ovchinnikov-v-a/cpython that referenced this issue Sep 7, 2022
For systems without `sigaction` (i.e. HAVE_SIGACTION is undefined)
`faulthandler_user` has to check previous signal handler to be
defined (non-NULL) before calling it. Otherwise the code is
subject to segmentation faults.
@vstinner
Copy link
Member

vstinner commented Sep 7, 2022

The bug can be reproduced simply with:

import faulthandler
import signal
import time

faulthandler.register(signal.SIGALRM, chain=True)
signal.alarm(1)
time.sleep(2)
print("exit")

The question is more what is your platform. It doesn't support sigaction()?

@vstinner
Copy link
Member

vstinner commented Sep 7, 2022

PR #96666 should fix the issue.

@ovchinnikov-v-a
Copy link
Author

The question is more what is your platform. It doesn't support sigaction()?

It's our customer's OS we're porting CPython to.
In theory it should support sigaction but for the time being there are some issues with SA_* flags preventing us from building CPython with HAVE_SIGACTION defined.

ovchinnikov-v-a pushed a commit to ovchinnikov-v-a/cpython that referenced this issue Sep 8, 2022
@vstinner
Copy link
Member

vstinner commented Sep 8, 2022

It's our customer's OS we're porting CPython to.

Can you share the OS name? Or is it private? If yes, please mention the OS version.

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 8, 2022
…ythonGH-96666)

Fix the faulthandler implementation of faulthandler.register(signal,
chain=True) if the sigaction() function is not available: don't call
the previous signal handler if it's NULL.
(cherry picked from commit c580a81)

Co-authored-by: Victor Stinner <vstinner@python.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 8, 2022
…ythonGH-96666)

Fix the faulthandler implementation of faulthandler.register(signal,
chain=True) if the sigaction() function is not available: don't call
the previous signal handler if it's NULL.
(cherry picked from commit c580a81)

Co-authored-by: Victor Stinner <vstinner@python.org>
vstinner added a commit that referenced this issue Sep 8, 2022
Fix the faulthandler implementation of faulthandler.register(signal,
chain=True) if the sigaction() function is not available: don't call
the previous signal handler if it's NULL.
@ovchinnikov-v-a
Copy link
Author

It's our customer's OS we're porting CPython to.

Can you share the OS name? Or is it private? If yes, please mention the OS version.

It's covered by NDA so far, sorry. Might be disclosed later provided they decide to publicly share OS and this port in particular.

miss-islington added a commit that referenced this issue Sep 8, 2022
Fix the faulthandler implementation of faulthandler.register(signal,
chain=True) if the sigaction() function is not available: don't call
the previous signal handler if it's NULL.
(cherry picked from commit c580a81)

Co-authored-by: Victor Stinner <vstinner@python.org>
miss-islington added a commit that referenced this issue Sep 8, 2022
Fix the faulthandler implementation of faulthandler.register(signal,
chain=True) if the sigaction() function is not available: don't call
the previous signal handler if it's NULL.
(cherry picked from commit c580a81)

Co-authored-by: Victor Stinner <vstinner@python.org>
@ovchinnikov-v-a
Copy link
Author

ovchinnikov-v-a commented Sep 8, 2022

Fixed with #96666, also back-ported to 3.10 and 3.11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 only security fixes type-bug An unexpected behavior, bug, or error type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Development

No branches or pull requests

3 participants