-
-
Notifications
You must be signed in to change notification settings - Fork 32k
gh-126554: ctypes: Correctly handle NULL dlsym values #126555
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
Merged
Merged
Changes from all commits
Commits
Show all changes
54 commits
Select commit
Hold shift + click to select a range
3c3c29d
ctypes: Correctly handle NULL dlsym values
grgalex f71abe8
Update Misc/NEWS.d/next/C_API/2024-11-07-20-24-58.gh-issue-126554.ri1…
grgalex 64a7394
Update Modules/_ctypes/_ctypes.c
grgalex 512a084
Update Modules/_ctypes/_ctypes.c
grgalex cd7d5d7
Add dlerror() comment, handle callproc.c
grgalex cf39b5b
test: Add ctypes null dlsym case
grgalex a72ce98
Add comment to test, minor wip-debugging prints
grgalex c820d16
Remove wip/debug prints
grgalex 6751125
Update Modules/_ctypes/_ctypes.c
grgalex e1f3b43
Update Modules/_ctypes/callproc.c
grgalex 8f7587d
Update Modules/_ctypes/_ctypes.c
grgalex 2b3a88c
Explicitly fail if not AttributeError raised
grgalex 9a1af29
Tidy up Assertion in test
grgalex 3e02e09
Update Modules/_ctypes/_ctypes.c
grgalex fbdbc26
Update Modules/_ctypes/_ctypes.c
grgalex fdf6013
Update Modules/_ctypes/callproc.c
grgalex 3aa29e5
Update Lib/test/test_ctypes/test_dlerror.py
grgalex 0616214
Update Lib/test/test_ctypes/test_dlerror.py
grgalex 6d59ab7
test: Smoothly check for existence of GCC
grgalex 31c621d
test: Clean up libfoo.so compilation command
grgalex 53475a4
Update Lib/test/test_ctypes/test_dlerror.py
grgalex 7fc538e
Update Modules/_ctypes/_ctypes.c
grgalex e20d97f
Update Modules/_ctypes/_ctypes.c
grgalex 95b072e
Update Modules/_ctypes/_ctypes.c
grgalex 048385c
Update Modules/_ctypes/callproc.c
grgalex 658098f
Update Modules/_ctypes/callproc.c
grgalex 6f4b921
Update Modules/_ctypes/_ctypes.c
grgalex 25b6cf9
Update Misc/NEWS.d/next/C_API/2024-11-07-20-24-58.gh-issue-126554.ri1…
grgalex 4bac254
test: Clarify string vs list in subprocess.run
grgalex ee6122c
test: Use string in gcc --version
grgalex 08a7b88
Update Lib/test/test_ctypes/test_dlerror.py
grgalex 8c6b8ae
Write to a pipe instead of stderr
encukou 291bf16
Reorganize dlerror() code
encukou 1cbffcc
Use PyUnicode_DecodeLocale for the error
encukou 5748c13
Clean up PyCFuncPtr_FromDll, uses #define USE_DLERROR
grgalex 65e2b7e
Clean up py_dl_sym, uses #define USE_DLERROR
grgalex 38fbaed
test: Exercise 3 code paths, one for each C function we want to test
grgalex cc8e366
Add empty statement after label, placate C compiler
grgalex 4319332
Move label to before #endif (WIN32)
grgalex 8356cf4
Import ctypes, _ctypes inside test, so we don't get ImportError in wi…
grgalex 1e1e78b
Revert "Import ctypes, _ctypes inside test, so we don't get ImportErr…
grgalex 4e87040
Remove unnecessary goto, fix preprocessor cmd indentation
grgalex 6a3aea7
test: Remove shell=True from gcc check, use separate arguments
grgalex 73918bf
Remove extra whitespace
grgalex 0ca32d4
test: Move imports to func, avoid ImportError on Windows
grgalex 6db5963
test: Explain why we moved imports to within test function
grgalex d179377
dlerror: Use surrogateescape error strategy for decoding locale
grgalex 6933314
Update Modules/_ctypes/callproc.c
grgalex 33cf8b7
Clear DecodeLocale error, indent #ifdef, #undef
grgalex 94dfbaa
Decref the message
encukou cda7771
Update Lib/test/test_ctypes/test_dlerror.py
grgalex 4b1bb47
test: Handle different architectures, where IFUNC not supported
grgalex 2290aa4
Update Modules/_ctypes/callproc.c
grgalex ab22349
Don't break when under 80 chars
grgalex File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
import os | ||
import sys | ||
import unittest | ||
import platform | ||
|
||
FOO_C = r""" | ||
#include <unistd.h> | ||
|
||
/* This is a 'GNU indirect function' (IFUNC) that will be called by | ||
dlsym() to resolve the symbol "foo" to an address. Typically, such | ||
a function would return the address of an actual function, but it | ||
can also just return NULL. For some background on IFUNCs, see | ||
https://willnewton.name/uncategorized/using-gnu-indirect-functions. | ||
|
||
Adapted from Michael Kerrisk's answer: https://stackoverflow.com/a/53590014. | ||
*/ | ||
|
||
asm (".type foo STT_GNU_IFUNC"); | ||
|
||
void *foo(void) | ||
{ | ||
write($DESCRIPTOR, "OK", 2); | ||
return NULL; | ||
} | ||
""" | ||
|
||
|
||
@unittest.skipUnless(sys.platform.startswith('linux'), | ||
'Test only valid for Linux') | ||
class TestNullDlsym(unittest.TestCase): | ||
"""GH-126554: Ensure that we catch NULL dlsym return values | ||
|
||
In rare cases, such as when using GNU IFUNCs, dlsym(), | ||
the C function that ctypes' CDLL uses to get the address | ||
of symbols, can return NULL. | ||
|
||
The objective way of telling if an error during symbol | ||
lookup happened is to call glibc's dlerror() and check | ||
for a non-NULL return value. | ||
|
||
However, there can be cases where dlsym() returns NULL | ||
and dlerror() is also NULL, meaning that glibc did not | ||
encounter any error. | ||
|
||
In the case of ctypes, we subjectively treat that as | ||
an error, and throw a relevant exception. | ||
|
||
This test case ensures that we correctly enforce | ||
this 'dlsym returned NULL -> throw Error' rule. | ||
picnixz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
""" | ||
|
||
def test_null_dlsym(self): | ||
ZeroIntensity marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import subprocess | ||
import tempfile | ||
|
||
# To avoid ImportErrors on Windows, where _ctypes does not have | ||
# dlopen and dlsym, | ||
# import here, i.e., inside the test function. | ||
# The skipUnless('linux') decorator ensures that we're on linux | ||
# if we're executing these statements. | ||
from ctypes import CDLL, c_int | ||
from _ctypes import dlopen, dlsym | ||
|
||
retcode = subprocess.call(["gcc", "--version"], | ||
stdout=subprocess.DEVNULL, | ||
stderr=subprocess.DEVNULL) | ||
if retcode != 0: | ||
self.skipTest("gcc is missing") | ||
|
||
pipe_r, pipe_w = os.pipe() | ||
self.addCleanup(os.close, pipe_r) | ||
self.addCleanup(os.close, pipe_w) | ||
|
||
with tempfile.TemporaryDirectory() as d: | ||
# Create a C file with a GNU Indirect Function (FOO_C) | ||
# and compile it into a shared library. | ||
srcname = os.path.join(d, 'foo.c') | ||
dstname = os.path.join(d, 'libfoo.so') | ||
with open(srcname, 'w') as f: | ||
f.write(FOO_C.replace('$DESCRIPTOR', str(pipe_w))) | ||
args = ['gcc', '-fPIC', '-shared', '-o', dstname, srcname] | ||
p = subprocess.run(args, capture_output=True) | ||
|
||
if p.returncode != 0: | ||
# IFUNC is not supported on all architectures. | ||
if platform.machine() == 'x86_64': | ||
# It should be supported here. Something else went wrong. | ||
p.check_returncode() | ||
else: | ||
# IFUNC might not be supported on this machine. | ||
self.skipTest(f"could not compile indirect function: {p}") | ||
|
||
# Case #1: Test 'PyCFuncPtr_FromDll' from Modules/_ctypes/_ctypes.c | ||
L = CDLL(dstname) | ||
with self.assertRaisesRegex(AttributeError, "function 'foo' not found"): | ||
# Try accessing the 'foo' symbol. | ||
# It should resolve via dlsym() to NULL, | ||
# and since we subjectively treat NULL | ||
# addresses as errors, we should get | ||
# an error. | ||
L.foo | ||
ZeroIntensity marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# Assert that the IFUNC was called | ||
self.assertEqual(os.read(pipe_r, 2), b'OK') | ||
|
||
# Case #2: Test 'CDataType_in_dll_impl' from Modules/_ctypes/_ctypes.c | ||
with self.assertRaisesRegex(ValueError, "symbol 'foo' not found"): | ||
c_int.in_dll(L, "foo") | ||
|
||
# Assert that the IFUNC was called | ||
self.assertEqual(os.read(pipe_r, 2), b'OK') | ||
|
||
# Case #3: Test 'py_dl_sym' from Modules/_ctypes/callproc.c | ||
L = dlopen(dstname) | ||
with self.assertRaisesRegex(OSError, "symbol 'foo' not found"): | ||
dlsym(L, "foo") | ||
|
||
# Assert that the IFUNC was called | ||
self.assertEqual(os.read(pipe_r, 2), b'OK') | ||
|
||
|
||
if __name__ == "__main__": | ||
unittest.main() |
2 changes: 2 additions & 0 deletions
2
Misc/NEWS.d/next/C_API/2024-11-07-20-24-58.gh-issue-126554.ri12eb.rst
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
Fix error handling in :class:`ctypes.CDLL` objects | ||
which could result in a crash in rare situations. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.