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

Struct fields with size > 65535 bytes throw TypeError #126937

Open
Melissa0x1f992 opened this issue Nov 17, 2024 · 3 comments
Open

Struct fields with size > 65535 bytes throw TypeError #126937

Melissa0x1f992 opened this issue Nov 17, 2024 · 3 comments
Assignees
Labels
3.14 new features, bugs and security fixes stdlib Python modules in the Lib dir topic-ctypes type-bug An unexpected behavior, bug, or error

Comments

@Melissa0x1f992
Copy link

Melissa0x1f992 commented Nov 17, 2024

Bug report

Bug description:

PyCField_new_impl() processes these fields with size > 65535 bytes as bitfields, due to relying on the NUM_BITS() function. It throws a TypeError because such fields aren't a valid bitfield type.

Minimal Example

from ctypes import Structure, c_byte

class OneByte(Structure):
  _fields_ = [('b', c_byte),]

TooBig = OneByte * 65536

class CausesTypeError(Structure):
  _fields_ = [('tb', TooBig),] # TypeError: bit fields not allowed for type OneByte_Array_65536

Root Cause

When ctypes/_layout.py creates fields for a type, if the field type is a bitfield, it passes the size in bits to CField()'s bit_size parameter. Otherwise, it passes None. That parameter gets passed as PyCField_new_impl()'s bit_size_obj parameter. However, that parameter goes unused. Instead, when checking if the field is a bitfield, we use NUM_BITS(), passing in size (in bytes), which effectively does an int division by 65536, and check if the result > 0. So, for fields with size > 65535 bytes, they will be treated as bitfields, and rejected as they're not one of the bitfield-compatible types.

Tested versions

No Bug

Python 3.12.1 (main, Jul 26 2024, 14:03:47) [Clang 19.0.0git (https:/github.com/llvm/llvm-project 0a8cd1ed1f4f35905df318015b on emscripten
Python 3.11.5 (tags/v3.11.5:cce6ba9, Aug 24 2023, 14:38:34) [MSC v.1936 64 bit (AMD64)] on win32

Bug

Python 3.14.0a1 (tags/v3.14.0a1:8cdaca8, Oct 15 2024, 20:08:21) [MSC v.1941 64 bit (AMD64)] on win32

Commentary

Found this bug while using the latest 3.14 python build and attempting to import the Scapy library. A TypeError is thrown on import. They use a struct with a large field to hold tables they expect to receive from a Windows API call.

If it's considered expected behavior to limit the size of a field to 64KB, then that would contradict the size test earlier in PyCField_new_impl(), which throws a ValueError only if the size is bigger than what Py_ssize_t can hold.

As long as it's possible to define one of these types without having to actually instantiate, we should add a test that defines a max-size field.

I don't understand the naming of NUM_BITS(). See below for reference. In context, it's tightly integrated in bitfield get/set behavior, in a way that's too convoluted for me to follow. And it's used sporadically throughout Modules/_ctypes/cfield.c, so I'd hesitate to touch other usages. But this bug represents one misusage, so are there others?

static inline
Py_ssize_t NUM_BITS(Py_ssize_t bitsize) {
    return bitsize >> 16;
}

CPython versions tested on:

3.14, CPython main branch

Operating systems tested on:

Windows

Linked PRs

@terryjreedy
Copy link
Member

Running from IDLE editor, there is no output with 3.13.0, but the TypeError with 3.14.0a1. So this seems a regression in 3.14 only.

@brianschubert
Copy link
Contributor

Bisected to ce9f84a (#123352)

cc @encukou

@ZeroIntensity ZeroIntensity added topic-ctypes stdlib Python modules in the Lib dir 3.14 new features, bugs and security fixes labels Nov 18, 2024
@encukou
Copy link
Member

encukou commented Nov 18, 2024

Thank you for testing with the alpha release, @Melissa0x1f992. This early report should give us enough time to fix this properly.


Note this behaviour on 3.13 and below:

>>> CausesTypeError.tb
<Field type=OneByte_Array_65536, ofs=0:0, bits=1>

i.e. even on older versions, tb shows up as a bit-field.
This is mitigated by (what I think is) a coincidence: arrays are not allowed as bit fields, and they use a generic getter that ignores the field's size/bit-count.

I think that the proper fix here will be to separate the size and bit-count, rather than packing them in a single value. I'll try that approach and see if there are any roadblocks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.14 new features, bugs and security fixes stdlib Python modules in the Lib dir topic-ctypes type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

5 participants