gh-127945: fix thread safety of creating instances of ctypes structures (#131716)

This commit is contained in:
Kumar Aditya 2025-03-30 15:22:30 +05:30 committed by GitHub
parent edfbd8c062
commit bc5a028c13
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 82 additions and 23 deletions

View file

@ -109,6 +109,7 @@ bytes(cdata)
#include "pycore_call.h" // _PyObject_CallNoArgs()
#include "pycore_ceval.h" // _Py_EnterRecursiveCall()
#include "pycore_unicodeobject.h" // _PyUnicode_EqualToASCIIString()
#include "pycore_pyatomic_ft_wrappers.h"
#ifdef MS_WIN32
# include "pycore_modsupport.h" // _PyArg_NoKeywords()
#endif
@ -710,13 +711,15 @@ StructUnionType_init(PyObject *self, PyObject *args, PyObject *kwds, int isStruc
if (baseinfo == NULL) {
return 0;
}
int ret = 0;
STGINFO_LOCK(baseinfo);
/* copy base info */
if (PyCStgInfo_clone(info, baseinfo) < 0) {
return -1;
ret = PyCStgInfo_clone(info, baseinfo);
if (ret >= 0) {
stginfo_set_dict_final_lock_held(baseinfo); /* set the 'final' flag in the baseclass info */
}
info->flags &= ~DICTFLAG_FINAL; /* clear the 'final' flag in the subclass info */
baseinfo->flags |= DICTFLAG_FINAL; /* set the 'final' flag in the baseclass info */
STGINFO_UNLOCK();
return ret;
}
return 0;
}
@ -3122,6 +3125,7 @@ PyCData_MallocBuffer(CDataObject *obj, StgInfo *info)
* access.
*/
assert (Py_REFCNT(obj) == 1);
assert(stginfo_get_dict_final(info) == 1);
if ((size_t)info->size <= sizeof(obj->b_value)) {
/* No need to call malloc, can use the default buffer */
@ -3167,7 +3171,7 @@ PyCData_FromBaseObj(ctypes_state *st,
return NULL;
}
info->flags |= DICTFLAG_FINAL;
stginfo_set_dict_final(info);
cmem = (CDataObject *)((PyTypeObject *)type)->tp_alloc((PyTypeObject *)type, 0);
if (cmem == NULL) {
return NULL;
@ -3216,7 +3220,7 @@ PyCData_AtAddress(ctypes_state *st, PyObject *type, void *buf)
return NULL;
}
info->flags |= DICTFLAG_FINAL;
stginfo_set_dict_final(info);
pd = (CDataObject *)((PyTypeObject *)type)->tp_alloc((PyTypeObject *)type, 0);
if (!pd) {
@ -3451,7 +3455,7 @@ generic_pycdata_new(ctypes_state *st,
return NULL;
}
info->flags |= DICTFLAG_FINAL;
stginfo_set_dict_final(info);
obj = (CDataObject *)type->tp_alloc(type, 0);
if (!obj)

View file

@ -6,6 +6,8 @@
#include "pycore_moduleobject.h" // _PyModule_GetState()
#include "pycore_typeobject.h" // _PyType_GetModuleState()
#include "pycore_critical_section.h"
#include "pycore_pyatomic_ft_wrappers.h"
// Do we support C99 complex types in ffi?
// For Apple's libffi, this must be determined at runtime (see gh-128156).
@ -375,7 +377,7 @@ typedef struct CFieldObject {
typedef struct {
int initialized;
Py_ssize_t size; /* number of bytes */
Py_ssize_t align; /* alignment requirements */
Py_ssize_t align; /* alignment reqwuirements */
Py_ssize_t length; /* number of fields */
ffi_type ffi_type_pointer;
PyObject *proto; /* Only for Pointer/ArrayObject */
@ -390,6 +392,10 @@ typedef struct {
PyObject *checker;
PyObject *module;
int flags; /* calling convention and such */
#ifdef Py_GIL_DISABLED
PyMutex mutex; /* critical section mutex */
#endif
uint8_t dict_final;
/* pep3118 fields, pointers need PyMem_Free */
char *format;
@ -399,6 +405,51 @@ typedef struct {
/* Py_ssize_t *suboffsets; */ /* unused in ctypes */
} StgInfo;
/*
To ensure thread safety in the free threading build, the `STGINFO_LOCK` and
`STGINFO_UNLOCK` macros use critical sections to protect against concurrent
modifications to `StgInfo` and assignment of the `dict_final` field. Once
`dict_final` is set, `StgInfo` is treated as read-only, and no further
modifications are allowed. This approach allows most read operations to
proceed without acquiring the critical section lock.
The `dict_final` field is written only after all other modifications to
`StgInfo` are complete. The reads and writes of `dict_final` use the
sequentially consistent memory ordering to ensure that all other fields are
visible to other threads before the `dict_final` bit is set.
*/
#define STGINFO_LOCK(stginfo) Py_BEGIN_CRITICAL_SECTION_MUT(&(stginfo)->mutex)
#define STGINFO_UNLOCK() Py_END_CRITICAL_SECTION()
static inline uint8_t
stginfo_get_dict_final(StgInfo *info)
{
return FT_ATOMIC_LOAD_UINT8(info->dict_final);
}
static inline void
stginfo_set_dict_final_lock_held(StgInfo *info)
{
_Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&info->mutex);
FT_ATOMIC_STORE_UINT8(info->dict_final, 1);
}
// Set the `dict_final` bit in StgInfo. It checks if the bit is already set
// and in that avoids acquiring the critical section (general case).
static inline void
stginfo_set_dict_final(StgInfo *info)
{
if (stginfo_get_dict_final(info) == 1) {
return;
}
STGINFO_LOCK(info);
stginfo_set_dict_final_lock_held(info);
STGINFO_UNLOCK();
}
extern int PyCStgInfo_clone(StgInfo *dst_info, StgInfo *src_info);
extern void ctype_clear_stginfo(StgInfo *info);
@ -427,8 +478,6 @@ PyObject *_ctypes_callproc(ctypes_state *st,
#define TYPEFLAG_ISPOINTER 0x100
#define TYPEFLAG_HASPOINTER 0x200
#define DICTFLAG_FINAL 0x1000
struct tagPyCArgObject {
PyObject_HEAD
ffi_type *pffi_type;

View file

@ -34,6 +34,10 @@ PyCStgInfo_clone(StgInfo *dst_info, StgInfo *src_info)
dst_info->ffi_type_pointer.elements = NULL;
memcpy(dst_info, src_info, sizeof(StgInfo));
#ifdef Py_GIL_DISABLED
dst_info->mutex = (PyMutex){0};
#endif
dst_info->dict_final = 0;
Py_XINCREF(dst_info->proto);
Py_XINCREF(dst_info->argtypes);
@ -248,23 +252,23 @@ PyCStructUnionType_update_stginfo(PyObject *type, PyObject *fields, int isStruct
ctypes_state *st = get_module_state_by_def(Py_TYPE(type));
StgInfo *stginfo;
if (PyStgInfo_FromType(st, type, &stginfo) < 0) {
goto error;
return -1;
}
if (!stginfo) {
PyErr_SetString(PyExc_TypeError,
"ctypes state is not initialized");
goto error;
return -1;
}
PyObject *base = (PyObject *)((PyTypeObject *)type)->tp_base;
StgInfo *baseinfo;
if (PyStgInfo_FromType(st, base, &baseinfo) < 0) {
goto error;
return -1;
}
STGINFO_LOCK(stginfo);
/* If this structure/union is already marked final we cannot assign
_fields_ anymore. */
if (stginfo->flags & DICTFLAG_FINAL) {/* is final ? */
if (stginfo_get_dict_final(stginfo) == 1) {/* is final ? */
PyErr_SetString(PyExc_AttributeError,
"_fields_ is final");
goto error;
@ -422,12 +426,13 @@ PyCStructUnionType_update_stginfo(PyObject *type, PyObject *fields, int isStruct
goto error;
}
assert(info);
STGINFO_LOCK(info);
stginfo->ffi_type_pointer.elements[ffi_ofs + i] = &info->ffi_type_pointer;
if (info->flags & (TYPEFLAG_ISPOINTER | TYPEFLAG_HASPOINTER))
stginfo->flags |= TYPEFLAG_HASPOINTER;
info->flags |= DICTFLAG_FINAL; /* mark field type final */
stginfo_set_dict_final_lock_held(info); /* mark field type final */
STGINFO_UNLOCK();
if (-1 == PyObject_SetAttr(type, prop->name, prop_obj)) {
goto error;
}
@ -461,15 +466,15 @@ PyCStructUnionType_update_stginfo(PyObject *type, PyObject *fields, int isStruct
/* We did check that this flag was NOT set above, it must not
have been set until now. */
if (stginfo->flags & DICTFLAG_FINAL) {
if (stginfo_get_dict_final(stginfo) == 1) {
PyErr_SetString(PyExc_AttributeError,
"Structure or union cannot contain itself");
goto error;
}
stginfo->flags |= DICTFLAG_FINAL;
stginfo_set_dict_final_lock_held(stginfo);
retval = MakeAnonFields(type);
error:
error:;
Py_XDECREF(layout_func);
Py_XDECREF(kwnames);
Py_XDECREF(align_obj);
@ -478,6 +483,7 @@ error:
Py_XDECREF(layout_fields);
Py_XDECREF(layout);
Py_XDECREF(format_spec_obj);
STGINFO_UNLOCK();
return retval;
}