From 3043f80bbd54b846edb2ac3f6a1af21abb1b017a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 6 Feb 2025 10:53:31 +0100 Subject: [PATCH 1/3] fix UBSan failures for `OverlappedObject` --- Modules/overlapped.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/Modules/overlapped.c b/Modules/overlapped.c index 308a0dab7fab1a..0a3379caab2007 100644 --- a/Modules/overlapped.c +++ b/Modules/overlapped.c @@ -124,6 +124,8 @@ typedef struct { }; } OverlappedObject; +#define OverlappedObject_CAST(op) ((OverlappedObject *)(op)) + static inline void steal_buffer(Py_buffer * dst, Py_buffer * src) @@ -668,8 +670,9 @@ _overlapped_Overlapped_impl(PyTypeObject *type, HANDLE event) /* Note (bpo-32710): OverlappedType.tp_clear is not defined to not release buffers while overlapped are still running, to prevent a crash. */ static int -Overlapped_clear(OverlappedObject *self) +Overlapped_clear(PyObject *op) { + OverlappedObject *self = OverlappedObject_CAST(op); switch (self->type) { case TYPE_READ: case TYPE_ACCEPT: { @@ -713,12 +716,13 @@ Overlapped_clear(OverlappedObject *self) } static void -Overlapped_dealloc(OverlappedObject *self) +Overlapped_dealloc(PyObject *op) { DWORD bytes; DWORD olderr = GetLastError(); BOOL wait = FALSE; BOOL ret; + OverlappedObject *self = OverlappedObject_CAST(op); if (!HasOverlappedIoCompleted(&self->overlapped) && self->type != TYPE_NOT_STARTED) @@ -767,7 +771,7 @@ Overlapped_dealloc(OverlappedObject *self) CloseHandle(self->overlapped.hEvent); } - Overlapped_clear(self); + (void)Overlapped_clear(op); SetLastError(olderr); PyTypeObject *tp = Py_TYPE(self); @@ -1641,21 +1645,24 @@ _overlapped_Overlapped_ConnectPipe_impl(OverlappedObject *self, } static PyObject* -Overlapped_getaddress(OverlappedObject *self) +Overlapped_getaddress(PyObject *op, void *Py_UNUSED(closure)) { + OverlappedObject *self = OverlappedObject_CAST(op); return PyLong_FromVoidPtr(&self->overlapped); } static PyObject* -Overlapped_getpending(OverlappedObject *self) +Overlapped_getpending(PyObject *op, void *Py_UNUSED(closure)) { + OverlappedObject *self = OverlappedObject_CAST(op); return PyBool_FromLong(!HasOverlappedIoCompleted(&self->overlapped) && self->type != TYPE_NOT_STARTED); } static int -Overlapped_traverse(OverlappedObject *self, visitproc visit, void *arg) +Overlapped_traverse(PyObject *op, visitproc visit, void *arg) { + OverlappedObject *self = OverlappedObject_CAST(op); switch (self->type) { case TYPE_READ: case TYPE_ACCEPT: @@ -1975,9 +1982,9 @@ static PyMemberDef Overlapped_members[] = { }; static PyGetSetDef Overlapped_getsets[] = { - {"address", (getter)Overlapped_getaddress, NULL, + {"address", Overlapped_getaddress, NULL, "Address of overlapped structure"}, - {"pending", (getter)Overlapped_getpending, NULL, + {"pending", Overlapped_getpending, NULL, "Whether the operation is pending"}, {NULL}, }; From 39c7792006d98a5ea5bcf85ce888c37901e87537 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 8 Feb 2025 10:13:42 +0100 Subject: [PATCH 2/3] fix compilation --- Modules/overlapped.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Modules/overlapped.c b/Modules/overlapped.c index 828ce6259bddcb..c13a965af55d14 100644 --- a/Modules/overlapped.c +++ b/Modules/overlapped.c @@ -1024,7 +1024,7 @@ do_ReadFile(OverlappedObject *self, HANDLE handle, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - Overlapped_clear(self); + (void)Overlapped_clear((PyObject *)self); return SetFromWindowsErr(err); } } @@ -1127,7 +1127,7 @@ do_WSARecv(OverlappedObject *self, HANDLE handle, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - Overlapped_clear(self); + (void)Overlapped_clear((PyObject *)self); return SetFromWindowsErr(err); } } @@ -1254,7 +1254,7 @@ _overlapped_Overlapped_WriteFile_impl(OverlappedObject *self, HANDLE handle, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - Overlapped_clear(self); + (void)Overlapped_clear((PyObject *)self); return SetFromWindowsErr(err); } } @@ -1309,7 +1309,7 @@ _overlapped_Overlapped_WSASend_impl(OverlappedObject *self, HANDLE handle, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - Overlapped_clear(self); + (void)Overlapped_clear((PyObject *)self); return SetFromWindowsErr(err); } } @@ -1362,7 +1362,7 @@ _overlapped_Overlapped_AcceptEx_impl(OverlappedObject *self, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - Overlapped_clear(self); + (void)Overlapped_clear((PyObject *)self); return SetFromWindowsErr(err); } } @@ -1477,7 +1477,7 @@ _overlapped_Overlapped_ConnectEx_impl(OverlappedObject *self, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - Overlapped_clear(self); + (void)Overlapped_clear((PyObject *)self); return SetFromWindowsErr(err); } } @@ -1517,7 +1517,7 @@ _overlapped_Overlapped_DisconnectEx_impl(OverlappedObject *self, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - Overlapped_clear(self); + (void)Overlapped_clear((PyObject *)self); return SetFromWindowsErr(err); } } @@ -1569,7 +1569,7 @@ _overlapped_Overlapped_TransmitFile_impl(OverlappedObject *self, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - Overlapped_clear(self); + (void)Overlapped_clear((PyObject *)self); return SetFromWindowsErr(err); } } @@ -1612,7 +1612,7 @@ _overlapped_Overlapped_ConnectNamedPipe_impl(OverlappedObject *self, case ERROR_IO_PENDING: Py_RETURN_FALSE; default: - Overlapped_clear(self); + (void)Overlapped_clear((PyObject *)self); return SetFromWindowsErr(err); } } From 4619843d2b57e433019d51e118fa28c7ad8a21bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 8 Feb 2025 10:35:34 +0100 Subject: [PATCH 3/3] simplify the PR --- Modules/overlapped.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/Modules/overlapped.c b/Modules/overlapped.c index c13a965af55d14..0eba109f4902fc 100644 --- a/Modules/overlapped.c +++ b/Modules/overlapped.c @@ -668,11 +668,16 @@ _overlapped_Overlapped_impl(PyTypeObject *type, HANDLE event) /* Note (bpo-32710): OverlappedType.tp_clear is not defined to not release - buffers while overlapped are still running, to prevent a crash. */ -static int -Overlapped_clear(PyObject *op) + * buffers while overlapped are still running, to prevent a crash. + * + * Note (gh-111178): Since OverlappedType.tp_clear is not used, we do not + * need to prevent an undefined behaviour by changing the type of 'self'. + * To avoid suppressing unused return values, we however make this function + * return nothing instead of 0, as we never use it. + */ +static void +Overlapped_clear(OverlappedObject *self) { - OverlappedObject *self = OverlappedObject_CAST(op); switch (self->type) { case TYPE_READ: case TYPE_ACCEPT: { @@ -712,7 +717,6 @@ Overlapped_clear(PyObject *op) } } self->type = TYPE_NOT_STARTED; - return 0; } static void @@ -772,7 +776,7 @@ Overlapped_dealloc(PyObject *op) CloseHandle(self->overlapped.hEvent); } - (void)Overlapped_clear(op); + Overlapped_clear(self); SetLastError(olderr); PyTypeObject *tp = Py_TYPE(self); @@ -1024,7 +1028,7 @@ do_ReadFile(OverlappedObject *self, HANDLE handle, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - (void)Overlapped_clear((PyObject *)self); + Overlapped_clear(self); return SetFromWindowsErr(err); } } @@ -1127,7 +1131,7 @@ do_WSARecv(OverlappedObject *self, HANDLE handle, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - (void)Overlapped_clear((PyObject *)self); + Overlapped_clear(self); return SetFromWindowsErr(err); } } @@ -1254,7 +1258,7 @@ _overlapped_Overlapped_WriteFile_impl(OverlappedObject *self, HANDLE handle, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - (void)Overlapped_clear((PyObject *)self); + Overlapped_clear(self); return SetFromWindowsErr(err); } } @@ -1309,7 +1313,7 @@ _overlapped_Overlapped_WSASend_impl(OverlappedObject *self, HANDLE handle, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - (void)Overlapped_clear((PyObject *)self); + Overlapped_clear(self); return SetFromWindowsErr(err); } } @@ -1362,7 +1366,7 @@ _overlapped_Overlapped_AcceptEx_impl(OverlappedObject *self, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - (void)Overlapped_clear((PyObject *)self); + Overlapped_clear(self); return SetFromWindowsErr(err); } } @@ -1477,7 +1481,7 @@ _overlapped_Overlapped_ConnectEx_impl(OverlappedObject *self, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - (void)Overlapped_clear((PyObject *)self); + Overlapped_clear(self); return SetFromWindowsErr(err); } } @@ -1517,7 +1521,7 @@ _overlapped_Overlapped_DisconnectEx_impl(OverlappedObject *self, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - (void)Overlapped_clear((PyObject *)self); + Overlapped_clear(self); return SetFromWindowsErr(err); } } @@ -1569,7 +1573,7 @@ _overlapped_Overlapped_TransmitFile_impl(OverlappedObject *self, case ERROR_IO_PENDING: Py_RETURN_NONE; default: - (void)Overlapped_clear((PyObject *)self); + Overlapped_clear(self); return SetFromWindowsErr(err); } } @@ -1612,7 +1616,7 @@ _overlapped_Overlapped_ConnectNamedPipe_impl(OverlappedObject *self, case ERROR_IO_PENDING: Py_RETURN_FALSE; default: - (void)Overlapped_clear((PyObject *)self); + Overlapped_clear(self); return SetFromWindowsErr(err); } }