Skip to content

Commit ea5a4de

Browse files
committed
[3.14] gh-149816: fix a race when fetching OpenSSL digests (GH-153019)
(cherry picked from commit 2346981) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
1 parent b0a0405 commit ea5a4de

2 files changed

Lines changed: 53 additions & 47 deletions

File tree

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:mod:`hashlib`: fix a crash arising from a race condition when looking
2+
up an OpenSSL hash algorithm by name the first time. Patch by Bénédikt Tran.
3+

‎Modules/_hashopenssl.c‎

Lines changed: 50 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -392,65 +392,68 @@ py_digest_name(const EVP_MD *md)
392392
return PyUnicode_FromString(name);
393393
}
394394

395+
static inline int
396+
disable_fips_property(enum Py_hash_type py_ht)
397+
{
398+
switch (py_ht) {
399+
case Py_ht_evp:
400+
case Py_ht_mac:
401+
case Py_ht_pbkdf2:
402+
return 0;
403+
case Py_ht_evp_nosecurity:
404+
return 1;
405+
default:
406+
Py_FatalError("unsupported hash type");
407+
}
408+
}
409+
395410
/* Get EVP_MD by HID and purpose */
396411
static PY_EVP_MD*
397412
py_digest_by_name(PyObject *module, const char *name, enum Py_hash_type py_ht)
398413
{
399-
PY_EVP_MD *digest = NULL;
400-
PY_EVP_MD *other_digest = NULL;
414+
PY_EVP_MD *digest = NULL, *fresh = NULL;
401415
_hashlibstate *state = get_hashlib_state(module);
402-
py_hashentry_t *entry = (py_hashentry_t *)_Py_hashtable_get(
403-
state->hashtable, (const void*)name
404-
);
416+
py_hashentry_t *entry = _Py_hashtable_get(state->hashtable, name);
417+
int fips = disable_fips_property(py_ht);
405418

406419
if (entry != NULL) {
407-
switch (py_ht) {
408-
case Py_ht_evp:
409-
case Py_ht_mac:
410-
case Py_ht_pbkdf2:
411-
digest = FT_ATOMIC_LOAD_PTR_RELAXED(entry->evp);
412-
if (digest == NULL) {
413-
digest = PY_EVP_MD_fetch(entry->ossl_name, NULL);
414-
#ifdef Py_GIL_DISABLED
415-
// exchange just in case another thread did same thing at same time
416-
other_digest = _Py_atomic_exchange_ptr(&entry->evp, (void *)digest);
417-
#else
418-
entry->evp = digest;
419-
#endif
420+
PY_EVP_MD **cached = fips ? &entry->evp : &entry->evp_nosecurity;
421+
digest = FT_ATOMIC_LOAD_PTR_RELAXED(*cached);
422+
if (digest == NULL) {
423+
fresh = PY_EVP_MD_fetch(entry->ossl_name, fips ? NULL : "-fips");
424+
if (fresh != NULL) {
425+
/*
426+
* Publish a freshly fetched digest into the cache exactly once.
427+
*
428+
* On the first publication, 'cached' adopts the 'fresh' ref.
429+
* If another thread wins the race, 'fresh' is released and
430+
* the cached digest is used instead.
431+
*
432+
* See https://github.com/python/cpython/issues/128657
433+
* and https://github.com/python/cpython/issues/149816.
434+
*/
435+
PY_EVP_MD *expected = NULL;
436+
if (_Py_atomic_compare_exchange_ptr(cached, &expected, (void *)fresh)) {
437+
digest = fresh; // we won: 'cached' now owns the fresh digest
438+
}
439+
else {
440+
PY_EVP_MD_free(fresh); // we lost: discard the fresh digest
441+
digest = expected; // reuse the winner's digest
442+
}
420443
}
421-
break;
422-
case Py_ht_evp_nosecurity:
423-
digest = FT_ATOMIC_LOAD_PTR_RELAXED(entry->evp_nosecurity);
424-
if (digest == NULL) {
425-
digest = PY_EVP_MD_fetch(entry->ossl_name, "-fips");
426-
#ifdef Py_GIL_DISABLED
427-
// exchange just in case another thread did same thing at same time
428-
other_digest = _Py_atomic_exchange_ptr(&entry->evp_nosecurity, (void *)digest);
429-
#else
430-
entry->evp_nosecurity = digest;
431-
#endif
432-
}
433-
break;
434444
}
435-
// if another thread same thing at same time make sure we got same ptr
436-
assert(other_digest == NULL || other_digest == digest);
445+
/* Hand the caller its own reference; the cache keeps its own. Valid
446+
* whether `digest` was already cached, freshly published by this
447+
* thread, or published by a thread that won the race. */
437448
if (digest != NULL) {
438-
if (other_digest == NULL) {
439-
PY_EVP_MD_up_ref(digest);
440-
}
449+
PY_EVP_MD_up_ref(digest);
441450
}
442-
} else {
451+
}
452+
else {
443453
// Fall back for looking up an unindexed OpenSSL specific name.
444-
switch (py_ht) {
445-
case Py_ht_evp:
446-
case Py_ht_mac:
447-
case Py_ht_pbkdf2:
448-
digest = PY_EVP_MD_fetch(name, NULL);
449-
break;
450-
case Py_ht_evp_nosecurity:
451-
digest = PY_EVP_MD_fetch(name, "-fips");
452-
break;
453-
}
454+
const char *props = fips ? NULL : "-fips";
455+
(void)props; // will only be used in OpenSSL 3.0 and later
456+
digest = PY_EVP_MD_fetch(name, props);
454457
}
455458
if (digest == NULL) {
456459
raise_ssl_error(state->unsupported_digestmod_error,

0 commit comments

Comments
 (0)