Skip to content

Data race: ctypes PyCData_NewGetBuffer reads b_ptr without the critical section _ctypes_resize holds #154524

Description

@Naserume

Bug report

Bug description:

#131336 / #128182 made ctypes.resize and addressof/byref thread-safe by taking a critical section in _ctypes_resize, which reallocates obj->b_ptr:

/*[clinic input]
@critical_section obj
_ctypes.resize
obj: object(subclass_of="clinic_state()->PyCData_Type", type="CDataObject *")
size: Py_ssize_t
/
[clinic start generated code]*/
static PyObject *
_ctypes_resize_impl(PyObject *module, CDataObject *obj, Py_ssize_t size)
/*[clinic end generated code: output=11c89c7dbdbcd53f input=bf5a6aaea8514261]*/
{
ctypes_state *st = get_module_state(module);
StgInfo *info;
int result = PyStgInfo_FromObject(st, (PyObject *)obj, &info);
if (result < 0) {
return NULL;
}
if (info == NULL) {
PyErr_SetString(PyExc_TypeError,
"expected ctypes instance");
return NULL;
}
if (size < info->size) {
PyErr_Format(PyExc_ValueError,
"minimum size is %zd",
info->size);
return NULL;
}
if (obj->b_needsfree == 0) {
PyErr_Format(PyExc_ValueError,
"Memory cannot be resized because this object doesn't own it");
return NULL;
}
if ((size_t)size <= sizeof(obj->b_value)) {
/* internal default buffer is large enough */
obj->b_size = size;
goto done;
}
if (!_CDataObject_HasExternalBuffer(obj)) {
/* We are currently using the objects default buffer, but it
isn't large enough any more. */
void *ptr = PyMem_Calloc(1, size);
if (ptr == NULL)
return PyErr_NoMemory();
memmove(ptr, obj->b_ptr, obj->b_size);
obj->b_ptr = ptr;
obj->b_size = size;
} else {
void * ptr = PyMem_Realloc(obj->b_ptr, size);
if (ptr == NULL)
return PyErr_NoMemory();
obj->b_ptr = ptr;
obj->b_size = size;
}
done:
Py_RETURN_NONE;
}
static PyObject *

But the buffer-protocol getbuffer, PyCData_NewGetBuffer (reached via memoryview(cdata)), reads self->b_ptr without that critical section:

static int
PyCData_NewGetBuffer(PyObject *myself, Py_buffer *view, int flags)
{
CDataObject *self = _CDataObject_CAST(myself);
ctypes_state *st = get_module_state_by_def(Py_TYPE(Py_TYPE(myself)));
StgInfo *info;
if (PyStgInfo_FromObject(st, myself, &info) < 0) {
return -1;
}
assert(info);
PyObject *item_type = PyCData_item_type(st, (PyObject*)Py_TYPE(myself));
if (item_type == NULL) {
return 0;
}
if (view == NULL) return 0;
StgInfo *item_info;
if (PyStgInfo_FromType(st, item_type, &item_info) < 0) {
return -1;
}
assert(item_info);
view->buf = self->b_ptr;
view->obj = Py_NewRef(myself);
view->len = self->b_size;

So memoryview(obj) racing ctypes.resize(obj) reads a b_ptr that resize concurrently reallocates.

Reproducer:

import ctypes
from threading import Thread

buf = (ctypes.c_char * 64)()

def viewer():
    for _ in range(20000):
        try:
            memoryview(buf)
        except Exception:
            pass

def resizer():
    for i in range(20000):
        try:
            ctypes.resize(buf, 128 if (i & 1) else 256)
        except Exception:
            pass

threads  = [Thread(target=viewer)  for _ in range(6)]
threads += [Thread(target=resizer) for _ in range(2)]
for t in threads: t.start()
for t in threads: t.join()

TSAN Report:

==================
WARNING: ThreadSanitizer: data race (pid=1524546)
  Read of size 8 at 0x7fffb6b50208 by thread T1:
    #0 PyCData_NewGetBuffer /cpython/./Modules/_ctypes/_ctypes.c:3129:23 
    #1 PyObject_GetBuffer /cpython/Objects/abstract.c:455:15
    #2 _PyManagedBuffer_FromObject /cpython/Objects/memoryobject.c:97:9 
    #3 PyMemoryView_FromObjectAndFlags /cpython/Objects/memoryobject.c:813:42 
    #4 PyMemoryView_FromObject /cpython/Objects/memoryobject.c:856:12
    #5 memoryview_impl /cpython/Objects/memoryobject.c:1017:12 
    #6 memoryview /cpython/Objects/clinic/memoryobject.c.h:63:20 
    #7 type_call /cpython/Objects/typeobject.c:2472:11 
    #8 _PyObject_MakeTpCall /cpython/Objects/call.c:242:18
    #9 _PyObject_VectorcallTstate /cpython/./Include/internal/pycore_call.h:142:16)
    #10 PyObject_Vectorcall /cpython/Objects/call.c:327:12
    #11 _Py_VectorCall_StackRefSteal /cpython/Python/ceval.c:726:11 
    #12 _PyEval_EvalFrameDefault /cpython/Python/generated_cases.c.h:4559:35 

  Previous write of size 8 at 0x7fffb6b50208 by thread T7:
    #0 _ctypes_resize_impl /cpython/./Modules/_ctypes/callproc.c
    #1 _ctypes_resize /cpython/./Modules/_ctypes/clinic/callproc.c.h:139:20
    #2 _Py_BuiltinCallFast_StackRef /cpython/Python/ceval.c:817:11
    #3 _PyEval_EvalFrameDefault /cpython/Python/generated_cases.c.h:2510:35
    #4 _PyEval_EvalFrame /cpython/./Include/internal/pycore_ceval.h:122:16

SUMMARY: ThreadSanitizer: data race /cpython/./Modules/_ctypes/_ctypes.c:3129:23 in PyCData_NewGetBuffer
==================

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions