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

Linked PRs

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions