Skip to content

Commit f58811f

Browse files
committed
gh-149816: fix a race when fetching OpenSSL digests
1 parent cfe3e07 commit f58811f

2 files changed

Lines changed: 33 additions & 24 deletions

File tree

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
:mod:`hashlib`: fix a race when fetching OpenSSL digests. Patch by Bénédikt
2+
Tran.

Modules/_hashopenssl.c

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -692,43 +692,50 @@ static PY_EVP_MD *
692692
get_openssl_evp_md_by_utf8name(_hashlibstate *state, const char *name,
693693
Py_hash_type py_ht)
694694
{
695-
PY_EVP_MD *digest = NULL, *other_digest = NULL;
695+
PY_EVP_MD *digest = NULL, *fresh = NULL;
696696
py_hashentry_t *entry = _Py_hashtable_get(state->hashtable, name);
697+
int fips = !disable_fips_property(py_ht);
697698

698699
if (entry != NULL) {
699-
if (!disable_fips_property(py_ht)) {
700-
digest = FT_ATOMIC_LOAD_PTR_RELAXED(entry->evp);
701-
if (digest == NULL) {
702-
digest = PY_EVP_MD_fetch(entry->ossl_name, NULL);
700+
PY_EVP_MD **cached = fips ? &entry->evp : &entry->evp_nosecurity;
701+
digest = FT_ATOMIC_LOAD_PTR_RELAXED(*cached);
702+
if (digest == NULL) {
703+
fresh = PY_EVP_MD_fetch(entry->ossl_name, fips ? NULL : "-fips");
704+
if (fresh != NULL) {
705+
/*
706+
* Publish a freshly fetched digest into the cache exactly once.
707+
*
708+
* On the first publication, 'cached' adopts the 'fresh' ref.
709+
* If another thread wins the race, 'fresh' is released and
710+
* the cached digest is used instead.
711+
*
712+
* See https://github.com/python/cpython/issues/128657
713+
* and https://github.com/python/cpython/issues/149816.
714+
*/
703715
#ifdef Py_GIL_DISABLED
704-
// exchange just in case another thread did same thing at same time
705-
other_digest = _Py_atomic_exchange_ptr(&entry->evp, (void *)digest);
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+
}
706724
#else
707-
entry->evp = digest;
725+
digest = *cached = fresh;
708726
#endif
709727
}
710728
}
711-
else {
712-
digest = FT_ATOMIC_LOAD_PTR_RELAXED(entry->evp_nosecurity);
713-
if (digest == NULL) {
714-
digest = PY_EVP_MD_fetch(entry->ossl_name, "-fips");
715-
#ifdef Py_GIL_DISABLED
716-
// exchange just in case another thread did same thing at same time
717-
other_digest = _Py_atomic_exchange_ptr(&entry->evp_nosecurity, (void *)digest);
718-
#else
719-
entry->evp_nosecurity = digest;
720-
#endif
721-
}
722-
}
723-
// if another thread same thing at same time make sure we got same ptr
724-
assert(other_digest == NULL || other_digest == digest);
725-
if (digest != NULL && other_digest == NULL) {
729+
/* Hand the caller its own reference; the cache keeps its own. Valid
730+
* whether `digest` was already cached, freshly published by this
731+
* thread, or published by a thread that won the race. */
732+
if (digest != NULL) {
726733
PY_EVP_MD_up_ref(digest);
727734
}
728735
}
729736
else {
730737
// Fall back for looking up an unindexed OpenSSL specific name.
731-
const char *props = disable_fips_property(py_ht) ? "-fips" : NULL;
738+
const char *props = fips ? NULL : "-fips";
732739
(void)props; // will only be used in OpenSSL 3.0 and later
733740
digest = PY_EVP_MD_fetch(name, props);
734741
}

0 commit comments

Comments
 (0)