From 89d0b197f130ca7ea4d287d5203142ffc6a1beff Mon Sep 17 00:00:00 2001 From: Kirill Ignatev Date: Mon, 18 May 2026 12:03:32 -0700 Subject: [PATCH 01/11] Add test_sni_callback_race --- Lib/test/test_ssl.py | 50 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index f1f7a07701de16..926b9f88a9ef96 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -1606,6 +1606,56 @@ def dummycallback(sock, servername, ctx, cycle=ctx): gc.collect() self.assertIs(wr(), None) + @unittest.skipUnless(support.Py_GIL_DISABLED, + "test is only useful if the GIL is disabled") + @threading_helper.requires_working_threading() + def test_sni_callback_race(self): + # Replacing sni_callback while handshakes are in-flight must not + # crash (use-after-free on the callback in free-threaded builds). + client_ctx, server_ctx, hostname = testing_context() + server_ctx.sni_callback = lambda *a: None + done = threading.Event() + + def do_handshakes(): + for _ in range(100): + c_in = ssl.MemoryBIO() + c_out = ssl.MemoryBIO() + s_in = ssl.MemoryBIO() + s_out = ssl.MemoryBIO() + client = client_ctx.wrap_bio( + c_in, c_out, server_hostname=hostname) + server = server_ctx.wrap_bio(s_in, s_out, server_side=True) + for _ in range(50): + try: + client.do_handshake() + except ssl.SSLWantReadError: + pass + except ssl.SSLError: + break + if c_out.pending: + s_in.write(c_out.read()) + try: + server.do_handshake() + except ssl.SSLWantReadError: + pass + except ssl.SSLError: + break + if s_out.pending: + c_in.write(s_out.read()) + + def toggle_callback(): + while not done.is_set(): + server_ctx.sni_callback = lambda *a: None + server_ctx.sni_callback = None + + threads = [threading.Thread(target=do_handshakes) for _ in range(4)] + threads.append(threading.Thread(target=toggle_callback)) + + with threading_helper.catch_threading_exception() as cm: + with threading_helper.start_threads(threads, done.set): + pass + self.assertIsNone(cm.exc_value) + def test_cert_store_stats(self): ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) self.assertEqual(ctx.cert_store_stats(), From be11d8b93ee5f84fd4afd7323397b39076f86c46 Mon Sep 17 00:00:00 2001 From: Kirill Ignatev Date: Mon, 18 May 2026 12:41:47 -0700 Subject: [PATCH 02/11] Improve test_sni_callback_race --- Lib/test/test_ssl.py | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 926b9f88a9ef96..03ca4f5224b2d2 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -1613,11 +1613,19 @@ def test_sni_callback_race(self): # Replacing sni_callback while handshakes are in-flight must not # crash (use-after-free on the callback in free-threaded builds). client_ctx, server_ctx, hostname = testing_context() - server_ctx.sni_callback = lambda *a: None + + def make_callback(n): + def sni_cb(_ssl_obj, _servername, _ctx): + if n == -1 and _servername == "": + raise AssertionError("unreachable") + return None + return sni_cb + + server_ctx.sni_callback = make_callback(0) done = threading.Event() def do_handshakes(): - for _ in range(100): + while not done.is_set(): c_in = ssl.MemoryBIO() c_out = ssl.MemoryBIO() s_in = ssl.MemoryBIO() @@ -1644,16 +1652,21 @@ def do_handshakes(): c_in.write(s_out.read()) def toggle_callback(): + i = 0 while not done.is_set(): - server_ctx.sni_callback = lambda *a: None + server_ctx.sni_callback = make_callback(i) server_ctx.sni_callback = None + server_ctx.sni_callback = make_callback(-i) + i += 1 - threads = [threading.Thread(target=do_handshakes) for _ in range(4)] + workers = max(4, (os.cpu_count() or 4) * 2) + threads = [threading.Thread(target=do_handshakes) + for _ in range(workers)] threads.append(threading.Thread(target=toggle_callback)) with threading_helper.catch_threading_exception() as cm: - with threading_helper.start_threads(threads, done.set): - pass + with threading_helper.start_threads(threads): + done.set() self.assertIsNone(cm.exc_value) def test_cert_store_stats(self): From 63d7bab79c15ccc1c0c4fee1bd8feda81779804d Mon Sep 17 00:00:00 2001 From: Kirill Ignatev Date: Mon, 18 May 2026 15:26:41 -0700 Subject: [PATCH 03/11] Fix SNI callback callable race use-after-free In Modules/_ssl.c --- Modules/_ssl.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/Modules/_ssl.c b/Modules/_ssl.c index 3224ca7d0f93b9..dad1f8256569b5 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -26,7 +26,9 @@ #define OPENSSL_NO_DEPRECATED 1 #include "Python.h" +#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION() #include "pycore_fileutils.h" // _PyIsSelectable_fd() +#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_PTR_RELAXED() #include "pycore_long.h" // _PyLong_UnsignedLongLong_Converter() #include "pycore_pyerrors.h" // _PyErr_ChainExceptions1() #include "pycore_time.h" // _PyDeadline_Init() @@ -5153,12 +5155,15 @@ _servername_callback(SSL *s, int *al, void *args) PyObject *result; /* The high-level ssl.SSLSocket object */ PyObject *ssl_socket; + PyObject *sni_cb; const char *servername = SSL_get_servername(s, TLSEXT_NAMETYPE_host_name); PyGILState_STATE gstate = PyGILState_Ensure(); - if (sslctx->set_sni_cb == NULL) { - /* remove race condition in this the call back while if removing the - * callback is in progress */ + Py_BEGIN_CRITICAL_SECTION(sslctx); + sni_cb = Py_XNewRef(FT_ATOMIC_LOAD_PTR_RELAXED(sslctx->set_sni_cb)); + Py_END_CRITICAL_SECTION(); + + if (sni_cb == NULL) { PyGILState_Release(gstate); return SSL_TLSEXT_ERR_OK; } @@ -5185,7 +5190,7 @@ _servername_callback(SSL *s, int *al, void *args) goto error; if (servername == NULL) { - result = PyObject_CallFunctionObjArgs(sslctx->set_sni_cb, ssl_socket, + result = PyObject_CallFunctionObjArgs(sni_cb, ssl_socket, Py_None, sslctx, NULL); } else { @@ -5212,7 +5217,7 @@ _servername_callback(SSL *s, int *al, void *args) } Py_DECREF(servername_bytes); result = PyObject_CallFunctionObjArgs( - sslctx->set_sni_cb, ssl_socket, servername_str, + sni_cb, ssl_socket, servername_str, sslctx, NULL); Py_DECREF(servername_str); } @@ -5222,7 +5227,7 @@ _servername_callback(SSL *s, int *al, void *args) PyErr_FormatUnraisable("Exception ignored " "in ssl servername callback " "while calling set SNI callback %R", - sslctx->set_sni_cb); + sni_cb); *al = SSL_AD_HANDSHAKE_FAILURE; ret = SSL_TLSEXT_ERR_ALERT_FATAL; } @@ -5247,11 +5252,13 @@ _servername_callback(SSL *s, int *al, void *args) Py_DECREF(result); } + Py_DECREF(sni_cb); PyGILState_Release(gstate); return ret; error: Py_XDECREF(ssl_socket); + Py_DECREF(sni_cb); *al = SSL_AD_INTERNAL_ERROR; ret = SSL_TLSEXT_ERR_ALERT_FATAL; PyGILState_Release(gstate); @@ -5277,7 +5284,7 @@ static PyObject * _ssl__SSLContext_sni_callback_get_impl(PySSLContext *self) /*[clinic end generated code: output=961e6575cdfaf036 input=3aee06696b0874d9]*/ { - PyObject *cb = self->set_sni_cb; + PyObject *cb = FT_ATOMIC_LOAD_PTR_RELAXED(self->set_sni_cb); if (cb == NULL) { Py_RETURN_NONE; } @@ -5312,7 +5319,7 @@ _ssl__SSLContext_sni_callback_set_impl(PySSLContext *self, PyObject *value) "not a callable object"); return -1; } - self->set_sni_cb = Py_NewRef(value); + FT_ATOMIC_STORE_PTR_RELAXED(self->set_sni_cb, Py_NewRef(value)); SSL_CTX_set_tlsext_servername_callback(self->ctx, _servername_callback); SSL_CTX_set_tlsext_servername_arg(self->ctx, self); } From 1920fcb967337d300bc7d329c0e5dae8876efe71 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Mon, 18 May 2026 22:45:55 +0000 Subject: [PATCH 04/11] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2026-05-18-22-45-54.gh-issue-149816.T68vc_.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2026-05-18-22-45-54.gh-issue-149816.T68vc_.rst diff --git a/Misc/NEWS.d/next/Library/2026-05-18-22-45-54.gh-issue-149816.T68vc_.rst b/Misc/NEWS.d/next/Library/2026-05-18-22-45-54.gh-issue-149816.T68vc_.rst new file mode 100644 index 00000000000000..9996cc7ec0e866 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-05-18-22-45-54.gh-issue-149816.T68vc_.rst @@ -0,0 +1 @@ +Fix race condition in :attr:`ssl.SSLContext.sni_callback` From fb02f9df3851a4d4f5fcfb0d44665e994fb4767e Mon Sep 17 00:00:00 2001 From: Kirill Ignatev Date: Mon, 18 May 2026 16:00:42 -0700 Subject: [PATCH 05/11] Reorder imports --- Modules/_ssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_ssl.c b/Modules/_ssl.c index dad1f8256569b5..300381750c162b 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -28,8 +28,8 @@ #include "Python.h" #include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION() #include "pycore_fileutils.h" // _PyIsSelectable_fd() -#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_PTR_RELAXED() #include "pycore_long.h" // _PyLong_UnsignedLongLong_Converter() +#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_PTR_RELAXED() #include "pycore_pyerrors.h" // _PyErr_ChainExceptions1() #include "pycore_time.h" // _PyDeadline_Init() #include "pycore_tuple.h" // _PyTuple_FromPair From dd20d000a5903f6e6fe92a18301a9185bd2a00cb Mon Sep 17 00:00:00 2001 From: Kirill Ignatev Date: Mon, 18 May 2026 16:00:53 -0700 Subject: [PATCH 06/11] Py_XDECREF in error case --- Modules/_ssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_ssl.c b/Modules/_ssl.c index 300381750c162b..5b36530e272e7f 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -5258,7 +5258,7 @@ _servername_callback(SSL *s, int *al, void *args) error: Py_XDECREF(ssl_socket); - Py_DECREF(sni_cb); + Py_XDECREF(sni_cb); *al = SSL_AD_INTERNAL_ERROR; ret = SSL_TLSEXT_ERR_ALERT_FATAL; PyGILState_Release(gstate); From 1548b6e900762d2110be0f530602827882bde538 Mon Sep 17 00:00:00 2001 From: Kirill Ignatev Date: Mon, 18 May 2026 16:23:08 -0700 Subject: [PATCH 07/11] Remove callback before Py_CLEAR --- Modules/_ssl.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Modules/_ssl.c b/Modules/_ssl.c index 5b36530e272e7f..3c9d062f4dfbd2 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -5308,13 +5308,10 @@ _ssl__SSLContext_sni_callback_set_impl(PySSLContext *self, PyObject *value) "sni_callback cannot be set on TLS_CLIENT context"); return -1; } + SSL_CTX_set_tlsext_servername_callback(self->ctx, NULL); Py_CLEAR(self->set_sni_cb); - if (value == Py_None) { - SSL_CTX_set_tlsext_servername_callback(self->ctx, NULL); - } - else { + if (value != Py_None) { if (!PyCallable_Check(value)) { - SSL_CTX_set_tlsext_servername_callback(self->ctx, NULL); PyErr_SetString(PyExc_TypeError, "not a callable object"); return -1; From 67da443971599f7bc89f0f3190e0db3783c81af2 Mon Sep 17 00:00:00 2001 From: Kirill Ignatev Date: Mon, 18 May 2026 16:28:20 -0700 Subject: [PATCH 08/11] Remove FT_ATOMIC_LOAD_PTR_RELAXED --- Modules/_ssl.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Modules/_ssl.c b/Modules/_ssl.c index 3c9d062f4dfbd2..3e4982e666580e 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -29,7 +29,6 @@ #include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION() #include "pycore_fileutils.h" // _PyIsSelectable_fd() #include "pycore_long.h" // _PyLong_UnsignedLongLong_Converter() -#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_PTR_RELAXED() #include "pycore_pyerrors.h" // _PyErr_ChainExceptions1() #include "pycore_time.h" // _PyDeadline_Init() #include "pycore_tuple.h" // _PyTuple_FromPair @@ -5160,7 +5159,7 @@ _servername_callback(SSL *s, int *al, void *args) PyGILState_STATE gstate = PyGILState_Ensure(); Py_BEGIN_CRITICAL_SECTION(sslctx); - sni_cb = Py_XNewRef(FT_ATOMIC_LOAD_PTR_RELAXED(sslctx->set_sni_cb)); + sni_cb = Py_XNewRef(sslctx->set_sni_cb); Py_END_CRITICAL_SECTION(); if (sni_cb == NULL) { @@ -5284,7 +5283,7 @@ static PyObject * _ssl__SSLContext_sni_callback_get_impl(PySSLContext *self) /*[clinic end generated code: output=961e6575cdfaf036 input=3aee06696b0874d9]*/ { - PyObject *cb = FT_ATOMIC_LOAD_PTR_RELAXED(self->set_sni_cb); + PyObject *cb = self->set_sni_cb; if (cb == NULL) { Py_RETURN_NONE; } @@ -5316,7 +5315,7 @@ _ssl__SSLContext_sni_callback_set_impl(PySSLContext *self, PyObject *value) "not a callable object"); return -1; } - FT_ATOMIC_STORE_PTR_RELAXED(self->set_sni_cb, Py_NewRef(value)); + self->set_sni_cb = Py_NewRef(value); SSL_CTX_set_tlsext_servername_callback(self->ctx, _servername_callback); SSL_CTX_set_tlsext_servername_arg(self->ctx, self); } From 1ecc7df393f55354dd90a241dd09ee6256f94f5e Mon Sep 17 00:00:00 2001 From: Kirill Ignatev Date: Mon, 18 May 2026 17:10:32 -0700 Subject: [PATCH 09/11] Don't allow callback == NULL --- Modules/_ssl.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/Modules/_ssl.c b/Modules/_ssl.c index 3e4982e666580e..c0d0e4a9c79b55 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -5307,17 +5307,20 @@ _ssl__SSLContext_sni_callback_set_impl(PySSLContext *self, PyObject *value) "sni_callback cannot be set on TLS_CLIENT context"); return -1; } - SSL_CTX_set_tlsext_servername_callback(self->ctx, NULL); - Py_CLEAR(self->set_sni_cb); - if (value != Py_None) { - if (!PyCallable_Check(value)) { - PyErr_SetString(PyExc_TypeError, - "not a callable object"); + if (!PyCallable_Check(value)) { + SSL_CTX_set_tlsext_servername_callback(self->ctx, NULL); + Py_CLEAR(self->set_sni_cb); + if (value != Py_None) { + PyErr_SetString(PyExc_TypeError, "not a callable object"); return -1; } - self->set_sni_cb = Py_NewRef(value); - SSL_CTX_set_tlsext_servername_callback(self->ctx, _servername_callback); + } + else { + Py_INCREF(value); + PyObject *old_cb = _Py_atomic_exchange_ptr(&self->set_sni_cb, value); + Py_XDECREF(old_cb); SSL_CTX_set_tlsext_servername_arg(self->ctx, self); + SSL_CTX_set_tlsext_servername_callback(self->ctx, _servername_callback); } return 0; } From 32e5ba4c2b70a7d8b3b7839977161e81fc0f504c Mon Sep 17 00:00:00 2001 From: Kirill Ignatev Date: Mon, 18 May 2026 18:31:58 -0700 Subject: [PATCH 10/11] Simplify test --- Lib/test/test_ssl.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index 03ca4f5224b2d2..7f237276617152 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -1614,14 +1614,7 @@ def test_sni_callback_race(self): # crash (use-after-free on the callback in free-threaded builds). client_ctx, server_ctx, hostname = testing_context() - def make_callback(n): - def sni_cb(_ssl_obj, _servername, _ctx): - if n == -1 and _servername == "": - raise AssertionError("unreachable") - return None - return sni_cb - - server_ctx.sni_callback = make_callback(0) + server_ctx.sni_callback = lambda *a: None done = threading.Event() def do_handshakes(): @@ -1652,12 +1645,9 @@ def do_handshakes(): c_in.write(s_out.read()) def toggle_callback(): - i = 0 while not done.is_set(): - server_ctx.sni_callback = make_callback(i) + server_ctx.sni_callback = lambda *a: None server_ctx.sni_callback = None - server_ctx.sni_callback = make_callback(-i) - i += 1 workers = max(4, (os.cpu_count() or 4) * 2) threads = [threading.Thread(target=do_handshakes) From e1f1f4ad387acb97fa783901c26fc54e686b2216 Mon Sep 17 00:00:00 2001 From: Kirill Ignatev Date: Mon, 18 May 2026 18:33:04 -0700 Subject: [PATCH 11/11] Simplify exchange in critical section --- Modules/_ssl.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Modules/_ssl.c b/Modules/_ssl.c index c0d0e4a9c79b55..35754e566a1528 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -5316,9 +5316,7 @@ _ssl__SSLContext_sni_callback_set_impl(PySSLContext *self, PyObject *value) } } else { - Py_INCREF(value); - PyObject *old_cb = _Py_atomic_exchange_ptr(&self->set_sni_cb, value); - Py_XDECREF(old_cb); + Py_XSETREF(self->set_sni_cb, Py_NewRef(value)); SSL_CTX_set_tlsext_servername_arg(self->ctx, self); SSL_CTX_set_tlsext_servername_callback(self->ctx, _servername_callback); }