From 2e470c363c4d58c51d366e10afb57b0605b7addb 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, 19 Oct 2026 17:55:09 -0700 Subject: [PATCH] gh-159786: Convert HACL* update() errors in `_hmac` after releasing the lock (GH-159805) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * gh-159786: Convert HACL* update() errors in `_hmac` after releasing the lock `_hacl_hmac_state_update_once()` converted the HACL* exit code into an exception while the thread state was detached and, for HMAC.update(), while the object's mutex was held. On debug builds, this re-attached the thread state under the mutex and then called PyErr_Occurred() with the thread state detached in the chunking loop of `_hacl_hmac_state_update()`. Return the raw HACL* exit code instead and convert it in `_hmac.new()` and `_hmac.HMAC.update()` once the thread state is re-attached and the mutex is released, as `hmac_digest_compute_locked()` already does. Release builds keep ignoring the update() exit code. Since `_hacl_convert_errno()` is now always called with an attached thread state, remove the `PyGILState_Ensure()` added by gh-135561 and assert that a thread state is attached instead. * simplify ifdefs: always check for the hacl error --------- (cherry picked from commit 0fba211fefd110e84cf3a064f7fde5074810bc95) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> Co-authored-by: Gregory P. Smith --- ...-10-19-21-20-28.gh-issue-159786.fThehQ.rst | 6 +++ Modules/hmacmodule.c | 50 ++++++++----------- 2 files changed, 27 insertions(+), 29 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-10-19-21-20-28.gh-issue-159786.fThehQ.rst diff --git a/Misc/NEWS.d/next/Library/2026-10-19-21-20-28.gh-issue-159786.fThehQ.rst b/Misc/NEWS.d/next/Library/2026-10-19-21-20-28.gh-issue-159786.fThehQ.rst new file mode 100644 index 000000000000000..3f901326c87f68e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-10-19-21-20-28.gh-issue-159786.fThehQ.rst @@ -0,0 +1,6 @@ +Fix the error handling of the HACL* routine used by :func:`hmac.new` and +:meth:`hmac.HMAC.update`. If the maximum input length of the underlying hash +function is exceeded, an :exc:`OverflowError` is now raised on all builds, +instead of crashing on DEBUG builds and being silently ignored otherwise. +Code correctness. Realistically this can only happen at implausible sizes. +Patch by Bénédikt Tran. diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index 96a91ce9754bdc0..a76a7056e494437 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -379,16 +379,17 @@ narrow_hmac_hash_kind(hmacmodule_state *state, HMAC_Hash_Kind kind) * * If 'code' represents a successful operation, this returns 0. * Otherwise, this sets an appropriate exception and returns -1. + * + * The caller must have an attached thread state. */ static int _hacl_convert_errno(hacl_errno_t code) { - assert(PyGILState_GetThisThreadState() != NULL); + assert(PyThreadState_GetUnchecked() != NULL); if (code == Hacl_Streaming_Types_Success) { return 0; } - PyGILState_STATE gstate = PyGILState_Ensure(); switch (code) { case Hacl_Streaming_Types_InvalidAlgorithm: { PyErr_SetString(PyExc_ValueError, "invalid HACL* algorithm"); @@ -413,7 +414,6 @@ _hacl_convert_errno(hacl_errno_t code) break; } } - PyGILState_Release(gstate); return -1; } @@ -449,39 +449,33 @@ _hacl_hmac_state_free(HACL_HMAC_state *state) /* * Call the HACL* HMAC-HASH update function on the given data. * - * On DEBUG builds, the update() call is verified. - * - * Return 0 on success; otherwise, set an exception and return -1 on failure. + * Return the HACL* exit code. No exception is set, so that this + * can be called without an attached thread state or with a lock held. */ -static int +static hacl_errno_t _hacl_hmac_state_update_once(HACL_HMAC_state *state, uint8_t *buf, uint32_t len) { -#ifndef NDEBUG - hacl_errno_t code = Hacl_Streaming_HMAC_update(state, buf, len); - return _hacl_convert_errno(code); -#else - (void)Hacl_Streaming_HMAC_update(state, buf, len); - return 0; -#endif + return Hacl_Streaming_HMAC_update(state, buf, len); } /* * Perform the HMAC-HASH update() operation in a streaming fashion. * - * On DEBUG builds, each update() call is verified. - * - * Return 0 on success; otherwise, set an exception and return -1 on failure. + * Return the HACL* exit code. No exception is set, so that this + * can be called without an attached thread state or with a lock held. + * Use _hacl_convert_errno() afterwards to set an appropriate exception. */ -static int +static hacl_errno_t _hacl_hmac_state_update(HACL_HMAC_state *state, uint8_t *buf, Py_ssize_t len) { assert(len >= 0); #ifdef Py_HMAC_SSIZE_LARGER_THAN_UINT32 while (len > UINT32_MAX_AS_SSIZE_T) { - if (_hacl_hmac_state_update_once(state, buf, UINT32_MAX) < 0) { - assert(PyErr_Occurred()); - return -1; + hacl_errno_t code = _hacl_hmac_state_update_once(state, buf, + UINT32_MAX); + if (code != Hacl_Streaming_Types_Success) { + return code; } buf += UINT32_MAX; len -= UINT32_MAX; @@ -786,18 +780,16 @@ _hmac_new_impl(PyObject *module, PyObject *keyobj, PyObject *msgobj, GET_BUFFER_VIEW_OR_ERROR(msgobj, &msg, goto error); /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ + hacl_errno_t code; HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( msg.len, - rc = _hacl_hmac_state_update(self->state, msg.buf, msg.len) + code = _hacl_hmac_state_update(self->state, msg.buf, msg.len) ); PyBuffer_Release(&msg); -#ifndef NDEBUG + rc = _hacl_convert_errno(code); if (rc < 0) { goto error; } -#else - (void)rc; -#endif } assert(rc == 0); return (PyObject *)self; @@ -889,15 +881,15 @@ static PyObject * _hmac_HMAC_update_impl(HMACObject *self, PyObject *msgobj) /*[clinic end generated code: output=962134ada5e55985 input=7c0ea830efb03367]*/ { - int rc = 0; + hacl_errno_t code; Py_buffer msg; GET_BUFFER_VIEW_OR_ERROUT(msgobj, &msg); HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( self, msg.len, - rc = _hacl_hmac_state_update(self->state, msg.buf, msg.len) + code = _hacl_hmac_state_update(self->state, msg.buf, msg.len) ); PyBuffer_Release(&msg); - return rc < 0 ? NULL : Py_None; + return _hacl_convert_errno(code) < 0 ? NULL : Py_None; } /*