From f58811fe037fcacb5a9f64a05d94656217340334 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 4 Jul 2026 13:27:12 +0200 Subject: [PATCH 1/4] gh-149816: fix a race when fetching OpenSSL digests --- ...-07-04-12-52-22.gh-issue-149816.XQOutB.rst | 2 + Modules/_hashopenssl.c | 55 +++++++++++-------- 2 files changed, 33 insertions(+), 24 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-07-04-12-52-22.gh-issue-149816.XQOutB.rst diff --git a/Misc/NEWS.d/next/Library/2026-07-04-12-52-22.gh-issue-149816.XQOutB.rst b/Misc/NEWS.d/next/Library/2026-07-04-12-52-22.gh-issue-149816.XQOutB.rst new file mode 100644 index 000000000000000..bce4628d025876d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-07-04-12-52-22.gh-issue-149816.XQOutB.rst @@ -0,0 +1,2 @@ +:mod:`hashlib`: fix a race when fetching OpenSSL digests. Patch by Bénédikt +Tran. diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index f895c9037485c43..a1444177a10ec0c 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -692,43 +692,50 @@ static PY_EVP_MD * get_openssl_evp_md_by_utf8name(_hashlibstate *state, const char *name, Py_hash_type py_ht) { - PY_EVP_MD *digest = NULL, *other_digest = NULL; + PY_EVP_MD *digest = NULL, *fresh = NULL; py_hashentry_t *entry = _Py_hashtable_get(state->hashtable, name); + int fips = !disable_fips_property(py_ht); if (entry != NULL) { - if (!disable_fips_property(py_ht)) { - digest = FT_ATOMIC_LOAD_PTR_RELAXED(entry->evp); - if (digest == NULL) { - digest = PY_EVP_MD_fetch(entry->ossl_name, NULL); + PY_EVP_MD **cached = fips ? &entry->evp : &entry->evp_nosecurity; + digest = FT_ATOMIC_LOAD_PTR_RELAXED(*cached); + if (digest == NULL) { + fresh = PY_EVP_MD_fetch(entry->ossl_name, fips ? NULL : "-fips"); + if (fresh != NULL) { + /* + * Publish a freshly fetched digest into the cache exactly once. + * + * On the first publication, 'cached' adopts the 'fresh' ref. + * If another thread wins the race, 'fresh' is released and + * the cached digest is used instead. + * + * See https://github.com/python/cpython/issues/128657 + * and https://github.com/python/cpython/issues/149816. + */ #ifdef Py_GIL_DISABLED - // exchange just in case another thread did same thing at same time - other_digest = _Py_atomic_exchange_ptr(&entry->evp, (void *)digest); + PY_EVP_MD *expected = NULL; + if (_Py_atomic_compare_exchange_ptr(cached, &expected, (void *)fresh)) { + digest = fresh; // we won: 'cached' now owns the fresh digest + } + else { + PY_EVP_MD_free(fresh); // we lost: discard the fresh digest + digest = expected; // reuse the winner's digest + } #else - entry->evp = digest; + digest = *cached = fresh; #endif } } - else { - digest = FT_ATOMIC_LOAD_PTR_RELAXED(entry->evp_nosecurity); - if (digest == NULL) { - digest = PY_EVP_MD_fetch(entry->ossl_name, "-fips"); -#ifdef Py_GIL_DISABLED - // exchange just in case another thread did same thing at same time - other_digest = _Py_atomic_exchange_ptr(&entry->evp_nosecurity, (void *)digest); -#else - entry->evp_nosecurity = digest; -#endif - } - } - // if another thread same thing at same time make sure we got same ptr - assert(other_digest == NULL || other_digest == digest); - if (digest != NULL && other_digest == NULL) { + /* Hand the caller its own reference; the cache keeps its own. Valid + * whether `digest` was already cached, freshly published by this + * thread, or published by a thread that won the race. */ + if (digest != NULL) { PY_EVP_MD_up_ref(digest); } } else { // Fall back for looking up an unindexed OpenSSL specific name. - const char *props = disable_fips_property(py_ht) ? "-fips" : NULL; + const char *props = fips ? NULL : "-fips"; (void)props; // will only be used in OpenSSL 3.0 and later digest = PY_EVP_MD_fetch(name, props); } From dc7085966c31436ceec50fe2cdeee9eddb8442a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 6 Jul 2026 10:01:32 +0200 Subject: [PATCH 2/4] unconditionally do a CAS --- Modules/_hashopenssl.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index a1444177a10ec0c..bd1c5f1ad1cbc69 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -712,7 +712,6 @@ get_openssl_evp_md_by_utf8name(_hashlibstate *state, const char *name, * See https://github.com/python/cpython/issues/128657 * and https://github.com/python/cpython/issues/149816. */ -#ifdef Py_GIL_DISABLED PY_EVP_MD *expected = NULL; if (_Py_atomic_compare_exchange_ptr(cached, &expected, (void *)fresh)) { digest = fresh; // we won: 'cached' now owns the fresh digest @@ -721,9 +720,6 @@ get_openssl_evp_md_by_utf8name(_hashlibstate *state, const char *name, PY_EVP_MD_free(fresh); // we lost: discard the fresh digest digest = expected; // reuse the winner's digest } -#else - digest = *cached = fresh; -#endif } } /* Hand the caller its own reference; the cache keeps its own. Valid From 8e95250deee6f79dfb986f6c5aa7202ac06cab13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 6 Jul 2026 10:05:26 +0200 Subject: [PATCH 3/4] amend NEWS --- .../Library/2026-07-04-12-52-22.gh-issue-149816.XQOutB.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2026-07-04-12-52-22.gh-issue-149816.XQOutB.rst b/Misc/NEWS.d/next/Library/2026-07-04-12-52-22.gh-issue-149816.XQOutB.rst index bce4628d025876d..bad4a478d6b2313 100644 --- a/Misc/NEWS.d/next/Library/2026-07-04-12-52-22.gh-issue-149816.XQOutB.rst +++ b/Misc/NEWS.d/next/Library/2026-07-04-12-52-22.gh-issue-149816.XQOutB.rst @@ -1,2 +1,3 @@ -:mod:`hashlib`: fix a race when fetching OpenSSL digests. Patch by Bénédikt -Tran. +:mod:`hashlib`: fix a memory leak arising from a race condition when looking +up an OpenSSL hash algorithm by name the first time. Patch by Bénédikt Tran. + From ae22c227a051120e8e642c5757080be6b925ce9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 6 Jul 2026 10:06:14 +0200 Subject: [PATCH 4/4] amend NEWS --- .../next/Library/2026-07-04-12-52-22.gh-issue-149816.XQOutB.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2026-07-04-12-52-22.gh-issue-149816.XQOutB.rst b/Misc/NEWS.d/next/Library/2026-07-04-12-52-22.gh-issue-149816.XQOutB.rst index bad4a478d6b2313..eb273865894aae0 100644 --- a/Misc/NEWS.d/next/Library/2026-07-04-12-52-22.gh-issue-149816.XQOutB.rst +++ b/Misc/NEWS.d/next/Library/2026-07-04-12-52-22.gh-issue-149816.XQOutB.rst @@ -1,3 +1,3 @@ -:mod:`hashlib`: fix a memory leak arising from a race condition when looking +:mod:`hashlib`: fix a crash arising from a race condition when looking up an OpenSSL hash algorithm by name the first time. Patch by Bénédikt Tran.