Skip to content

Commit e81a055

Browse files
committed
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.
1 parent b2b0a33 commit e81a055

2 files changed

Lines changed: 27 additions & 18 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix a crash on DEBUG builds when the HACL* routine used by
2+
:meth:`hmac.HMAC.update` fails. This only happens for messages larger
3+
than 4 GiB once the maximum input length of the underlying hash function
4+
is exceeded. Patch by Bénédikt Tran.

‎Modules/hmacmodule.c‎

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -379,16 +379,17 @@ narrow_hmac_hash_kind(hmacmodule_state *state, HMAC_Hash_Kind kind)
379379
*
380380
* If 'code' represents a successful operation, this returns 0.
381381
* Otherwise, this sets an appropriate exception and returns -1.
382+
*
383+
* The caller must have an attached thread state.
382384
*/
383385
static int
384386
_hacl_convert_errno(hacl_errno_t code)
385387
{
386-
assert(PyGILState_GetThisThreadState() != NULL);
388+
assert(PyThreadState_GetUnchecked() != NULL);
387389
if (code == Hacl_Streaming_Types_Success) {
388390
return 0;
389391
}
390392

391-
PyGILState_STATE gstate = PyGILState_Ensure();
392393
switch (code) {
393394
case Hacl_Streaming_Types_InvalidAlgorithm: {
394395
PyErr_SetString(PyExc_ValueError, "invalid HACL* algorithm");
@@ -413,7 +414,6 @@ _hacl_convert_errno(hacl_errno_t code)
413414
break;
414415
}
415416
}
416-
PyGILState_Release(gstate);
417417
return -1;
418418
}
419419

@@ -451,18 +451,18 @@ _hacl_hmac_state_free(HACL_HMAC_state *state)
451451
*
452452
* On DEBUG builds, the update() call is verified.
453453
*
454-
* Return 0 on success; otherwise, set an exception and return -1 on failure.
454+
* Return the HACL* exit code. No exception is set, so that this
455+
* can be called without an attached thread state or with a lock held.
455456
*/
456-
static int
457+
static hacl_errno_t
457458
_hacl_hmac_state_update_once(HACL_HMAC_state *state,
458459
uint8_t *buf, uint32_t len)
459460
{
460461
#ifndef NDEBUG
461-
hacl_errno_t code = Hacl_Streaming_HMAC_update(state, buf, len);
462-
return _hacl_convert_errno(code);
462+
return Hacl_Streaming_HMAC_update(state, buf, len);
463463
#else
464464
(void)Hacl_Streaming_HMAC_update(state, buf, len);
465-
return 0;
465+
return Hacl_Streaming_Types_Success;
466466
#endif
467467
}
468468

@@ -471,17 +471,20 @@ _hacl_hmac_state_update_once(HACL_HMAC_state *state,
471471
*
472472
* On DEBUG builds, each update() call is verified.
473473
*
474-
* Return 0 on success; otherwise, set an exception and return -1 on failure.
474+
* Return the HACL* exit code. No exception is set, so that this
475+
* can be called without an attached thread state or with a lock held.
476+
* Use _hacl_convert_errno() afterwards to set an appropriate exception.
475477
*/
476-
static int
478+
static hacl_errno_t
477479
_hacl_hmac_state_update(HACL_HMAC_state *state, uint8_t *buf, Py_ssize_t len)
478480
{
479481
assert(len >= 0);
480482
#ifdef Py_HMAC_SSIZE_LARGER_THAN_UINT32
481483
while (len > UINT32_MAX_AS_SSIZE_T) {
482-
if (_hacl_hmac_state_update_once(state, buf, UINT32_MAX) < 0) {
483-
assert(PyErr_Occurred());
484-
return -1;
484+
hacl_errno_t code = _hacl_hmac_state_update_once(state, buf,
485+
UINT32_MAX);
486+
if (code != Hacl_Streaming_Types_Success) {
487+
return code;
485488
}
486489
buf += UINT32_MAX;
487490
len -= UINT32_MAX;
@@ -786,17 +789,19 @@ _hmac_new_impl(PyObject *module, PyObject *keyobj, PyObject *msgobj,
786789
GET_BUFFER_VIEW_OR_ERROR(msgobj, &msg, goto error);
787790
/* Do not use self->mutex here as this is the constructor
788791
* where it is not yet possible to have concurrent access. */
792+
hacl_errno_t code;
789793
HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED(
790794
msg.len,
791-
rc = _hacl_hmac_state_update(self->state, msg.buf, msg.len)
795+
code = _hacl_hmac_state_update(self->state, msg.buf, msg.len)
792796
);
793797
PyBuffer_Release(&msg);
794798
#ifndef NDEBUG
799+
rc = _hacl_convert_errno(code);
795800
if (rc < 0) {
796801
goto error;
797802
}
798803
#else
799-
(void)rc;
804+
(void)code;
800805
#endif
801806
}
802807
assert(rc == 0);
@@ -889,15 +894,15 @@ static PyObject *
889894
_hmac_HMAC_update_impl(HMACObject *self, PyObject *msgobj)
890895
/*[clinic end generated code: output=962134ada5e55985 input=7c0ea830efb03367]*/
891896
{
892-
int rc = 0;
897+
hacl_errno_t code;
893898
Py_buffer msg;
894899
GET_BUFFER_VIEW_OR_ERROUT(msgobj, &msg);
895900
HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED(
896901
self, msg.len,
897-
rc = _hacl_hmac_state_update(self->state, msg.buf, msg.len)
902+
code = _hacl_hmac_state_update(self->state, msg.buf, msg.len)
898903
);
899904
PyBuffer_Release(&msg);
900-
return rc < 0 ? NULL : Py_None;
905+
return _hacl_convert_errno(code) < 0 ? NULL : Py_None;
901906
}
902907

903908
/*

0 commit comments

Comments
 (0)