Title: Undefined behavior calling C functions with ctypes.Union arguments
Type: crash Stage: resolved
Components: ctypes Versions: Python 3.6, Python 3.5, Python 2.7
Status: closed Resolution: duplicate
Dependencies: 16575 Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, belopolsky, eryksun, meador.inge, mwichmann, tilsche, vinay.sajip
Priority: normal Keywords:

Created on 2016-03-23 20:52 by tilsche, last changed 2020-04-07 12:59 by mwichmann. This issue is now closed.

File name Uploaded Description Edit tilsche, 2016-03-23 20:52 minimal example
libfoo.c tilsche, 2016-04-05 10:37 example library taking union argument tilsche, 2016-04-05 10:40 example call to the library that crashes
Messages (7)
msg262307 - (view) Author: Thomas (tilsche) * Date: 2016-03-23 20:52
Passing ctypes.Union types as arguments crashes python.

Attached is a minimal example to reproduce. Due to undefined behavior, you may have to increase the union _fields_ to reproduce. I tested with 3.5.1 and 2.7.11.

It seems that cffi treats the union as a normal struct. In classify_argument, it loops through the type->elements. The byte_offset increases for each union element until pos exceeds enum x86_64_reg_class classes[MAX_CLASSES], causing an invalid write here:

size_t pos = byte_offset / 8;
classes[i + pos] = merge_classes (subclasses[i], classes[i + pos]);

I am quite scared considering the lack of any index checks in this code. At this point I'm not yet sure whether this is a bug in ctypes or libffi.

#0  classify_argument (type=0xce41b8, classes=0x7fffffffb4e0, byte_offset=8) at Python-3.5.1/Modules/_ctypes/libffi/src/x86/ffi64.c:248
#1  0x00007ffff6bc6409 in examine_argument (type=0xce41b8, classes=0x7fffffffb4e0, in_return=false, pngpr=0x7fffffffb4dc, pnsse=0x7fffffffb4d8)
    at Python-3.5.1/Modules/_ctypes/libffi/src/x86/ffi64.c:318
#2  0x00007ffff6bc68ce in ffi_call (cif=0x7fffffffb590, fn=0x7ffff751d5a0, rvalue=0x7fffffffb660, avalue=0x7fffffffb640) at Python-3.5.1/Modules/_ctypes/libffi/src/x86/ffi64.c:462
#3  0x00007ffff6bb589e in _call_function_pointer (flags=4353, pProc=0x7ffff751d5a0, avalues=0x7fffffffb640, atypes=0x7fffffffb620, restype=0xcdd488, resmem=0x7fffffffb660, argcount=1)
    at Python-3.5.1/Modules/_ctypes/callproc.c:811
#4  0x00007ffff6bb6593 in _ctypes_callproc (pProc=0x7ffff751d5a0, argtuple=0xc8b3e8, flags=4353, argtypes=0xcb2098, restype=0xcdcd38, checker=0x0)
    at Python-3.5.1/Modules/_ctypes/callproc.c:1149
#5  0x00007ffff6baf84f in PyCFuncPtr_call (self=0xcf3708, inargs=0xc8b3e8, kwds=0x0) at Python-3.5.1/Modules/_ctypes/_ctypes.c:3869
#6  0x000000000043b66a in PyObject_Call (func=0xcf3708, arg=0xc8b3e8, kw=0x0) at ../../Python-3.5.1/Objects/abstract.c:2165
msg262310 - (view) Author: Thomas (tilsche) * Date: 2016-03-23 21:08
Note []

> Although ‘libffi’ has no special support for unions or bit-fields, it is perfectly happy passing structures back and forth. You must first describe the structure to ‘libffi’ by creating a new ffi_type object for it.
msg262342 - (view) Author: Thomas (tilsche) * Date: 2016-03-24 13:27
So after some more pondering about the issue I read the documentation again:

> Warning ctypes does not support passing unions or structures with bit-fields to functions by value.

Previously I always read this as 'does not support passing unions with bit-fields'... I guess it is meant otherwise. IMHO this should be formulated more clearly, like: "does not support passing structures with bit-fields or unions to functions by value.".

Also I would strongly argue to generally prohibit this with an exception instead of just trying if libffi maybe handles this on the current architecture. libffi clearly does not support unions. This just introduces subtle bugs.

See also:
msg262397 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2016-03-25 03:04
> I would strongly argue to generally prohibit this 
> with an exception

I agree. A warning in the tutorial isn't sufficient. ctypes should raise an error when setting a union or bitfield struct type in argtypes or when passing one by value.

Here's some background to flesh out this issue. Both struct and union types are defined as the libffi type FFI_TYPE_STRUCT (13), since FFI_TYPE_UNION doesn't exist. 

    class MyUnion(ctypes.Union):
        _fields_ = [("x%s" % i, ctypes.c_double) for i in range(11)]

    >>> stgdict(MyUnion).length
    >>> stgdict(MyUnion).ffi_type_pointer
    size: 8
    alignment: 8
    type: 13
    elements: 17918992

    >>> (ctypes.c_void_p * 12).from_address(17918992)[:]
    [140354450953784, 140354450953784, 140354450953784, 140354450953784,
     140354450953784, 140354450953784, 140354450953784, 140354450953784,
     140354450953784, 140354450953784, 140354450953784, None]

    >>> ffi_type.from_address(140354450953784)
    size: 8
    alignment: 8
    type: 3
    elements: LP_LP_ffi_type(<NULL>)


    class MyStruct(ctypes.Structure):
        _fields_ = [("x%s" % i, ctypes.c_double) for i in range(11)]

    >>> stgdict(MyStruct).length
    >>> stgdict(MyStruct).ffi_type_pointer
    size: 88
    alignment: 8
    type: 13
    elements: 17868096
    >>> (ctypes.c_void_p * 12).from_address(17868096)[:]
    [140354450953784, 140354450953784, 140354450953784, 140354450953784,
     140354450953784, 140354450953784, 140354450953784, 140354450953784,
     140354450953784, 140354450953784, 140354450953784, None]

As shown above, FFI_TYPE_STRUCT uses the `elements` array in its ffi_type. Each element is a pointer to an ffi_type for the corresponding field in the struct or union. This array is terminated by a NULL pointer. 

The C data size of a MyUnion instance is 8 bytes. This is less than 32 bytes (i.e. four 8-byte words), so the code in classify_argument rightfully assumes the argument won't be passed on the stack. Thus it classifies the register type(s) to use, presuming it's dealing with a struct. But since it's a union the total size of the elements is unrelated to the union's size. libffi would need to implement an FFI_TYPE_UNION type to support this as a separate case. Otherwise ctypes needs to actively forbid passing a union by value.
msg262899 - (view) Author: Thomas (tilsche) * Date: 2016-04-05 10:40
Thanks Eryk for the additional explanation. I added a more elaborate example that doesn't abuse the standard c function that actually doesn't expect a union:

 % gcc -shared -fPIC libfoo.c -o -Wall
 % python                             
*** stack smashing detected ***: python terminated
[1]    28463 segmentation fault (core dumped)  python

The underling issue is exactly the same as previously described.

I still argue that ctypes should refuse to attempt such a call, and the documentation should be clarified, as long as libffi does not support unions.
msg357903 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-12-06 06:59
This is now fixed as a result of fixing bpo-16575. The script now gives an error message:

$ python3.9 ~/tmp/ 
Traceback (most recent call last):
  File "/home/vinay/tmp/", line 10, in <module>
    sin.argtypes = [MyUnion]
TypeError: item 1 in _argtypes_ passes a union by value, which is unsupported.
msg365904 - (view) Author: Mats Wichmann (mwichmann) * Date: 2020-04-07 12:59
For readers who got here via a search after hitting the new traceback, note the fix in bpo-16575 was reverted. It's still a duplicate issue, so follow the progress there.
Date User Action Args
2020-04-07 12:59:46mwichmannsetnosy: + mwichmann
messages: + msg365904
2020-01-12 08:30:24vinay.sajipsetdependencies: + ctypes: unions as arguments
resolution: fixed -> duplicate
2019-12-06 06:59:42vinay.sajipsetstatus: open -> closed
resolution: fixed
messages: + msg357903

stage: resolved
2017-02-23 08:11:32vinay.sajipsetnosy: + vinay.sajip
2016-04-05 10:40:32tilschesetfiles: +

messages: + msg262899
2016-04-05 10:37:03tilschesetfiles: + libfoo.c
2016-03-25 03:04:40eryksunsetnosy: + eryksun

messages: + msg262397
versions: + Python 3.6
2016-03-24 13:27:15tilschesetmessages: + msg262342
title: Segfault in cffi with ctypes.union argument -> Undefined behavior calling C functions with ctypes.Union arguments
2016-03-23 21:08:14tilschesetmessages: + msg262310
2016-03-23 20:57:56SilentGhostsetnosy: + amaury.forgeotdarc, belopolsky, meador.inge
2016-03-23 20:52:16tilschecreate