Skip to content

Commit 2346981

Browse files
authored
gh-149816: fix a race when fetching OpenSSL digests (#153019)
1 parent c6a084a commit 2346981

2 files changed

Lines changed: 33 additions & 27 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: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -693,43 +693,46 @@ static PY_EVP_MD *
693693
get_openssl_evp_md_by_utf8name(_hashlibstate *state, const char *name,
694694
Py_hash_type py_ht)
695695
{
696-
PY_EVP_MD *digest = NULL, *other_digest = NULL;
696+
PY_EVP_MD *digest = NULL, *fresh = NULL;
697697
py_hashentry_t *entry = _Py_hashtable_get(state->hashtable, name);
698+
int fips = !disable_fips_property(py_ht);
698699

699700
if (entry != NULL) {
700-
if (!disable_fips_property(py_ht)) {
701-
digest = FT_ATOMIC_LOAD_PTR_RELAXED(entry->evp);
702-
if (digest == NULL) {
703-
digest = PY_EVP_MD_fetch(entry->ossl_name, NULL);
704-
#ifdef Py_GIL_DISABLED
705-
// exchange just in case another thread did same thing at same time
706-
other_digest = _Py_atomic_exchange_ptr(&entry->evp, (void *)digest);
707-
#else
708-
entry->evp = digest;
709-
#endif
710-
}
711-
}
712-
else {
713-
digest = FT_ATOMIC_LOAD_PTR_RELAXED(entry->evp_nosecurity);
714-
if (digest == NULL) {
715-
digest = PY_EVP_MD_fetch(entry->ossl_name, "-fips");
716-
#ifdef Py_GIL_DISABLED
717-
// exchange just in case another thread did same thing at same time
718-
other_digest = _Py_atomic_exchange_ptr(&entry->evp_nosecurity, (void *)digest);
719-
#else
720-
entry->evp_nosecurity = digest;
721-
#endif
701+
PY_EVP_MD **cached = fips ? &entry->evp : &entry->evp_nosecurity;
702+
digest = FT_ATOMIC_LOAD_PTR_RELAXED(*cached);
703+
if (digest == NULL) {
704+
fresh = PY_EVP_MD_fetch(entry->ossl_name, fips ? NULL : "-fips");
705+
if (fresh != NULL) {
706+
/*
707+
* Publish a freshly fetched digest into the cache exactly once.
708+
*
709+
* On the first publication, 'cached' adopts the 'fresh' ref.
710+
* If another thread wins the race, 'fresh' is released and
711+
* the cached digest is used instead.
712+
*
713+
* See https://github.com/python/cpython/issues/128657
714+
* and https://github.com/python/cpython/issues/149816.
715+
*/
716+
PY_EVP_MD *expected = NULL;
717+
if (_Py_atomic_compare_exchange_ptr(cached, &expected, (void *)fresh)) {
718+
digest = fresh; // we won: 'cached' now owns the fresh digest
719+
}
720+
else {
721+
PY_EVP_MD_free(fresh); // we lost: discard the fresh digest
722+
digest = expected; // reuse the winner's digest
723+
}
722724
}
723725
}
724-
// if another thread same thing at same time make sure we got same ptr
725-
assert(other_digest == NULL || other_digest == digest);
726-
if (digest != NULL && other_digest == NULL) {
726+
/* Hand the caller its own reference; the cache keeps its own. Valid
727+
* whether `digest` was already cached, freshly published by this
728+
* thread, or published by a thread that won the race. */
729+
if (digest != NULL) {
727730
PY_EVP_MD_up_ref(digest);
728731
}
729732
}
730733
else {
731734
// Fall back for looking up an unindexed OpenSSL specific name.
732-
const char *props = disable_fips_property(py_ht) ? "-fips" : NULL;
735+
const char *props = fips ? NULL : "-fips";
733736
(void)props; // will only be used in OpenSSL 3.0 and later
734737
digest = PY_EVP_MD_fetch(name, props);
735738
}

0 commit comments

Comments
 (0)