Repository navigation
gh-160708: Use the type size for resized ctypes simple objects - #160720
Open
iamsaugatpandey wants to merge 1 commit into
Open
iamsaugatpandey wants to merge 1 commit into
iamsaugatpandey wants to merge 1 commit into
Conversation
After ctypes.resize(), a simple object's buffer is larger than its type. bool() and passing the object to a foreign function still used the buffer size, so bool() read past a 16-byte string literal and the call copied the whole buffer into the fixed-size PyCArgObject value, corrupting the heap. Use the type's size instead, like the value getter does since pythongh-159739. bool() now checks the value's bytes directly, which also covers simple types larger than 16 bytes such as c_longdouble_complex on x86-64.
Author
|
Hi @JelleZijlstra, since you reviewed gh-159739 (#159767, reading resized ctypes scalars using their declared size): this PR is the follow-up for the two places that still use b_size after ctypes.resize(). Those are bool(), which reads past a 16-byte literal, and passing the object to a foreign function, which overflows PyCArgObject.value and corrupts the heap. Both now use info->size, like the value getter. CI is green. Would you have time to take a look? cc @ZeroIntensity |
ZeroIntensity
approved these changes
Nov 4, 2026
Comment on lines
+5538
to
+5540
| assert(info); /* Cannot be NULL for CDataObject instances */ | ||
|
|
||
| /* Only look at the value, not at extra space added by resize(). */ |
Member
There was a problem hiding this comment.
These comments don't seem very helpful:
Suggested change
| assert(info); /* Cannot be NULL for CDataObject instances */ | |
| /* Only look at the value, not at extra space added by resize(). */ | |
| assert(info != NULL); | |
| assert(info); /* Cannot be NULL for CDataObject instances */ | ||
|
|
||
| /* Only look at the value, not at extra space added by resize(). */ | ||
| for (Py_ssize_t i = 0; i < info->size; i++) { |
Member
There was a problem hiding this comment.
It'd be nice to have an assertion documenting the invariant:
Suggested change
| for (Py_ssize_t i = 0; i < info->size; i++) { | |
| assert(info->size <= self->b_size); | |
| for (Py_ssize_t i = 0; i < info->size; i++) { |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
After
ctypes.resize(), the buffer of a simple ctypes object is bigger than the type itself. Two places still used the buffer size (b_size) where they needed the type size:Simple_bool_impl()comparedb_sizebytes against a 16-byte zero string literal. For a resized object it reads past the literal, and it can also return the wrong answer (bool(c_int(0))isTrueif the extra space isn't zero).PyCSimpleType_paramfunc()copiedb_sizebytes intoPyCArgObject.value, which is a fixed-size union, so passing a resized object to a foreign function overwrote the heap.Both now use
info->size, the same as the value getter since gh-159739.bool()checks the value's bytes in a loop instead of comparing with the literal. This also covers simple types larger than 16 bytes, such asc_longdouble_complexon x86-64.Tests:
bool()on a resizedc_intwhose extra space is non-zero, a call totf_iwith a resizedc_int, andbool()on the complex types.