Skip to content

gh-159786: Convert HACL* update() errors in _hmac after releasing the lock - #159805

Merged
gpshead merged 2 commits into
python:mainfrom
picnixz:meta/crypto/c/hmac/33-45/gil-issue-159786
Oct 20, 2026
Merged

gpshead merged 2 commits into
python:mainfrom
picnixz:meta/crypto/c/hmac/33-45/gil-issue-159786

Conversation

@picnixz

@picnixz picnixz commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

_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 now also check for the error making for far simpler code. The actual trigger for an error from hacl was an unrealistically large data size.

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.

…sing 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 pythongh-135561 and
assert that a thread state is attached instead.

@gpshead gpshead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a commit... lets also simplify and always check for the error regardless of debug build or not.

@picnixz picnixz added the sprint label Oct 20, 2026
@picnixz picnixz self-assigned this Oct 20, 2026
@github-project-automation github-project-automation Bot moved this to Todo in Sprint Oct 20, 2026
@github-project-automation github-project-automation Bot moved this from Todo to In Progress in Sprint Oct 20, 2026
@gpshead
gpshead merged commit 0fba211 into python:main Oct 20, 2026
62 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Sprint Oct 20, 2026
@miss-islington-app

Copy link
Copy Markdown

Thanks @picnixz for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖

@bedevere-app

bedevere-app Bot commented Oct 20, 2026

Copy link
Copy Markdown

GH-159825 is a backport of this pull request to the 3.15 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 bugs and security fixes label Oct 20, 2026
@picnixz
picnixz deleted the meta/crypto/c/hmac/33-45/gil-issue-159786 branch October 20, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants