[3.13] gh-134698: Hold a lock when the thread state is detached in ssl (GH-134724...

Lock when the thread state is detached.
(cherry picked from commit e047a35b23c1aa69ab8d5da56f36319cec4d36b8) or really from the 3.14 backport fd565fdfc9c0001900d03d627e2fda83f1bcca90

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py
index 83cf7e08dfa39869d6d709c230a6dc3ed79867e6..36f2fdddc96233ac2459ab5a134c0baacee430f2 100644 (file)
--- a/Lib/test/test_ssl.py
+++ b/Lib/test/test_ssl.py
@@ -1266,6 +1266,25 @@ class ContextTests(unittest.TestCase):
         # Make sure the password function isn't called if it isn't needed
         ctx.load_cert_chain(CERTFILE, password=getpass_exception)
 
+    @threading_helper.requires_working_threading()
+    def test_load_cert_chain_thread_safety(self):
+        # gh-134698: _ssl detaches the thread state (and as such,
+        # releases the GIL and critical sections) around expensive
+        # OpenSSL calls. Unfortunately, OpenSSL structures aren't
+        # thread-safe, so executing these calls concurrently led
+        # to crashes.
+        ctx = ssl.create_default_context()
+
+        def race():
+            ctx.load_cert_chain(CERTFILE)
+
+        threads = [threading.Thread(target=race) for _ in range(8)]
+        with threading_helper.catch_threading_exception() as cm:
+            with threading_helper.start_threads(threads):
+                pass
+
+            self.assertIsNone(cm.exc_value)
+
     def test_load_verify_locations(self):
         ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
         ctx.load_verify_locations(CERTFILE)

diff --git a/Misc/NEWS.d/next/Library/2025-05-26-10-52-27.gh-issue-134698.aJ1mZ1.rst b/Misc/NEWS.d/next/Library/2025-05-26-10-52-27.gh-issue-134698.aJ1mZ1.rst
new file mode 100644 (file)
index 0000000..cf3901a
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2025-05-26-10-52-27.gh-issue-134698.aJ1mZ1.rst
@@ -0,0 +1,2 @@
+Fix a crash when calling methods of :class:`ssl.SSLContext` or
+:class:`ssl.SSLSocket` across multiple threads.

diff --git a/Modules/_ssl.c b/Modules/_ssl.c
index aa846f6864144754844e3f25abe0ca90ceb881c5..981c3d6a936ec87b1473f844abc737436e1d99bc 100644 (file)
--- a/Modules/_ssl.c
+++ b/Modules/_ssl.c
@@ -42,14 +42,14 @@
 /* Redefined below for Windows debug builds after important #includes */
 #define _PySSL_FIX_ERRNO
 
-#define PySSL_BEGIN_ALLOW_THREADS_S(save) \
-    do { (save) = PyEval_SaveThread(); } while(0)
-#define PySSL_END_ALLOW_THREADS_S(save) \
-    do { PyEval_RestoreThread(save); _PySSL_FIX_ERRNO; } while(0)
-#define PySSL_BEGIN_ALLOW_THREADS { \
+#define PySSL_BEGIN_ALLOW_THREADS_S(save, mutex) \
+    do { (save) = PyEval_SaveThread(); PyMutex_Lock(mutex); } while(0)
+#define PySSL_END_ALLOW_THREADS_S(save, mutex) \
+    do { PyMutex_Unlock(mutex); PyEval_RestoreThread(save); _PySSL_FIX_ERRNO; } while(0)
+#define PySSL_BEGIN_ALLOW_THREADS(self) { \
             PyThreadState *_save = NULL;  \
-            PySSL_BEGIN_ALLOW_THREADS_S(_save);
-#define PySSL_END_ALLOW_THREADS PySSL_END_ALLOW_THREADS_S(_save); }
+            PySSL_BEGIN_ALLOW_THREADS_S(_save, &self->tstate_mutex);
+#define PySSL_END_ALLOW_THREADS(self) PySSL_END_ALLOW_THREADS_S(_save, &self->tstate_mutex); }
 
 #if defined(HAVE_POLL_H)
 #include <poll.h>
@@ -304,6 +304,9 @@ typedef struct {
     PyObject *psk_client_callback;
     PyObject *psk_server_callback;
 #endif
+    /* Lock to synchronize calls when the thread state is detached.
+       See also gh-134698. */
+    PyMutex tstate_mutex;
 } PySSLContext;
 
 typedef struct {
@@ -329,6 +332,9 @@ typedef struct {
      * and shutdown methods check for chained exceptions.
      */
     PyObject *exc;
+    /* Lock to synchronize calls when the thread state is detached.
+       See also gh-134698. */
+    PyMutex tstate_mutex;
 } PySSLSocket;
 
 typedef struct {
@@ -840,13 +846,14 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
     self->server_hostname = NULL;
     self->err = err;
     self->exc = NULL;
+    self->tstate_mutex = (PyMutex){0};
 
     /* Make sure the SSL error state is initialized */
     ERR_clear_error();
 
-    PySSL_BEGIN_ALLOW_THREADS
+    PySSL_BEGIN_ALLOW_THREADS(sslctx)
     self->ssl = SSL_new(ctx);
-    PySSL_END_ALLOW_THREADS
+    PySSL_END_ALLOW_THREADS(sslctx)
     if (self->ssl == NULL) {
         Py_DECREF(self);
         _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
@@ -912,12 +919,12 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
         BIO_set_nbio(SSL_get_wbio(self->ssl), 1);
     }
 
-    PySSL_BEGIN_ALLOW_THREADS
+    PySSL_BEGIN_ALLOW_THREADS(self)
     if (socket_type == PY_SSL_CLIENT)
         SSL_set_connect_state(self->ssl);
     else
         SSL_set_accept_state(self->ssl);
-    PySSL_END_ALLOW_THREADS
+    PySSL_END_ALLOW_THREADS(self)
 
     self->socket_type = socket_type;
     if (sock != NULL) {
@@ -986,10 +993,10 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
     /* Actually negotiate SSL connection */
     /* XXX If SSL_do_handshake() returns 0, it's also a failure. */
     do {
-        PySSL_BEGIN_ALLOW_THREADS
+        PySSL_BEGIN_ALLOW_THREADS(self)
         ret = SSL_do_handshake(self->ssl);
         err = _PySSL_errno(ret < 1, self->ssl, ret);
-        PySSL_END_ALLOW_THREADS
+        PySSL_END_ALLOW_THREADS(self)
         self->err = err;
 
         if (PyErr_CheckSignals())
@@ -2362,9 +2369,10 @@ PySSL_select(PySocketSockObject *s, int writing, PyTime_t timeout)
     ms = (int)_PyTime_AsMilliseconds(timeout, _PyTime_ROUND_CEILING);
     assert(ms <= INT_MAX);
 
-    PySSL_BEGIN_ALLOW_THREADS
+    Py_BEGIN_ALLOW_THREADS
     rc = poll(&pollfd, 1, (int)ms);
-    PySSL_END_ALLOW_THREADS
+    Py_END_ALLOW_THREADS
+    _PySSL_FIX_ERRNO;
 #else
     /* Guard against socket too large for select*/
     if (!_PyIsSelectable_fd(s->sock_fd))
@@ -2376,13 +2384,14 @@ PySSL_select(PySocketSockObject *s, int writing, PyTime_t timeout)
     FD_SET(s->sock_fd, &fds);
 
     /* Wait until the socket becomes ready */
-    PySSL_BEGIN_ALLOW_THREADS
+    Py_BEGIN_ALLOW_THREADS
     nfds = Py_SAFE_DOWNCAST(s->sock_fd+1, SOCKET_T, int);
     if (writing)
         rc = select(nfds, NULL, &fds, NULL, &tv);
     else
         rc = select(nfds, &fds, NULL, NULL, &tv);
-    PySSL_END_ALLOW_THREADS
+    Py_END_ALLOW_THREADS
+    _PySSL_FIX_ERRNO;
 #endif
 
     /* Return SOCKET_TIMED_OUT on timeout, SOCKET_OPERATION_OK otherwise
@@ -2453,10 +2462,10 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
     }
 
     do {
-        PySSL_BEGIN_ALLOW_THREADS
+        PySSL_BEGIN_ALLOW_THREADS(self)
         retval = SSL_write_ex(self->ssl, b->buf, (size_t)b->len, &count);
         err = _PySSL_errno(retval == 0, self->ssl, retval);
-        PySSL_END_ALLOW_THREADS
+        PySSL_END_ALLOW_THREADS(self)
         self->err = err;
 
         if (PyErr_CheckSignals())
@@ -2514,10 +2523,10 @@ _ssl__SSLSocket_pending_impl(PySSLSocket *self)
     int count = 0;
     _PySSLError err;
 
-    PySSL_BEGIN_ALLOW_THREADS
+    PySSL_BEGIN_ALLOW_THREADS(self)
     count = SSL_pending(self->ssl);
     err = _PySSL_errno(count < 0, self->ssl, count);
-    PySSL_END_ALLOW_THREADS
+    PySSL_END_ALLOW_THREADS(self)
     self->err = err;
 
     if (count < 0)
@@ -2608,10 +2617,10 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len,
         deadline = _PyDeadline_Init(timeout);
 
     do {
-        PySSL_BEGIN_ALLOW_THREADS
+        PySSL_BEGIN_ALLOW_THREADS(self)
         retval = SSL_read_ex(self->ssl, mem, (size_t)len, &count);
         err = _PySSL_errno(retval == 0, self->ssl, retval);
-        PySSL_END_ALLOW_THREADS
+        PySSL_END_ALLOW_THREADS(self)
         self->err = err;
 
         if (PyErr_CheckSignals())
@@ -2710,7 +2719,7 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
     }
 
     while (1) {
-        PySSL_BEGIN_ALLOW_THREADS
+        PySSL_BEGIN_ALLOW_THREADS(self)
         /* Disable read-ahead so that unwrap can work correctly.
          * Otherwise OpenSSL might read in too much data,
          * eating clear text data that happens to be
@@ -2723,7 +2732,7 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
             SSL_set_read_ahead(self->ssl, 0);
         ret = SSL_shutdown(self->ssl);
         err = _PySSL_errno(ret < 0, self->ssl, ret);
-        PySSL_END_ALLOW_THREADS
+        PySSL_END_ALLOW_THREADS(self)
         self->err = err;
 
         /* If err == 1, a secure shutdown with SSL_shutdown() is complete */
@@ -3115,9 +3124,10 @@ _ssl__SSLContext_impl(PyTypeObject *type, int proto_version)
     // no other thread can be touching this object yet.
     // (Technically, we can't even lock if we wanted to, as the
     // lock hasn't been initialized yet.)
-    PySSL_BEGIN_ALLOW_THREADS
+    Py_BEGIN_ALLOW_THREADS
     ctx = SSL_CTX_new(method);
-    PySSL_END_ALLOW_THREADS
+    Py_END_ALLOW_THREADS
+    _PySSL_FIX_ERRNO;
 
     if (ctx == NULL) {
         _setSSLError(get_ssl_state(module), NULL, 0, __FILE__, __LINE__);
@@ -3143,6 +3153,7 @@ _ssl__SSLContext_impl(PyTypeObject *type, int proto_version)
     self->psk_client_callback = NULL;
     self->psk_server_callback = NULL;
 #endif
+    self->tstate_mutex = (PyMutex){0};
 
     /* Don't check host name by default */
     if (proto_version == PY_SSL_VERSION_TLS_CLIENT) {
@@ -3259,9 +3270,10 @@ context_clear(PySSLContext *self)
     Py_CLEAR(self->psk_server_callback);
 #endif
     if (self->keylog_bio != NULL) {
-        PySSL_BEGIN_ALLOW_THREADS
+        Py_BEGIN_ALLOW_THREADS
         BIO_free_all(self->keylog_bio);
-        PySSL_END_ALLOW_THREADS
+        Py_END_ALLOW_THREADS
+        _PySSL_FIX_ERRNO;
         self->keylog_bio = NULL;
     }
     return 0;
@@ -3980,7 +3992,8 @@ _password_callback(char *buf, int size, int rwflag, void *userdata)
     _PySSLPasswordInfo *pw_info = (_PySSLPasswordInfo*) userdata;
     PyObject *fn_ret = NULL;
 
-    PySSL_END_ALLOW_THREADS_S(pw_info->thread_state);
+    pw_info->thread_state = PyThreadState_Swap(pw_info->thread_state);
+    _PySSL_FIX_ERRNO;
 
     if (pw_info->error) {
         /* already failed previously. OpenSSL 3.0.0-alpha14 invokes the
@@ -4010,13 +4023,13 @@ _password_callback(char *buf, int size, int rwflag, void *userdata)
         goto error;
     }
 
-    PySSL_BEGIN_ALLOW_THREADS_S(pw_info->thread_state);
+    pw_info->thread_state = PyThreadState_Swap(pw_info->thread_state);
     memcpy(buf, pw_info->password, pw_info->size);
     return pw_info->size;
 
 error:
     Py_XDECREF(fn_ret);
-    PySSL_BEGIN_ALLOW_THREADS_S(pw_info->thread_state);
+    pw_info->thread_state = PyThreadState_Swap(pw_info->thread_state);
     pw_info->error = 1;
     return -1;
 }
@@ -4069,10 +4082,10 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile,
         SSL_CTX_set_default_passwd_cb(self->ctx, _password_callback);
         SSL_CTX_set_default_passwd_cb_userdata(self->ctx, &pw_info);
     }
-    PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state);
+    PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
     r = SSL_CTX_use_certificate_chain_file(self->ctx,
         PyBytes_AS_STRING(certfile_bytes));
-    PySSL_END_ALLOW_THREADS_S(pw_info.thread_state);
+    PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
     if (r != 1) {
         if (pw_info.error) {
             ERR_clear_error();
@@ -4087,11 +4100,11 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile,
         }
         goto error;
     }
-    PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state);
+    PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
     r = SSL_CTX_use_PrivateKey_file(self->ctx,
         PyBytes_AS_STRING(keyfile ? keyfile_bytes : certfile_bytes),
         SSL_FILETYPE_PEM);
-    PySSL_END_ALLOW_THREADS_S(pw_info.thread_state);
+    PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
     Py_CLEAR(keyfile_bytes);
     Py_CLEAR(certfile_bytes);
     if (r != 1) {
@@ -4108,9 +4121,9 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile,
         }
         goto error;
     }
-    PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state);
+    PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
     r = SSL_CTX_check_private_key(self->ctx);
-    PySSL_END_ALLOW_THREADS_S(pw_info.thread_state);
+    PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
     if (r != 1) {
         _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
         goto error;
@@ -4327,9 +4340,9 @@ _ssl__SSLContext_load_verify_locations_impl(PySSLContext *self,
             cafile_buf = PyBytes_AS_STRING(cafile_bytes);
         if (capath)
             capath_buf = PyBytes_AS_STRING(capath_bytes);
-        PySSL_BEGIN_ALLOW_THREADS
+        PySSL_BEGIN_ALLOW_THREADS(self)
         r = SSL_CTX_load_verify_locations(self->ctx, cafile_buf, capath_buf);
-        PySSL_END_ALLOW_THREADS
+        PySSL_END_ALLOW_THREADS(self)
         if (r != 1) {
             if (errno != 0) {
                 PyErr_SetFromErrno(PyExc_OSError);
@@ -4381,10 +4394,11 @@ _ssl__SSLContext_load_dh_params_impl(PySSLContext *self, PyObject *filepath)
         return NULL;
 
     errno = 0;
-    PySSL_BEGIN_ALLOW_THREADS
+    Py_BEGIN_ALLOW_THREADS
     dh = PEM_read_DHparams(f, NULL, NULL, NULL);
     fclose(f);
-    PySSL_END_ALLOW_THREADS
+    Py_END_ALLOW_THREADS
+    _PySSL_FIX_ERRNO;
     if (dh == NULL) {
         if (errno != 0) {
             PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, filepath);
@@ -4536,6 +4550,7 @@ _ssl__SSLContext_set_default_verify_paths_impl(PySSLContext *self)
     Py_BEGIN_ALLOW_THREADS
     rc = SSL_CTX_set_default_verify_paths(self->ctx);
     Py_END_ALLOW_THREADS
+    _PySSL_FIX_ERRNO;
     if (!rc) {
         _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
         return NULL;

diff --git a/Modules/_ssl/debughelpers.c b/Modules/_ssl/debughelpers.c
index 5fc69a07184806060ef42670c7708c4191d1219f..fb8ae7c4e0bf4169e6a45711bebcd8f6e5dfe314 100644 (file)
--- a/Modules/_ssl/debughelpers.c
+++ b/Modules/_ssl/debughelpers.c
@@ -135,13 +135,15 @@ _PySSL_keylog_callback(const SSL *ssl, const char *line)
      * critical debug helper.
      */
 
-    PySSL_BEGIN_ALLOW_THREADS
+    assert(PyMutex_IsLocked(&ssl_obj->tstate_mutex));
+    Py_BEGIN_ALLOW_THREADS
     PyThread_acquire_lock(lock, 1);
     res = BIO_printf(ssl_obj->ctx->keylog_bio, "%s\n", line);
     e = errno;
     (void)BIO_flush(ssl_obj->ctx->keylog_bio);
     PyThread_release_lock(lock);
-    PySSL_END_ALLOW_THREADS
+    Py_END_ALLOW_THREADS
+    _PySSL_FIX_ERRNO;
 
     if (res == -1) {
         errno = e;
@@ -177,9 +179,10 @@ _PySSLContext_set_keylog_filename(PySSLContext *self, PyObject *arg, void *c) {
     if (self->keylog_bio != NULL) {
         BIO *bio = self->keylog_bio;
         self->keylog_bio = NULL;
-        PySSL_BEGIN_ALLOW_THREADS
+        Py_BEGIN_ALLOW_THREADS
         BIO_free_all(bio);
-        PySSL_END_ALLOW_THREADS
+        Py_END_ALLOW_THREADS
+        _PySSL_FIX_ERRNO;
     }
 
     if (arg == Py_None) {
@@ -201,13 +204,13 @@ _PySSLContext_set_keylog_filename(PySSLContext *self, PyObject *arg, void *c) {
     self->keylog_filename = Py_NewRef(arg);
 
     /* Write a header for seekable, empty files (this excludes pipes). */
-    PySSL_BEGIN_ALLOW_THREADS
+    PySSL_BEGIN_ALLOW_THREADS(self)
     if (BIO_tell(self->keylog_bio) == 0) {
         BIO_puts(self->keylog_bio,
                  "# TLS secrets log file, generated by OpenSSL / Python\n");
         (void)BIO_flush(self->keylog_bio);
     }
-    PySSL_END_ALLOW_THREADS
+    PySSL_END_ALLOW_THREADS(self)
     SSL_CTX_set_keylog_callback(self->ctx, _PySSL_keylog_callback);
     return 0;
 }