python / cpython

The Python programming language
https://www.python.org
Other
63.52k stars 30.43k forks source link

Struct fields with size > 65535 bytes throw TypeError #126937

Open Melissa0x1f992 opened 6 hours ago

Melissa0x1f992 commented 6 hours ago

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 commented 4 hours ago

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 commented 4 hours ago

Bisected to ce9f84a47bfbafedd09a25d0f6f0c8209550fb6c (#123352)

cc @encukou