Skip to content

Commit 4dcd5d4

Browse files
committed
gh-149816: fix a race when fetching OpenSSL digests
1 parent 8b1dbb1 commit 4dcd5d4

2 files changed

Lines changed: 34 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: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -692,43 +692,51 @@ 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+
const char *properties;
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+
*/
703716
#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);
717+
PY_EVP_MD *expected = NULL;
718+
if (_Py_atomic_compare_exchange_ptr(cached, &expected, (void *)fresh)) {
719+
digest = fresh; // we won: 'cached' now owns the fresh digest
720+
}
721+
else {
722+
PY_EVP_MD_free(fresh); // we lost: discard the fresh digest
723+
digest = expected; // reuse the winner's digest
724+
}
706725
#else
707-
entry->evp = digest;
726+
digest = *cached = fresh;
708727
#endif
709728
}
710729
}
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) {
730+
/* Hand the caller its own reference; the cache keeps its own. Valid
731+
* whether `digest` was already cached, freshly published by this
732+
* thread, or published by a thread that won the race. */
733+
if (digest != NULL) {
726734
PY_EVP_MD_up_ref(digest);
727735
}
728736
}
729737
else {
730738
// Fall back for looking up an unindexed OpenSSL specific name.
731-
const char *props = disable_fips_property(py_ht) ? "-fips" : NULL;
739+
const char *props = fips ? NULL : "-fips";
732740
(void)props; // will only be used in OpenSSL 3.0 and later
733741
digest = PY_EVP_MD_fetch(name, props);
734742
}

0 commit comments

Comments
 (0)