Skip to content

Commit 4ffb568

Browse files
ZeroIntensityZeroIntensity
committed
Lock when the thread state is detached.
1 parent 965662e commit 4ffb568

2 files changed

Lines changed: 61 additions & 48 deletions

File tree

‎Modules/_ssl.c‎

Lines changed: 55 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,14 @@
4343
/* Redefined below for Windows debug builds after important #includes */
4444
#define _PySSL_FIX_ERRNO
4545

46-
#define PySSL_BEGIN_ALLOW_THREADS_S(save) \
47-
do { (save) = PyEval_SaveThread(); } while(0)
48-
#define PySSL_END_ALLOW_THREADS_S(save) \
49-
do { PyEval_RestoreThread(save); _PySSL_FIX_ERRNO; } while(0)
50-
#define PySSL_BEGIN_ALLOW_THREADS { \
46+
#define PySSL_BEGIN_ALLOW_THREADS_S(save, mutex) \
47+
do { (save) = PyEval_SaveThread(); PyMutex_Lock(mutex); } while(0)
48+
#define PySSL_END_ALLOW_THREADS_S(save, mutex) \
49+
do { PyMutex_Unlock(mutex); PyEval_RestoreThread(save); _PySSL_FIX_ERRNO; } while(0)
50+
#define PySSL_BEGIN_ALLOW_THREADS(self) { \
5151
PyThreadState *_save = NULL; \
52-
PySSL_BEGIN_ALLOW_THREADS_S(_save);
53-
#define PySSL_END_ALLOW_THREADS PySSL_END_ALLOW_THREADS_S(_save); }
52+
PySSL_BEGIN_ALLOW_THREADS_S(_save, &self->tstate_mutex);
53+
#define PySSL_END_ALLOW_THREADS(self) PySSL_END_ALLOW_THREADS_S(_save, &self->tstate_mutex); }
5454

5555
#if defined(HAVE_POLL_H)
5656
#include <poll.h>
@@ -309,6 +309,9 @@ typedef struct {
309309
PyObject *psk_client_callback;
310310
PyObject *psk_server_callback;
311311
#endif
312+
/* Lock to synchronize calls when the thread state is detached.
313+
See also gh-134698. */
314+
PyMutex tstate_mutex;
312315
} PySSLContext;
313316

314317
#define PySSLContext_CAST(op) ((PySSLContext *)(op))
@@ -336,6 +339,9 @@ typedef struct {
336339
* and shutdown methods check for chained exceptions.
337340
*/
338341
PyObject *exc;
342+
/* Lock to synchronize calls when the thread state is detached.
343+
See also gh-134698. */
344+
PyMutex tstate_mutex;
339345
} PySSLSocket;
340346

341347
#define PySSLSocket_CAST(op) ((PySSLSocket *)(op))
@@ -889,9 +895,9 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
889895
/* Make sure the SSL error state is initialized */
890896
ERR_clear_error();
891897

892-
PySSL_BEGIN_ALLOW_THREADS
898+
PySSL_BEGIN_ALLOW_THREADS(self)
893899
self->ssl = SSL_new(ctx);
894-
PySSL_END_ALLOW_THREADS
900+
PySSL_END_ALLOW_THREADS(self)
895901
if (self->ssl == NULL) {
896902
Py_DECREF(self);
897903
_setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
@@ -960,12 +966,12 @@ newPySSLSocket(PySSLContext *sslctx, PySocketSockObject *sock,
960966
BIO_set_nbio(SSL_get_wbio(self->ssl), 1);
961967
}
962968

963-
PySSL_BEGIN_ALLOW_THREADS
969+
PySSL_BEGIN_ALLOW_THREADS(self)
964970
if (socket_type == PY_SSL_CLIENT)
965971
SSL_set_connect_state(self->ssl);
966972
else
967973
SSL_set_accept_state(self->ssl);
968-
PySSL_END_ALLOW_THREADS
974+
PySSL_END_ALLOW_THREADS(self)
969975

970976
self->socket_type = socket_type;
971977
if (sock != NULL) {
@@ -1034,10 +1040,10 @@ _ssl__SSLSocket_do_handshake_impl(PySSLSocket *self)
10341040
/* Actually negotiate SSL connection */
10351041
/* XXX If SSL_do_handshake() returns 0, it's also a failure. */
10361042
do {
1037-
PySSL_BEGIN_ALLOW_THREADS
1043+
PySSL_BEGIN_ALLOW_THREADS(self)
10381044
ret = SSL_do_handshake(self->ssl);
10391045
err = _PySSL_errno(ret < 1, self->ssl, ret);
1040-
PySSL_END_ALLOW_THREADS
1046+
PySSL_END_ALLOW_THREADS(self)
10411047
self->err = err;
10421048

10431049
if (PyErr_CheckSignals())
@@ -2414,9 +2420,10 @@ PySSL_select(PySocketSockObject *s, int writing, PyTime_t timeout)
24142420
ms = (int)_PyTime_AsMilliseconds(timeout, _PyTime_ROUND_CEILING);
24152421
assert(ms <= INT_MAX);
24162422

2417-
PySSL_BEGIN_ALLOW_THREADS
2423+
Py_BEGIN_ALLOW_THREADS
24182424
rc = poll(&pollfd, 1, (int)ms);
2419-
PySSL_END_ALLOW_THREADS
2425+
Py_END_ALLOW_THREADS
2426+
_PySSL_FIX_ERRNO;
24202427
#else
24212428
/* Guard against socket too large for select*/
24222429
if (!_PyIsSelectable_fd(s->sock_fd))
@@ -2428,13 +2435,14 @@ PySSL_select(PySocketSockObject *s, int writing, PyTime_t timeout)
24282435
FD_SET(s->sock_fd, &fds);
24292436

24302437
/* Wait until the socket becomes ready */
2431-
PySSL_BEGIN_ALLOW_THREADS
2438+
Py_BEGIN_ALLOW_THREADS
24322439
nfds = Py_SAFE_DOWNCAST(s->sock_fd+1, SOCKET_T, int);
24332440
if (writing)
24342441
rc = select(nfds, NULL, &fds, NULL, &tv);
24352442
else
24362443
rc = select(nfds, &fds, NULL, NULL, &tv);
2437-
PySSL_END_ALLOW_THREADS
2444+
Py_END_ALLOW_THREADS
2445+
_PySSL_FIX_ERRNO;
24382446
#endif
24392447

24402448
/* Return SOCKET_TIMED_OUT on timeout, SOCKET_OPERATION_OK otherwise
@@ -2505,10 +2513,10 @@ _ssl__SSLSocket_write_impl(PySSLSocket *self, Py_buffer *b)
25052513
}
25062514

25072515
do {
2508-
PySSL_BEGIN_ALLOW_THREADS
2516+
PySSL_BEGIN_ALLOW_THREADS(self)
25092517
retval = SSL_write_ex(self->ssl, b->buf, (size_t)b->len, &count);
25102518
err = _PySSL_errno(retval == 0, self->ssl, retval);
2511-
PySSL_END_ALLOW_THREADS
2519+
PySSL_END_ALLOW_THREADS(self)
25122520
self->err = err;
25132521

25142522
if (PyErr_CheckSignals())
@@ -2566,10 +2574,10 @@ _ssl__SSLSocket_pending_impl(PySSLSocket *self)
25662574
int count = 0;
25672575
_PySSLError err;
25682576

2569-
PySSL_BEGIN_ALLOW_THREADS
2577+
PySSL_BEGIN_ALLOW_THREADS(self)
25702578
count = SSL_pending(self->ssl);
25712579
err = _PySSL_errno(count < 0, self->ssl, count);
2572-
PySSL_END_ALLOW_THREADS
2580+
PySSL_END_ALLOW_THREADS(self)
25732581
self->err = err;
25742582

25752583
if (count < 0)
@@ -2660,10 +2668,10 @@ _ssl__SSLSocket_read_impl(PySSLSocket *self, Py_ssize_t len,
26602668
deadline = _PyDeadline_Init(timeout);
26612669

26622670
do {
2663-
PySSL_BEGIN_ALLOW_THREADS
2671+
PySSL_BEGIN_ALLOW_THREADS(self)
26642672
retval = SSL_read_ex(self->ssl, mem, (size_t)len, &count);
26652673
err = _PySSL_errno(retval == 0, self->ssl, retval);
2666-
PySSL_END_ALLOW_THREADS
2674+
PySSL_END_ALLOW_THREADS(self)
26672675
self->err = err;
26682676

26692677
if (PyErr_CheckSignals())
@@ -2762,7 +2770,7 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
27622770
}
27632771

27642772
while (1) {
2765-
PySSL_BEGIN_ALLOW_THREADS
2773+
PySSL_BEGIN_ALLOW_THREADS(self)
27662774
/* Disable read-ahead so that unwrap can work correctly.
27672775
* Otherwise OpenSSL might read in too much data,
27682776
* eating clear text data that happens to be
@@ -2775,7 +2783,7 @@ _ssl__SSLSocket_shutdown_impl(PySSLSocket *self)
27752783
SSL_set_read_ahead(self->ssl, 0);
27762784
ret = SSL_shutdown(self->ssl);
27772785
err = _PySSL_errno(ret < 0, self->ssl, ret);
2778-
PySSL_END_ALLOW_THREADS
2786+
PySSL_END_ALLOW_THREADS(self)
27792787
self->err = err;
27802788

27812789
/* If err == 1, a secure shutdown with SSL_shutdown() is complete */
@@ -3167,9 +3175,10 @@ _ssl__SSLContext_impl(PyTypeObject *type, int proto_version)
31673175
// no other thread can be touching this object yet.
31683176
// (Technically, we can't even lock if we wanted to, as the
31693177
// lock hasn't been initialized yet.)
3170-
PySSL_BEGIN_ALLOW_THREADS
3178+
Py_BEGIN_ALLOW_THREADS
31713179
ctx = SSL_CTX_new(method);
3172-
PySSL_END_ALLOW_THREADS
3180+
Py_END_ALLOW_THREADS
3181+
_PySSL_FIX_ERRNO;
31733182

31743183
if (ctx == NULL) {
31753184
_setSSLError(get_ssl_state(module), NULL, 0, __FILE__, __LINE__);
@@ -3312,9 +3321,10 @@ context_clear(PyObject *op)
33123321
Py_CLEAR(self->psk_server_callback);
33133322
#endif
33143323
if (self->keylog_bio != NULL) {
3315-
PySSL_BEGIN_ALLOW_THREADS
3324+
Py_BEGIN_ALLOW_THREADS
33163325
BIO_free_all(self->keylog_bio);
3317-
PySSL_END_ALLOW_THREADS
3326+
Py_END_ALLOW_THREADS
3327+
_PySSL_FIX_ERRNO;
33183328
self->keylog_bio = NULL;
33193329
}
33203330
return 0;
@@ -4037,7 +4047,8 @@ _password_callback(char *buf, int size, int rwflag, void *userdata)
40374047
_PySSLPasswordInfo *pw_info = (_PySSLPasswordInfo*) userdata;
40384048
PyObject *fn_ret = NULL;
40394049

4040-
PySSL_END_ALLOW_THREADS_S(pw_info->thread_state);
4050+
pw_info->thread_state = PyThreadState_Swap(pw_info->thread_state);
4051+
_PySSL_FIX_ERRNO;
40414052

40424053
if (pw_info->error) {
40434054
/* already failed previously. OpenSSL 3.0.0-alpha14 invokes the
@@ -4067,13 +4078,13 @@ _password_callback(char *buf, int size, int rwflag, void *userdata)
40674078
goto error;
40684079
}
40694080

4070-
PySSL_BEGIN_ALLOW_THREADS_S(pw_info->thread_state);
4081+
pw_info->thread_state = PyThreadState_Swap(pw_info->thread_state);
40714082
memcpy(buf, pw_info->password, pw_info->size);
40724083
return pw_info->size;
40734084

40744085
error:
40754086
Py_XDECREF(fn_ret);
4076-
PySSL_BEGIN_ALLOW_THREADS_S(pw_info->thread_state);
4087+
pw_info->thread_state = PyThreadState_Swap(pw_info->thread_state);
40774088
pw_info->error = 1;
40784089
return -1;
40794090
}
@@ -4126,10 +4137,10 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile,
41264137
SSL_CTX_set_default_passwd_cb(self->ctx, _password_callback);
41274138
SSL_CTX_set_default_passwd_cb_userdata(self->ctx, &pw_info);
41284139
}
4129-
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state);
4140+
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
41304141
r = SSL_CTX_use_certificate_chain_file(self->ctx,
41314142
PyBytes_AS_STRING(certfile_bytes));
4132-
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state);
4143+
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
41334144
if (r != 1) {
41344145
if (pw_info.error) {
41354146
ERR_clear_error();
@@ -4144,11 +4155,11 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile,
41444155
}
41454156
goto error;
41464157
}
4147-
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state);
4158+
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
41484159
r = SSL_CTX_use_PrivateKey_file(self->ctx,
41494160
PyBytes_AS_STRING(keyfile ? keyfile_bytes : certfile_bytes),
41504161
SSL_FILETYPE_PEM);
4151-
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state);
4162+
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
41524163
Py_CLEAR(keyfile_bytes);
41534164
Py_CLEAR(certfile_bytes);
41544165
if (r != 1) {
@@ -4165,9 +4176,9 @@ _ssl__SSLContext_load_cert_chain_impl(PySSLContext *self, PyObject *certfile,
41654176
}
41664177
goto error;
41674178
}
4168-
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state);
4179+
PySSL_BEGIN_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
41694180
r = SSL_CTX_check_private_key(self->ctx);
4170-
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state);
4181+
PySSL_END_ALLOW_THREADS_S(pw_info.thread_state, &self->tstate_mutex);
41714182
if (r != 1) {
41724183
_setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
41734184
goto error;
@@ -4384,9 +4395,9 @@ _ssl__SSLContext_load_verify_locations_impl(PySSLContext *self,
43844395
cafile_buf = PyBytes_AS_STRING(cafile_bytes);
43854396
if (capath)
43864397
capath_buf = PyBytes_AS_STRING(capath_bytes);
4387-
PySSL_BEGIN_ALLOW_THREADS
4398+
PySSL_BEGIN_ALLOW_THREADS(self)
43884399
r = SSL_CTX_load_verify_locations(self->ctx, cafile_buf, capath_buf);
4389-
PySSL_END_ALLOW_THREADS
4400+
PySSL_END_ALLOW_THREADS(self)
43904401
if (r != 1) {
43914402
if (errno != 0) {
43924403
PyErr_SetFromErrno(PyExc_OSError);
@@ -4438,10 +4449,11 @@ _ssl__SSLContext_load_dh_params_impl(PySSLContext *self, PyObject *filepath)
44384449
return NULL;
44394450

44404451
errno = 0;
4441-
PySSL_BEGIN_ALLOW_THREADS
4452+
Py_BEGIN_ALLOW_THREADS
44424453
dh = PEM_read_DHparams(f, NULL, NULL, NULL);
44434454
fclose(f);
4444-
PySSL_END_ALLOW_THREADS
4455+
Py_END_ALLOW_THREADS
4456+
_PySSL_FIX_ERRNO;
44454457
if (dh == NULL) {
44464458
if (errno != 0) {
44474459
PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, filepath);
@@ -4593,6 +4605,7 @@ _ssl__SSLContext_set_default_verify_paths_impl(PySSLContext *self)
45934605
Py_BEGIN_ALLOW_THREADS
45944606
rc = SSL_CTX_set_default_verify_paths(self->ctx);
45954607
Py_END_ALLOW_THREADS
4608+
_PySSL_FIX_ERRNO;
45964609
if (!rc) {
45974610
_setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
45984611
return NULL;

‎Modules/_ssl/debughelpers.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -140,13 +140,13 @@ _PySSL_keylog_callback(const SSL *ssl, const char *line)
140140
* critical debug helper.
141141
*/
142142

143-
PySSL_BEGIN_ALLOW_THREADS
143+
PySSL_BEGIN_ALLOW_THREADS(ssl_obj)
144144
PyThread_acquire_lock(lock, 1);
145145
res = BIO_printf(ssl_obj->ctx->keylog_bio, "%s\n", line);
146146
e = errno;
147147
(void)BIO_flush(ssl_obj->ctx->keylog_bio);
148148
PyThread_release_lock(lock);
149-
PySSL_END_ALLOW_THREADS
149+
PySSL_END_ALLOW_THREADS(ssl_obj)
150150

151151
if (res == -1) {
152152
errno = e;
@@ -187,9 +187,9 @@ _PySSLContext_set_keylog_filename(PyObject *op, PyObject *arg,
187187
if (self->keylog_bio != NULL) {
188188
BIO *bio = self->keylog_bio;
189189
self->keylog_bio = NULL;
190-
PySSL_BEGIN_ALLOW_THREADS
190+
Py_BEGIN_ALLOW_THREADS
191191
BIO_free_all(bio);
192-
PySSL_END_ALLOW_THREADS
192+
Py_END_ALLOW_THREADS
193193
}
194194

195195
if (arg == Py_None) {
@@ -211,13 +211,13 @@ _PySSLContext_set_keylog_filename(PyObject *op, PyObject *arg,
211211
self->keylog_filename = Py_NewRef(arg);
212212

213213
/* Write a header for seekable, empty files (this excludes pipes). */
214-
PySSL_BEGIN_ALLOW_THREADS
214+
PySSL_BEGIN_ALLOW_THREADS(self)
215215
if (BIO_tell(self->keylog_bio) == 0) {
216216
BIO_puts(self->keylog_bio,
217217
"# TLS secrets log file, generated by OpenSSL / Python\n");
218218
(void)BIO_flush(self->keylog_bio);
219219
}
220-
PySSL_END_ALLOW_THREADS
220+
PySSL_END_ALLOW_THREADS(self)
221221
SSL_CTX_set_keylog_callback(self->ctx, _PySSL_keylog_callback);
222222
return 0;
223223
}

0 commit comments

Comments
 (0)