From c0538c51921f2d5c894ca5e79c99b9de9e41a36a Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Mon, 29 Dec 2025 14:35:19 -0500 Subject: [PATCH 1/3] gh-140795: Remove 'exc' field in SSLObject The 'exc' field was used by our debug SSL callbacks. Keep the exception in the normal per-thread state to avoid shared mutable state between threads. This also avoids a reference count leak if the Python callback raised an exception because it can be called multiple times per SSL operation. --- Lib/test/test_ssl.py | 14 +++++ Modules/_ssl.c | 109 ++++++++++++++++++++---------------- Modules/_ssl/debughelpers.c | 19 +++++-- 3 files changed, 89 insertions(+), 53 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index ebdf5455163c65..9dc99fbf5cf7d2 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -5277,6 +5277,20 @@ def msg_cb(conn, direction, version, content_type, msg_type, data): with self.assertRaises(TypeError): client_context._msg_callback = object() + def test_msg_callback_exception(self): + client_context, server_context, hostname = testing_context() + + def msg_cb(conn, direction, version, content_type, msg_type, data): + raise RuntimeError("msg_cb exception") + + client_context._msg_callback = msg_cb + server = ThreadedEchoServer(context=server_context, chatty=False) + with server: + with client_context.wrap_socket(socket.socket(), + server_hostname=hostname) as s: + with self.assertRaisesRegex(RuntimeError, "msg_cb exception"): + s.connect((HOST, server.port)) + def test_msg_callback_tls12(self): client_context, server_context, hostname = testing_context() client_context.maximum_version = ssl.TLSVersion.TLSv1_2 diff --git a/Modules/_ssl.c b/Modules/_ssl.c index 5d2f075ed0c675..db005c3dd747b8 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -360,11 +360,6 @@ typedef struct { enum py_ssl_server_or_client socket_type; PyObject *owner; /* Python level "owner" passed to servername callback */ PyObject *server_hostname; - /* Some SSL callbacks don't have error reporting. Callback wrappers - * store exception information on the socket. The handshake, read, write, - * and shutdown methods check for chained exceptions. - */ - PyObject *exc; } PySSLSocket; #define PySSLSocket_CAST(op) ((PySSLSocket *)(op)) @@ -657,18 +652,9 @@ fill_and_set_sslerror(_sslmodulestate *state, PyUnicodeWriter_Discard(writer); } -static int -PySSL_ChainExceptions(PySSLSocket *sslsock) { - if (sslsock->exc == NULL) - return 0; - - _PyErr_ChainExceptions1(sslsock->exc); - sslsock->exc = NULL; - return -1; -} - static PyObject * -PySSL_SetError(PySSLSocket *sslsock, _PySSLError err, const char *filename, int lineno) +PySSL_SetError(PySSLSocket *sslsock, _PySSLError err, PyObject *exc, + const char *filename, int lineno) { PyObject *type; char *errstr = NULL; @@ -776,7 +762,9 @@ PySSL_SetError(PySSLSocket *sslsock, _PySSLError err, const char *filename, int } fill_and_set_sslerror(state, sslsock, type, p, errstr, lineno, e); ERR_clear_error(); - PySSL_ChainExceptions(sslsock); + if (exc != NULL) { + _PyErr_ChainExceptions1(exc); + } return NULL; } @@ -908,7 +896,6 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock, self->shutdown_seen_zero = 0; self->owner = NULL; self->server_hostname = NULL; - self->exc = NULL; /* Make sure the SSL error state is initialized */ ERR_clear_error(); @@ -1029,6 +1016,7 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self) { int ret; _PySSLError err; + PyObject *exc = NULL; int sockstate, nonblocking; PySocketSockObject *sock = GET_SOCKET(self); PyTime_t timeout, deadline = 0; @@ -1064,6 +1052,12 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self) Py_END_ALLOW_THREADS; _PySSL_FIX_ERRNO; + // Get any exception that occurred in a debughelpers.c callback + exc = PyErr_GetRaisedException(); + if (exc != NULL) { + break; + } + if (PyErr_CheckSignals()) goto error; @@ -1098,13 +1092,14 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self) Py_XDECREF(sock); if (ret < 1) - return PySSL_SetError(self, err, __FILE__, __LINE__); - if (PySSL_ChainExceptions(self) < 0) + return PySSL_SetError(self, err, exc, __FILE__, __LINE__); + if (exc != NULL) { + PyErr_SetRaisedException(exc); return NULL; + } Py_RETURN_NONE; error: Py_XDECREF(sock); - PySSL_ChainExceptions(self); return NULL; } @@ -2434,17 +2429,7 @@ _ssl__SSLSocket_owner_set_impl(PySSLSocket *self, PyObject *value) static int PySSL_traverse(PyObject *op, visitproc visit, void *arg) { - PySSLSocket *self = PySSLSocket_CAST(op); - Py_VISIT(self->exc); - Py_VISIT(Py_TYPE(self)); - return 0; -} - -static int -PySSL_clear(PyObject *op) -{ - PySSLSocket *self = PySSLSocket_CAST(op); - Py_CLEAR(self->exc); + Py_VISIT(Py_TYPE(op)); return 0; } @@ -2619,6 +2604,7 @@ _ssl__SSLSocket_sendfile_impl(PySSLSocket *self, int fd, Py_off_t offset, Py_ssize_t retval; int sockstate; _PySSLError err; + PyObject *exc = NULL; PySocketSockObject *sock = GET_SOCKET(self); PyTime_t timeout, deadline = 0; int has_timeout; @@ -2666,6 +2652,11 @@ _ssl__SSLSocket_sendfile_impl(PySSLSocket *self, int fd, Py_off_t offset, Py_END_ALLOW_THREADS; _PySSL_FIX_ERRNO; + exc = PyErr_GetRaisedException(); + if (exc != NULL) { + break; + } + if (PyErr_CheckSignals()) { goto error; } @@ -2715,15 +2706,18 @@ _ssl__SSLSocket_sendfile_impl(PySSLSocket *self, int fd, Py_off_t offset, } Py_XDECREF(sock); if (retval < 0) { - return PySSL_SetError(self, err, __FILE__, __LINE__); + return PySSL_SetError(self, err, exc, __FILE__, __LINE__); } - if (PySSL_ChainExceptions(self) < 0) { + if (exc != NULL) { + PyErr_SetRaisedException(exc); return NULL; } return PyLong_FromSize_t(retval); error: Py_XDECREF(sock); - (void)PySSL_ChainExceptions(self); + if (exc != NULL) { + _PyErr_ChainExceptions1(exc); + } return NULL; } #endif /* BIO_get_ktls_send */ @@ -2747,6 +2741,7 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b) int retval; int sockstate; _PySSLError err; + PyObject *exc = NULL; int nonblocking; PySocketSockObject *sock = GET_SOCKET(self); PyTime_t timeout, deadline = 0; @@ -2797,6 +2792,11 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b) Py_END_ALLOW_THREADS; _PySSL_FIX_ERRNO; + exc = PyErr_GetRaisedException(); + if (exc != NULL) { + break; + } + if (PyErr_CheckSignals()) goto error; @@ -2828,13 +2828,14 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b) Py_XDECREF(sock); if (retval == 0) - return PySSL_SetError(self, err, __FILE__, __LINE__); - if (PySSL_ChainExceptions(self) < 0) + return PySSL_SetError(self, err, exc, __FILE__, __LINE__); + if (exc != NULL) { + PyErr_SetRaisedException(exc); return NULL; + } return PyLong_FromSize_t(count); error: Py_XDECREF(sock); - PySSL_ChainExceptions(self); return NULL; } @@ -2860,7 +2861,7 @@ _ssl__SSLSocket_pending_impl(PySSLSocket *self) _PySSL_FIX_ERRNO; if (count < 0) - return PySSL_SetError(self, err, __FILE__, __LINE__); + return PySSL_SetError(self, err, NULL, __FILE__, __LINE__); else return PyLong_FromLong(count); } @@ -2888,6 +2889,7 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len, int retval; int sockstate; _PySSLError err; + PyObject *exc; int nonblocking; PySocketSockObject *sock = GET_SOCKET(self); PyTime_t timeout, deadline = 0; @@ -2955,6 +2957,11 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len, Py_END_ALLOW_THREADS; _PySSL_FIX_ERRNO; + exc = PyErr_GetRaisedException(); + if (exc != NULL) { + break; + } + if (PyErr_CheckSignals()) goto error; @@ -2986,11 +2993,13 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len, err.ssl == SSL_ERROR_WANT_WRITE); if (retval == 0) { - PySSL_SetError(self, err, __FILE__, __LINE__); + PySSL_SetError(self, err, exc, __FILE__, __LINE__); goto error; } - if (self->exc != NULL) + else if (exc != NULL) { + PyErr_SetRaisedException(exc); goto error; + } done: Py_XDECREF(sock); @@ -3002,7 +3011,6 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len, } error: - PySSL_ChainExceptions(self); Py_XDECREF(sock); if (!group_right_1) { PyBytesWriter_Discard(writer); @@ -3022,6 +3030,7 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self) /*[clinic end generated code: output=ca1aa7ed9d25ca42 input=98d9635cd4e16514]*/ { _PySSLError err; + PyObject *exc = NULL; int sockstate, nonblocking, ret; int zeros = 0; PySocketSockObject *sock = GET_SOCKET(self); @@ -3067,6 +3076,11 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self) Py_END_ALLOW_THREADS; _PySSL_FIX_ERRNO; + exc = PyErr_GetRaisedException(); + if (exc != NULL) { + break; + } + /* If err == 1, a secure shutdown with SSL_shutdown() is complete */ if (ret > 0) break; @@ -3113,11 +3127,14 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self) } if (ret < 0) { Py_XDECREF(sock); - PySSL_SetError(self, err, __FILE__, __LINE__); + PySSL_SetError(self, err, exc, __FILE__, __LINE__); + return NULL; + } + else if (exc != NULL) { + PyErr_SetRaisedException(exc); + Py_XDECREF(sock); return NULL; } - if (self->exc != NULL) - goto error; if (sock) /* It's already INCREF'ed */ return (PyObject *) sock; @@ -3126,7 +3143,6 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self) error: Py_XDECREF(sock); - PySSL_ChainExceptions(self); return NULL; } @@ -3335,7 +3351,6 @@ static PyType_Slot PySSLSocket_slots[] = { {Py_tp_getset, ssl_getsetlist}, {Py_tp_dealloc, PySSL_dealloc}, {Py_tp_traverse, PySSL_traverse}, - {Py_tp_clear, PySSL_clear}, {0, 0}, }; diff --git a/Modules/_ssl/debughelpers.c b/Modules/_ssl/debughelpers.c index 866c172e4996f7..e0cb7ca9a09f91 100644 --- a/Modules/_ssl/debughelpers.c +++ b/Modules/_ssl/debughelpers.c @@ -26,6 +26,8 @@ _PySSL_msg_callback(int write_p, int version, int content_type, return; } + PyObject *exc = PyErr_GetRaisedException(); + PyObject *ssl_socket; /* ssl.SSLSocket or ssl.SSLObject */ if (ssl_obj->owner) PyWeakref_GetRef(ssl_obj->owner, &ssl_socket); @@ -73,13 +75,13 @@ _PySSL_msg_callback(int write_p, int version, int content_type, version, content_type, msg_type, buf, len ); - if (res == NULL) { - ssl_obj->exc = PyErr_GetRaisedException(); - } else { - Py_DECREF(res); - } + Py_XDECREF(res); Py_XDECREF(ssl_socket); + if (exc != NULL) { + _PyErr_ChainExceptions1(exc); + } + PyGILState_Release(threadstate); } @@ -122,10 +124,13 @@ _PySSL_keylog_callback(const SSL *ssl, const char *line) { PyGILState_STATE threadstate; PySSLSocket *ssl_obj = NULL; /* ssl._SSLSocket, borrowed ref */ + PyObject *exc; int res, e; threadstate = PyGILState_Ensure(); + exc = PyErr_GetRaisedException(); + ssl_obj = (PySSLSocket *)SSL_get_app_data(ssl); assert(Py_IS_TYPE(ssl_obj, get_state_sock(ssl_obj)->PySSLSocket_Type)); PyThread_type_lock lock = get_state_sock(ssl_obj)->keylog_lock; @@ -153,10 +158,12 @@ _PySSL_keylog_callback(const SSL *ssl, const char *line) errno = e; PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, ssl_obj->ctx->keylog_filename); - ssl_obj->exc = PyErr_GetRaisedException(); } done: + if (exc != NULL) { + _PyErr_ChainExceptions1(exc); + } PyGILState_Release(threadstate); } From 8cd4d98c03520f490d26ded162004316b6e9ad35 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 6 Jan 2026 15:35:58 -0500 Subject: [PATCH 2/3] Add comment to PySSL_SetError and other changes from review --- Modules/_ssl.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Modules/_ssl.c b/Modules/_ssl.c index db005c3dd747b8..5c1d7a50522a18 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -652,6 +652,9 @@ fill_and_set_sslerror(_sslmodulestate *state, PyUnicodeWriter_Discard(writer); } +// Set the appropriate SSL error exception. +// err - error information from SSL and libc +// exc - if not NULL, an exception from _debughelpers.c callback to be chained static PyObject * PySSL_SetError(PySSLSocket *sslsock, _PySSLError err, PyObject *exc, const char *filename, int lineno) @@ -762,9 +765,7 @@ PySSL_SetError(PySSLSocket *sslsock, _PySSLError err, PyObject *exc, } fill_and_set_sslerror(state, sslsock, type, p, errstr, lineno, e); ERR_clear_error(); - if (exc != NULL) { - _PyErr_ChainExceptions1(exc); - } + _PyErr_ChainExceptions1(exc); // chain any exceptions from callbacks return NULL; } @@ -3131,8 +3132,8 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self) return NULL; } else if (exc != NULL) { - PyErr_SetRaisedException(exc); Py_XDECREF(sock); + PyErr_SetRaisedException(exc); return NULL; } if (sock) From 31f02369bd556114fa70709ece8877452b176298 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Mon, 12 Jan 2026 11:47:00 -0500 Subject: [PATCH 3/3] Changes from review --- Modules/_ssl.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Modules/_ssl.c b/Modules/_ssl.c index 5c1d7a50522a18..7dd57e7892af41 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -1100,6 +1100,7 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self) } Py_RETURN_NONE; error: + assert(exc == NULL); Py_XDECREF(sock); return NULL; } @@ -2836,6 +2837,7 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b) } return PyLong_FromSize_t(count); error: + assert(exc == NULL); Py_XDECREF(sock); return NULL; } @@ -2890,7 +2892,7 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len, int retval; int sockstate; _PySSLError err; - PyObject *exc; + PyObject *exc = NULL; int nonblocking; PySocketSockObject *sock = GET_SOCKET(self); PyTime_t timeout, deadline = 0; @@ -2995,14 +2997,17 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len, if (retval == 0) { PySSL_SetError(self, err, exc, __FILE__, __LINE__); + exc = NULL; goto error; } else if (exc != NULL) { PyErr_SetRaisedException(exc); + exc = NULL; goto error; } done: + assert(exc == NULL); Py_XDECREF(sock); if (!group_right_1) { return PyBytesWriter_FinishWithSize(writer, count); @@ -3012,6 +3017,7 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len, } error: + assert(exc == NULL); Py_XDECREF(sock); if (!group_right_1) { PyBytesWriter_Discard(writer); @@ -3143,6 +3149,7 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self) Py_RETURN_NONE; error: + assert(exc == NULL); Py_XDECREF(sock); return NULL; }