Repository navigation
Data races in OpenSSL bindings #143756
Description
Activity
load_cert_chainlooks fixable. We should acquiretstate_mutex(viaPySSL_BEGIN_ALLOW_THREADS) around the SSL calls.For
test_thread_recv_while_main_thread_sends, you might be interested in the history here -- specifically #137583. In short, I tried to fix these races a while ago by adding locking around OpenSSL calls, but that ended up deadlocking libraries that attempted to write to a socket while another thread was reading from it. I can't think of any way to fix that without running into that problem again.Thanks for the pointers. I think that locking around the read and write calls is probably the right way to go. python-websockets will need to change their test code, but I think their code is currently broken. That's not the kind of change we should do in a bugfix release, but I think it's appropriate in a major release. cc @gpshead for his thoughts.
Separately, I don't think merging
Py_BEGIN_ALLOW_THREADSwith the mutex acquisition in SSL code makes sense. The mutex should be held across all of the OpenSSL calls in an operation, whilePy_BEGIN/END_ALLOW_THREADSis only for the blocking calls.I believe @aaugustin expressed the concern that it would be very difficult to implement websockets without the concurrent read/write problem.
The mutex should be held across all of the OpenSSL calls in an operation, while Py_BEGIN/END_ALLOW_THREADS is only for the blocking calls.
Yeah, you're right. I think my original impression was that OpenSSL calls are only done under
Py_BEGIN_ALLOW_THREADSblocks, but that doesn't look correct in hindsight.I wonder if we can configure the OpenSSL BIO's to always be nonblocking and rely on
PySSL_selectfor blocking. That way we could hold the mutex across theSSL_read_exandSSL_write_excalls, but not thePySSL_selectcalls.Reacted by Rudolf Polzerpython-websockets will need to change their test code
It is not a test problem, it's a production problem.
Currently, it's possible to have a thread send a message (
websocket.send(msg)) while another thread blocked waiting for the next message (msg = websocket.recv()). This translates directly to the same operations on a blocking socket — blocking on reads. I expect any other full-duplex protocol implemented with blocking reads to have the same problem.What would be the code change if that becomes impossible?
For a single connection, I guess it would just be a matter of replacing a blocking
recvby a non-blockingselectfollowed by a non-blockingrecv?For a server managing several connections, I suppose the best way is to
select(or equivalent) all connections rather than once per connection, but that's a more significant change that doesn't fit well into websockets' architecture.It is not a test problem, it's a production problem.
Thanks for the clarification.
What would be the code change if that becomes impossible? For a single connection, I guess it would just be a matter of replacing a blocking recv by a non-blocking select followed by a non-blocking recv?
You shouldn't need to change any code. My current plan is for CPython to basically do the thing you described. We'll internally configure the socket to be non-blocking so that the OpenSSL
SSL_read_ex/SSL_write_excalls are non-blocking. The SSLSocketrecv()/send()calls will still block (depending onsettimeout()andsetblocking()) usingselect/poll.We only need the lock around the
SSL_read_ex/SSL_write_excalls, notselect/poll.I don't think it'll be a big change -- this is already how
settimeout()is implemented -- although I'm a little worried about implementation details leaking through something likesocket.fileno().My current plan is for CPython to basically do the thing you described.
That would be amazing 🤩
- marked SSLSocket.recv/SSLSocket.send concurrency crash #151508 as a duplicate of this issue
on Jun 15, 2026 Just as an example, a repro to get a crash out of the
SSL_read/SSL_writedata race is here, tsan or asan not needed:import os import socket import ssl import threading import time os.system("openssl req -x509 -newkey rsa:2048 -keyout key.pem -out cert.pem -nodes -subj /CN=localhost") def ssl_loop(func, stop_me): while not stop_me.is_set(): try: func() except ssl.SSLError as e: if e.errno in (ssl.SSL_ERROR_WANT_READ, ssl.SSL_ERROR_WANT_WRITE): continue break except Exception: break def run_ssl_server(server_sock, stop_event): context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) context.load_cert_chain(certfile="cert.pem", keyfile="key.pem") conn, _ = server_sock.accept() ssl_conn = context.wrap_socket(conn, server_side=True) def server_step(): c.sendall(b"S" * 4096) c.recv(4096) ssl_loop(server_step, stop_event) def try_to_crash(): server_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) server_sock.bind(('127.0.0.1', 0)) server_sock.listen(5) stop_event = threading.Event() threading.Thread(target=run_ssl_server, args=(server_sock, stop_event), daemon=True).start() client_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) client_sock.connect(server_sock.getsockname()) context = ssl.create_default_context(ssl.Purpose.SERVER_AUTH) context.check_hostname = False context.verify_mode = ssl.CERT_NONE ssl_client = context.wrap_socket(client_sock) ssl_client.settimeout(0.001) # Can just retry if a short timeout is hit. # Spawn 10 reader and writer threads against the same SSL client, # and let them run for one second. threads = [] for _ in range(10): t_read = threading.Thread(target=lambda: ssl_loop(lambda: ssl_client.recv(4096), stop_event)) t_write = threading.Thread(target=lambda: ssl_loop(lambda: ssl_client.sendall(b"Y" * 4096), stop_event)) threads.extend([t_read, t_write]) t_read.start() t_write.start() time.sleep(1) stop_event.set() for t in threads: t.join(timeout=0.1) ssl_client.close() server_sock.close() for i in range(100): print(i) try_to_crash() print('did not crash')crashing with heap corruption:
* thread #9, name = 'python3', stop reason = signal SIGSEGV: sent by kernel (SI_KERNEL) * frame #0: 0x00007ffff7ca497d libc.so.6`_int_malloc(av=0x00007fffc0000030, bytes=21) at malloc.c:4217:14 frame #1: 0x00007ffff7ca57f2 libc.so.6`__libc_malloc2(bytes=21) at malloc.c:3458:12 frame #2: 0x00007ffff6a613ee libcrypto.so.3`CRYPTO_malloc at mem.c:214:11 frame #3: 0x00007ffff6a06eb5 libcrypto.so.3`err_set_debug(es=0x00007fffc0005f60, i=1, file="../ssl/record/methods/tls_common.c", line=873, fn="tls_get_more_records") at err_local.h:69:33 [inlined] frame #4: 0x00007ffff6a06e70 libcrypto.so.3`ERR_set_debug(file="../ssl/record/methods/tls_common.c", line=873, func="tls_get_more_records") at err_blocks.c:37:5 frame #5: 0x00007ffff70dfcf9 libssl.so.3`tls_get_more_records at tls_common.c:921:9 frame #6: 0x00007ffff70de29a libssl.so.3`tls_read_record at tls_common.c:1141:15 frame #7: 0x00007ffff70d6535 libssl.so.3`ssl3_read_bytes at rec_layer_s3.c:698:19 frame #8: 0x00007ffff706ac40 libssl.so.3`ssl3_read_internal.part.0 at s3_lib.c:5131:11 frame #9: 0x00007ffff7079f1d libssl.so.3`SSL_read_ex at ssl_lib.c:2388:15 frame #10: 0x00007ffff71b0c37 _ssl.cpython-313-x86_64-linux-gnu.so`_ssl__SSLSocket_read_impl(self=0x00007ffff674c6a0, len=4096, group_right_1=<unavailable>, buffer=0x00007fffda7fbaa0) at _ssl.c:2622:18 frame #11: 0x00007ffff71b0b51 _ssl.cpython-313-x86_64-linux-gnu.so`_ssl__SSLSocket_read(self=0x00007ffff674c6a0, args=<unavailable>) at _ssl.c.h:544:20 frame #12: 0x0000000000560d03 python3`method_vectorcall_VARARGS(func=<unavailable>, args=<unavailable>, nargsf=<unavailable>, kwnames=<unavailable>) at descrobject.c:324:24 frame #13: 0x0000000000559723 python3`_PyObject_VectorcallTstate(tstate=0x0000000000e248c0, callable=0x00007ffff727cc70, args=<unavailable>, nargsf=<unavailable>, kwnames=<unavailable>) at pycore_call.h:168:11 [inlined] frame #14: 0x0000000000559708 python3`PyObject_Vectorcall(callable=0x00007ffff727cc70, args=<unavailable>, nargsf=<unavailable>, kwnames=<unavailable>) at call.c:327:12 frame #15: 0x000000000056ea15 python3`_PyEval_EvalFrameDefault(tstate=<unavailable>, frame=<unavailable>, throwflag=<unavailable>) at generated_cases.c.h:1850:23 frame #16: 0x00000000005dd5ea python3`_PyObject_VectorcallTstate(tstate=0x0000000000e248c0, callable=0x00007ffff6755da0, args=0x00007fffda7fbdd8, nargsf=1, kwnames=0x0000000000000000) at pycore_call.h:168:11 [inlined] frame #17: 0x00000000005dd5b7 python3`method_vectorcall(method=<unavailable>, args=<unavailable>, nargsf=<unavailable>, kwnames=<unavailable>) at classobject.c:71:20 frame #18: 0x00000000006f7c10 python3`thread_run(boot_raw=0x0000000000d47a60) at _threadmodule.c:343:21 frame #19: 0x000000000069f908 python3`pythread_wrapper(arg=<unavailable>) at thread_pthread.h:242:5 frame #20: 0x00007ffff7c95dc9 libc.so.6`start_thread(arg=<unavailable>) at pthread_create.c:448:8 frame #21: 0x00007ffff7d14d88 libc.so.6`__clone3 at clone3.S:78As such, this probably should be fixed - I guess the idea above to basically do the
SSL_read/SSL_writecalls under the mutex, but nonblocking, instead waiting prior to them withselect, should work - with the one caveat thatselectindicating there are bytes may not imply theSSL_readwill actually read something, but it may just update internal state (so if the contract to the caller is that at least 1 byte must be read, it will have to loop).Be aware that the mutex probably should be held up until the
SSL_get_errorcall to ensure other threads can't interfere with the error value returned; so basically keep it with the `_PySSL_errno as a block, like it looks now.One thing worth putting on the record before a fix is written, because it rules out an approach that looks obvious:
@critical_sectionprovides no mutual exclusion across a GIL-released region.Python/pystate.c:2323, in the detach path:if (tstate->critical_section != 0) { _PyCriticalSection_SuspendAll(tstate); }
So any critical section held on entry is suspended for the duration of a
Py_BEGIN_ALLOW_THREADS/PySSL_BEGIN_ALLOW_THREADSblock, and re-acquired on reattach. For_sslthat means the 88@critical_sectionclinic directives in the module do not protect theSSL_*call they wrap — only the Python-level bookkeeping on either side of it.Demonstrated without a sanitizer: two threads driven through a barrier end up simultaneously inside one
SSL*, with OpenSSL reporting its owninternal error.This matches the direction the thread is already going — the mutex has to be held across the OpenSSL calls, and
Py_BEGIN/END_ALLOW_THREADSis only for the blocking ones — but it is worth being explicit that a fix expressed as "add@critical_sectionto these methods" would compile, look right, and change nothing.Same point applies to gh-150191.
Written with AI assistance; the
pystate.ccoordinate was read at4f3be1b5777and the two-thread demonstration was run.- added a commit that references this issue
on Oct 12, 2026
Bug report
We have some data races in our OpenSSL bindings that we were not previously detecting because we didn't compile OpenSSL with TSan (#143750):
test_load_cert_chain_thread_safety
Race between
use_certificate_chain_fileandSSL_CTX_set_default_passwd_cb:test_thread_recv_while_main_thread_sends
Linked PRs