From 5d8c093b4937bc41cff7a911464e8ce5bab3700c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 8 Jun 2025 16:24:43 +0200 Subject: [PATCH 01/39] add common object head for hashlib/hmac objects --- Modules/hashlib.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Modules/hashlib.h b/Modules/hashlib.h index e82ec92be25c57..23475afb683d93 100644 --- a/Modules/hashlib.h +++ b/Modules/hashlib.h @@ -49,6 +49,14 @@ */ #include "pythread.h" + +#define HASHLIB_OBJECT_HEAD \ + PyObject_HEAD \ + /* prevent undefined behavior via multiple + * threads entering the C API */ \ + bool use_mutex; \ + PyMutex mutex; + #define ENTER_HASHLIB(obj) \ if ((obj)->use_mutex) { \ PyMutex_Lock(&(obj)->mutex); \ From 81e30463b76cac7266bf88af645f390da7263da6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 8 Jun 2025 16:41:10 +0200 Subject: [PATCH 02/39] simplify digest computation --- Modules/md5module.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/Modules/md5module.c b/Modules/md5module.c index 9b5ea2d6e02605..46f23ea8e32151 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -22,6 +22,7 @@ #include "Python.h" #include "hashlib.h" +#include "pycore_strhex.h" // _Py_strhex() /*[clinic input] module _md5 @@ -126,6 +127,14 @@ MD5Type_copy_impl(MD5object *self, PyTypeObject *cls) return (PyObject *)newobj; } +static void +md5_digest_compute_cond_lock(MD5object *self, uint8_t *digest) +{ + ENTER_HASHLIB(self); + Hacl_Hash_MD5_digest(self->hash_state, digest); + LEAVE_HASHLIB(self); +} + /*[clinic input] MD5Type.digest @@ -136,10 +145,8 @@ static PyObject * MD5Type_digest_impl(MD5object *self) /*[clinic end generated code: output=eb691dc4190a07ec input=bc0c4397c2994be6]*/ { - unsigned char digest[MD5_DIGESTSIZE]; - ENTER_HASHLIB(self); - Hacl_Hash_MD5_digest(self->hash_state, digest); - LEAVE_HASHLIB(self); + uint8_t digest[MD5_DIGESTSIZE]; + md5_digest_compute_cond_lock(self, digest); return PyBytes_FromStringAndSize((const char *)digest, MD5_DIGESTSIZE); } @@ -153,20 +160,9 @@ static PyObject * MD5Type_hexdigest_impl(MD5object *self) /*[clinic end generated code: output=17badced1f3ac932 input=b60b19de644798dd]*/ { - unsigned char digest[MD5_DIGESTSIZE]; - ENTER_HASHLIB(self); - Hacl_Hash_MD5_digest(self->hash_state, digest); - LEAVE_HASHLIB(self); - - const char *hexdigits = "0123456789abcdef"; - char digest_hex[MD5_DIGESTSIZE * 2]; - char *str = digest_hex; - for (size_t i=0; i < MD5_DIGESTSIZE; i++) { - unsigned char byte = digest[i]; - *str++ = hexdigits[byte >> 4]; - *str++ = hexdigits[byte & 0x0f]; - } - return PyUnicode_FromStringAndSize(digest_hex, sizeof(digest_hex)); + uint8_t digest[MD5_DIGESTSIZE]; + md5_digest_compute_cond_lock(self, digest); + return _Py_strhex((const char *)digest, MD5_DIGESTSIZE); } static void From 7f9f7b746d6dd5b0add1826138c9d8c8f3edc48e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 8 Jun 2025 16:41:59 +0200 Subject: [PATCH 03/39] refactor update logic --- Modules/md5module.c | 51 ++++++++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/Modules/md5module.c b/Modules/md5module.c index 46f23ea8e32151..2cdf889589415c 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -166,7 +166,7 @@ MD5Type_hexdigest_impl(MD5object *self) } static void -update(Hacl_Hash_MD5_state_t *state, uint8_t *buf, Py_ssize_t len) +_hacl_md5_update(Hacl_Hash_MD5_state_t *state, uint8_t *buf, Py_ssize_t len) { /* * Note: we explicitly ignore the error code on the basis that it would @@ -184,6 +184,36 @@ update(Hacl_Hash_MD5_state_t *state, uint8_t *buf, Py_ssize_t len) (void)Hacl_Hash_MD5_update(state, buf, (uint32_t)len); } +static void +md5_update_state_with_lock(MD5object *self, uint8_t *buf, Py_ssize_t len) +{ + Py_BEGIN_ALLOW_THREADS + PyMutex_Lock(&self->mutex); // unconditionally acquire a lock + _hacl_md5_update(self->hash_state, buf, len); + PyMutex_Unlock(&self->mutex); + Py_END_ALLOW_THREADS +} + +static void +md5_update_state_cond_lock(MD5object *self, uint8_t *buf, Py_ssize_t len) +{ + ENTER_HASHLIB(self); // conditionally acquire a lock + _hacl_md5_update(self->hash_state, buf, len); + LEAVE_HASHLIB(self); +} + +static inline void +md5_update_state(MD5object *self, uint8_t *buf, Py_ssize_t len) +{ + assert(buf != 0); + assert(len >= 0); + if (len != 0) { + len < HASHLIB_GIL_MINSIZE + ? md5_update_state_cond_lock(self, buf, len) + : md5_update_state_with_lock(self, buf, len); + } +} + /*[clinic input] MD5Type.update @@ -200,20 +230,7 @@ MD5Type_update_impl(MD5object *self, PyObject *obj) Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - - if (!self->use_mutex && buf.len >= HASHLIB_GIL_MINSIZE) { - self->use_mutex = true; - } - if (self->use_mutex) { - Py_BEGIN_ALLOW_THREADS - PyMutex_Lock(&self->mutex); - update(self->hash_state, buf.buf, buf.len); - PyMutex_Unlock(&self->mutex); - Py_END_ALLOW_THREADS - } else { - update(self->hash_state, buf.buf, buf.len); - } - + md5_update_state(self, buf.buf, buf.len); PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -319,11 +336,11 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, /* We do not initialize self->lock here as this is the constructor * where it is not yet possible to have concurrent access. */ Py_BEGIN_ALLOW_THREADS - update(new->hash_state, buf.buf, buf.len); + _hacl_md5_update(new->hash_state, buf.buf, buf.len); Py_END_ALLOW_THREADS } else { - update(new->hash_state, buf.buf, buf.len); + _hacl_md5_update(new->hash_state, buf.buf, buf.len); } PyBuffer_Release(&buf); } From 15a4f2fad0e621e9f07ff342e9188678165679bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 8 Jun 2025 16:42:18 +0200 Subject: [PATCH 04/39] refactor alloc() logic --- Modules/md5module.c | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/Modules/md5module.c b/Modules/md5module.c index 2cdf889589415c..94029922a6e41d 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -39,10 +39,7 @@ class MD5Type "MD5object *" "&PyType_Type" typedef struct { - PyObject_HEAD - // Prevents undefined behavior via multiple threads entering the C API. - bool use_mutex; - PyMutex mutex; + HASHLIB_OBJECT_HEAD Hacl_Hash_MD5_state_t *hash_state; } MD5object; @@ -308,30 +305,20 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, } MD5object *new; - Py_buffer buf; - - if (string) { - GET_BUFFER_VIEW_OR_ERROUT(string, &buf); - } - MD5State *st = md5_get_state(module); if ((new = newMD5object(st)) == NULL) { - if (string) { - PyBuffer_Release(&buf); - } return NULL; } new->hash_state = Hacl_Hash_MD5_malloc(); if (new->hash_state == NULL) { Py_DECREF(new); - if (string) { - PyBuffer_Release(&buf); - } return PyErr_NoMemory(); } if (string) { + Py_buffer buf; + GET_BUFFER_VIEW_OR_ERROR(string, &buf, goto error); if (buf.len >= HASHLIB_GIL_MINSIZE) { /* We do not initialize self->lock here as this is the constructor * where it is not yet possible to have concurrent access. */ @@ -346,6 +333,10 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, } return (PyObject *)new; + +error: + Py_XDECREF(new); + return NULL; } From 5cd828acdcfef753aee5eec7e13f07682af40f46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 8 Jun 2025 16:46:21 +0200 Subject: [PATCH 05/39] finalizing touches --- Modules/md5module.c | 114 +++++++++++++++++++++----------------------- 1 file changed, 55 insertions(+), 59 deletions(-) diff --git a/Modules/md5module.c b/Modules/md5module.c index 94029922a6e41d..69f787ca5ebc32 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -24,12 +24,6 @@ #include "hashlib.h" #include "pycore_strhex.h" // _Py_strhex() -/*[clinic input] -module _md5 -class MD5Type "MD5object *" "&PyType_Type" -[clinic start generated code]*/ -/*[clinic end generated code: output=da39a3ee5e6b4b0d input=6e5261719957a912]*/ - /* The MD5 block size and message digest sizes, in bytes */ #define MD5_BLOCKSIZE 64 @@ -37,62 +31,64 @@ class MD5Type "MD5object *" "&PyType_Type" #include "_hacl/Hacl_Hash_MD5.h" - typedef struct { HASHLIB_OBJECT_HEAD - Hacl_Hash_MD5_state_t *hash_state; + Hacl_Hash_MD5_state_t *state; } MD5object; #define _MD5object_CAST(op) ((MD5object *)(op)) -#include "clinic/md5module.c.h" - - typedef struct { - PyTypeObject* md5_type; -} MD5State; + PyTypeObject *md5_type; +} md5module_state; -static inline MD5State* -md5_get_state(PyObject *module) +static inline md5module_state * +get_md5module_state(PyObject *module) { void *state = PyModule_GetState(module); assert(state != NULL); - return (MD5State *)state; + return (md5module_state *)state; } +/*[clinic input] +module _md5 +class MD5Type "MD5object *" "&PyType_Type" +[clinic start generated code]*/ +/*[clinic end generated code: output=da39a3ee5e6b4b0d input=6e5261719957a912]*/ + +#include "clinic/md5module.c.h" + static MD5object * -newMD5object(MD5State * st) +newMD5object(md5module_state *st) { - MD5object *md5 = PyObject_GC_New(MD5object, st->md5_type); - if (!md5) { + MD5object *self = PyObject_GC_New(MD5object, st->md5_type); + if (self == NULL) { return NULL; } - HASHLIB_INIT_MUTEX(md5); - - PyObject_GC_Track(md5); - return md5; + HASHLIB_INIT_MUTEX(self); + PyObject_GC_Track(self); + return self; } /* Internal methods for a hash object */ static int -MD5_traverse(PyObject *ptr, visitproc visit, void *arg) +MD5_traverse(PyObject *op, visitproc visit, void *arg) { - Py_VISIT(Py_TYPE(ptr)); + Py_VISIT(Py_TYPE(op)); return 0; } static void MD5_dealloc(PyObject *op) { - MD5object *ptr = _MD5object_CAST(op); - Hacl_Hash_MD5_free(ptr->hash_state); + MD5object *self = _MD5object_CAST(op); + Hacl_Hash_MD5_free(self->state); PyTypeObject *tp = Py_TYPE(op); - PyObject_GC_UnTrack(ptr); - PyObject_GC_Del(ptr); + PyObject_GC_UnTrack(self); + PyObject_GC_Del(self); Py_DECREF(tp); } - /* External methods for a hash object */ /*[clinic input] @@ -107,28 +103,28 @@ static PyObject * MD5Type_copy_impl(MD5object *self, PyTypeObject *cls) /*[clinic end generated code: output=bf055e08244bf5ee input=d89087dcfb2a8620]*/ { - MD5State *st = PyType_GetModuleState(cls); + md5module_state *st = PyType_GetModuleState(cls); - MD5object *newobj; - if ((newobj = newMD5object(st)) == NULL) { + MD5object *copy = newMD5object(st); + if (copy == NULL) { return NULL; } ENTER_HASHLIB(self); - newobj->hash_state = Hacl_Hash_MD5_copy(self->hash_state); + copy->state = Hacl_Hash_MD5_copy(self->state); LEAVE_HASHLIB(self); - if (newobj->hash_state == NULL) { - Py_DECREF(self); + if (copy->state == NULL) { + Py_DECREF(copy); return PyErr_NoMemory(); } - return (PyObject *)newobj; + return (PyObject *)copy; } static void md5_digest_compute_cond_lock(MD5object *self, uint8_t *digest) { ENTER_HASHLIB(self); - Hacl_Hash_MD5_digest(self->hash_state, digest); + Hacl_Hash_MD5_digest(self->state, digest); LEAVE_HASHLIB(self); } @@ -186,7 +182,7 @@ md5_update_state_with_lock(MD5object *self, uint8_t *buf, Py_ssize_t len) { Py_BEGIN_ALLOW_THREADS PyMutex_Lock(&self->mutex); // unconditionally acquire a lock - _hacl_md5_update(self->hash_state, buf, len); + _hacl_md5_update(self->state, buf, len); PyMutex_Unlock(&self->mutex); Py_END_ALLOW_THREADS } @@ -195,7 +191,7 @@ static void md5_update_state_cond_lock(MD5object *self, uint8_t *buf, Py_ssize_t len) { ENTER_HASHLIB(self); // conditionally acquire a lock - _hacl_md5_update(self->hash_state, buf, len); + _hacl_md5_update(self->state, buf, len); LEAVE_HASHLIB(self); } @@ -237,7 +233,7 @@ static PyMethodDef MD5_methods[] = { MD5TYPE_DIGEST_METHODDEF MD5TYPE_HEXDIGEST_METHODDEF MD5TYPE_UPDATE_METHODDEF - {NULL, NULL} /* sentinel */ + {NULL, NULL} /* sentinel */ }; static PyObject * @@ -262,7 +258,7 @@ static PyGetSetDef MD5_getseters[] = { {"block_size", MD5_get_block_size, NULL, NULL, NULL}, {"name", MD5_get_name, NULL, NULL, NULL}, {"digest_size", md5_get_digest_size, NULL, NULL, NULL}, - {NULL} /* Sentinel */ + {NULL} /* sentinel */ }; static PyType_Slot md5_type_slots[] = { @@ -270,12 +266,12 @@ static PyType_Slot md5_type_slots[] = { {Py_tp_methods, MD5_methods}, {Py_tp_getset, MD5_getseters}, {Py_tp_traverse, MD5_traverse}, - {0,0} + {0, 0} }; static PyType_Spec md5_type_spec = { .name = "_md5.md5", - .basicsize = sizeof(MD5object), + .basicsize = sizeof(MD5object), .flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_DISALLOW_INSTANTIATION | Py_TPFLAGS_IMMUTABLETYPE | Py_TPFLAGS_HAVE_GC), .slots = md5_type_slots @@ -304,15 +300,15 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, return NULL; } - MD5object *new; - MD5State *st = md5_get_state(module); - if ((new = newMD5object(st)) == NULL) { + md5module_state *st = get_md5module_state(module); + MD5object *self = newMD5object(st); + if (self == NULL) { return NULL; } - new->hash_state = Hacl_Hash_MD5_malloc(); - if (new->hash_state == NULL) { - Py_DECREF(new); + self->state = Hacl_Hash_MD5_malloc(); + if (self->state == NULL) { + Py_DECREF(self); return PyErr_NoMemory(); } @@ -323,19 +319,19 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, /* We do not initialize self->lock here as this is the constructor * where it is not yet possible to have concurrent access. */ Py_BEGIN_ALLOW_THREADS - _hacl_md5_update(new->hash_state, buf.buf, buf.len); + _hacl_md5_update(self->state, buf.buf, buf.len); Py_END_ALLOW_THREADS } else { - _hacl_md5_update(new->hash_state, buf.buf, buf.len); + _hacl_md5_update(self->state, buf.buf, buf.len); } PyBuffer_Release(&buf); } - return (PyObject *)new; + return (PyObject *)self; error: - Py_XDECREF(new); + Py_XDECREF(self); return NULL; } @@ -344,13 +340,13 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, static struct PyMethodDef MD5_functions[] = { _MD5_MD5_METHODDEF - {NULL, NULL} /* Sentinel */ + {NULL, NULL} /* sentinel */ }; static int _md5_traverse(PyObject *module, visitproc visit, void *arg) { - MD5State *state = md5_get_state(module); + md5module_state *state = get_md5module_state(module); Py_VISIT(state->md5_type); return 0; } @@ -358,7 +354,7 @@ _md5_traverse(PyObject *module, visitproc visit, void *arg) static int _md5_clear(PyObject *module) { - MD5State *state = md5_get_state(module); + md5module_state *state = get_md5module_state(module); Py_CLEAR(state->md5_type); return 0; } @@ -373,7 +369,7 @@ _md5_free(void *module) static int md5_exec(PyObject *m) { - MD5State *st = md5_get_state(m); + md5module_state *st = get_md5module_state(m); st->md5_type = (PyTypeObject *)PyType_FromModuleAndSpec( m, &md5_type_spec, NULL); @@ -399,7 +395,7 @@ static PyModuleDef_Slot _md5_slots[] = { static struct PyModuleDef _md5module = { PyModuleDef_HEAD_INIT, .m_name = "_md5", - .m_size = sizeof(MD5State), + .m_size = sizeof(md5module_state), .m_methods = MD5_functions, .m_slots = _md5_slots, .m_traverse = _md5_traverse, From 63db1de8317c569cb6f251c3546be9b98c71bd4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 15 Jun 2025 10:56:34 +0200 Subject: [PATCH 06/39] correct mutex usage --- Modules/hashlib.h | 51 ++++++++++++++++++++++++++------------------- Modules/md5module.c | 23 +++++++++++++------- 2 files changed, 45 insertions(+), 29 deletions(-) diff --git a/Modules/hashlib.h b/Modules/hashlib.h index 23475afb683d93..fe12acb27ce036 100644 --- a/Modules/hashlib.h +++ b/Modules/hashlib.h @@ -50,33 +50,42 @@ #include "pythread.h" -#define HASHLIB_OBJECT_HEAD \ - PyObject_HEAD \ - /* prevent undefined behavior via multiple - * threads entering the C API */ \ - bool use_mutex; \ +#define HASHLIB_LOCK_HEAD \ + /* + * Attributes to prevent undefined behaviors + * via multiple threads entering the C API. + */ \ + bool use_mutex; \ PyMutex mutex; -#define ENTER_HASHLIB(obj) \ - if ((obj)->use_mutex) { \ - PyMutex_Lock(&(obj)->mutex); \ - } -#define LEAVE_HASHLIB(obj) \ - if ((obj)->use_mutex) { \ - PyMutex_Unlock(&(obj)->mutex); \ - } +#define HASHLIB_SET_MUTEX_POLICY(OBJ, VALUE) \ + _Py_atomic_store_int_relaxed((int *)&(OBJ)->use_mutex, (int)(VALUE)) + +#define ENTER_HASHLIB(OBJ) \ + do { \ + if (_Py_atomic_load_int_relaxed((const int *)&(OBJ)->use_mutex)) { \ + PyMutex_Lock(&(OBJ)->mutex); \ + } \ + } while (0) + +#define LEAVE_HASHLIB(OBJ) \ + do { \ + if (_Py_atomic_load_int_relaxed((const int *)&(OBJ)->use_mutex)) { \ + PyMutex_Unlock(&(OBJ)->mutex); \ + } \ + } while (0) #ifdef Py_GIL_DISABLED -#define HASHLIB_INIT_MUTEX(obj) \ - do { \ - (obj)->mutex = (PyMutex){0}; \ - (obj)->use_mutex = true; \ +#define HASHLIB_INIT_MUTEX(OBJ) \ + do { \ + (OBJ)->mutex = (PyMutex){0}; \ + (OBJ)->use_mutex = true; \ } while (0) #else -#define HASHLIB_INIT_MUTEX(obj) \ - do { \ - (obj)->mutex = (PyMutex){0}; \ - (obj)->use_mutex = false; \ +#define HASHLIB_INIT_MUTEX(OBJ) \ + do { \ + (OBJ)->mutex = (PyMutex){0}; \ + (OBJ)->use_mutex = false; \ } while (0) #endif diff --git a/Modules/md5module.c b/Modules/md5module.c index 69f787ca5ebc32..8500d933f91f02 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -32,7 +32,8 @@ #include "_hacl/Hacl_Hash_MD5.h" typedef struct { - HASHLIB_OBJECT_HEAD + PyObject_HEAD + HASHLIB_LOCK_HEAD Hacl_Hash_MD5_state_t *state; } MD5object; @@ -181,7 +182,7 @@ static void md5_update_state_with_lock(MD5object *self, uint8_t *buf, Py_ssize_t len) { Py_BEGIN_ALLOW_THREADS - PyMutex_Lock(&self->mutex); // unconditionally acquire a lock + PyMutex_Lock(&self->mutex); // unconditionally acquire a lock _hacl_md5_update(self->state, buf, len); PyMutex_Unlock(&self->mutex); Py_END_ALLOW_THREADS @@ -190,7 +191,7 @@ md5_update_state_with_lock(MD5object *self, uint8_t *buf, Py_ssize_t len) static void md5_update_state_cond_lock(MD5object *self, uint8_t *buf, Py_ssize_t len) { - ENTER_HASHLIB(self); // conditionally acquire a lock + ENTER_HASHLIB(self); // conditionally acquire a lock _hacl_md5_update(self->state, buf, len); LEAVE_HASHLIB(self); } @@ -200,10 +201,16 @@ md5_update_state(MD5object *self, uint8_t *buf, Py_ssize_t len) { assert(buf != 0); assert(len >= 0); - if (len != 0) { - len < HASHLIB_GIL_MINSIZE - ? md5_update_state_cond_lock(self, buf, len) - : md5_update_state_with_lock(self, buf, len); + if (len == 0) { + return; + } + if (len < HASHLIB_GIL_MINSIZE) { + md5_update_state_cond_lock(self, buf, len); + } + else { + HASHLIB_SET_MUTEX_POLICY(self, 1); + md5_update_state_with_lock(self, buf, len); + HASHLIB_SET_MUTEX_POLICY(self, 0); } } @@ -316,7 +323,7 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, Py_buffer buf; GET_BUFFER_VIEW_OR_ERROR(string, &buf, goto error); if (buf.len >= HASHLIB_GIL_MINSIZE) { - /* We do not initialize self->lock here as this is the constructor + /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ Py_BEGIN_ALLOW_THREADS _hacl_md5_update(self->state, buf.buf, buf.len); From ea033a3f02b4318463b6fb976912bdc2f9ab7e81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 15 Jun 2025 11:42:41 +0200 Subject: [PATCH 07/39] Revert 5cd828acdcfef753aee5eec7e13f07682af40f46 --- Modules/md5module.c | 114 +++++++++++++++++++++++--------------------- 1 file changed, 59 insertions(+), 55 deletions(-) diff --git a/Modules/md5module.c b/Modules/md5module.c index 8500d933f91f02..db25c2bb971a05 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -24,6 +24,12 @@ #include "hashlib.h" #include "pycore_strhex.h" // _Py_strhex() +/*[clinic input] +module _md5 +class MD5Type "MD5object *" "&PyType_Type" +[clinic start generated code]*/ +/*[clinic end generated code: output=da39a3ee5e6b4b0d input=6e5261719957a912]*/ + /* The MD5 block size and message digest sizes, in bytes */ #define MD5_BLOCKSIZE 64 @@ -31,65 +37,63 @@ #include "_hacl/Hacl_Hash_MD5.h" + typedef struct { PyObject_HEAD HASHLIB_LOCK_HEAD - Hacl_Hash_MD5_state_t *state; + Hacl_Hash_MD5_state_t *hash_state; } MD5object; #define _MD5object_CAST(op) ((MD5object *)(op)) +#include "clinic/md5module.c.h" + + typedef struct { - PyTypeObject *md5_type; -} md5module_state; + PyTypeObject* md5_type; +} MD5State; -static inline md5module_state * -get_md5module_state(PyObject *module) +static inline MD5State* +md5_get_state(PyObject *module) { void *state = PyModule_GetState(module); assert(state != NULL); - return (md5module_state *)state; + return (MD5State *)state; } -/*[clinic input] -module _md5 -class MD5Type "MD5object *" "&PyType_Type" -[clinic start generated code]*/ -/*[clinic end generated code: output=da39a3ee5e6b4b0d input=6e5261719957a912]*/ - -#include "clinic/md5module.c.h" - static MD5object * -newMD5object(md5module_state *st) +newMD5object(MD5State * st) { - MD5object *self = PyObject_GC_New(MD5object, st->md5_type); - if (self == NULL) { + MD5object *md5 = PyObject_GC_New(MD5object, st->md5_type); + if (!md5) { return NULL; } - HASHLIB_INIT_MUTEX(self); - PyObject_GC_Track(self); - return self; + HASHLIB_INIT_MUTEX(md5); + + PyObject_GC_Track(md5); + return md5; } /* Internal methods for a hash object */ static int -MD5_traverse(PyObject *op, visitproc visit, void *arg) +MD5_traverse(PyObject *ptr, visitproc visit, void *arg) { - Py_VISIT(Py_TYPE(op)); + Py_VISIT(Py_TYPE(ptr)); return 0; } static void MD5_dealloc(PyObject *op) { - MD5object *self = _MD5object_CAST(op); - Hacl_Hash_MD5_free(self->state); + MD5object *ptr = _MD5object_CAST(op); + Hacl_Hash_MD5_free(ptr->hash_state); PyTypeObject *tp = Py_TYPE(op); - PyObject_GC_UnTrack(self); - PyObject_GC_Del(self); + PyObject_GC_UnTrack(ptr); + PyObject_GC_Del(ptr); Py_DECREF(tp); } + /* External methods for a hash object */ /*[clinic input] @@ -104,28 +108,28 @@ static PyObject * MD5Type_copy_impl(MD5object *self, PyTypeObject *cls) /*[clinic end generated code: output=bf055e08244bf5ee input=d89087dcfb2a8620]*/ { - md5module_state *st = PyType_GetModuleState(cls); + MD5State *st = PyType_GetModuleState(cls); - MD5object *copy = newMD5object(st); - if (copy == NULL) { + MD5object *newobj; + if ((newobj = newMD5object(st)) == NULL) { return NULL; } ENTER_HASHLIB(self); - copy->state = Hacl_Hash_MD5_copy(self->state); + newobj->hash_state = Hacl_Hash_MD5_copy(self->hash_state); LEAVE_HASHLIB(self); - if (copy->state == NULL) { - Py_DECREF(copy); + if (newobj->hash_state == NULL) { + Py_DECREF(self); return PyErr_NoMemory(); } - return (PyObject *)copy; + return (PyObject *)newobj; } static void md5_digest_compute_cond_lock(MD5object *self, uint8_t *digest) { ENTER_HASHLIB(self); - Hacl_Hash_MD5_digest(self->state, digest); + Hacl_Hash_MD5_digest(self->hash_state, digest); LEAVE_HASHLIB(self); } @@ -183,7 +187,7 @@ md5_update_state_with_lock(MD5object *self, uint8_t *buf, Py_ssize_t len) { Py_BEGIN_ALLOW_THREADS PyMutex_Lock(&self->mutex); // unconditionally acquire a lock - _hacl_md5_update(self->state, buf, len); + _hacl_md5_update(self->hash_state, buf, len); PyMutex_Unlock(&self->mutex); Py_END_ALLOW_THREADS } @@ -192,7 +196,7 @@ static void md5_update_state_cond_lock(MD5object *self, uint8_t *buf, Py_ssize_t len) { ENTER_HASHLIB(self); // conditionally acquire a lock - _hacl_md5_update(self->state, buf, len); + _hacl_md5_update(self->hash_state, buf, len); LEAVE_HASHLIB(self); } @@ -240,7 +244,7 @@ static PyMethodDef MD5_methods[] = { MD5TYPE_DIGEST_METHODDEF MD5TYPE_HEXDIGEST_METHODDEF MD5TYPE_UPDATE_METHODDEF - {NULL, NULL} /* sentinel */ + {NULL, NULL} /* sentinel */ }; static PyObject * @@ -265,7 +269,7 @@ static PyGetSetDef MD5_getseters[] = { {"block_size", MD5_get_block_size, NULL, NULL, NULL}, {"name", MD5_get_name, NULL, NULL, NULL}, {"digest_size", md5_get_digest_size, NULL, NULL, NULL}, - {NULL} /* sentinel */ + {NULL} /* Sentinel */ }; static PyType_Slot md5_type_slots[] = { @@ -273,12 +277,12 @@ static PyType_Slot md5_type_slots[] = { {Py_tp_methods, MD5_methods}, {Py_tp_getset, MD5_getseters}, {Py_tp_traverse, MD5_traverse}, - {0, 0} + {0,0} }; static PyType_Spec md5_type_spec = { .name = "_md5.md5", - .basicsize = sizeof(MD5object), + .basicsize = sizeof(MD5object), .flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_DISALLOW_INSTANTIATION | Py_TPFLAGS_IMMUTABLETYPE | Py_TPFLAGS_HAVE_GC), .slots = md5_type_slots @@ -307,15 +311,15 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, return NULL; } - md5module_state *st = get_md5module_state(module); - MD5object *self = newMD5object(st); - if (self == NULL) { + MD5object *new; + MD5State *st = md5_get_state(module); + if ((new = newMD5object(st)) == NULL) { return NULL; } - self->state = Hacl_Hash_MD5_malloc(); - if (self->state == NULL) { - Py_DECREF(self); + new->hash_state = Hacl_Hash_MD5_malloc(); + if (new->hash_state == NULL) { + Py_DECREF(new); return PyErr_NoMemory(); } @@ -326,19 +330,19 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ Py_BEGIN_ALLOW_THREADS - _hacl_md5_update(self->state, buf.buf, buf.len); + _hacl_md5_update(new->hash_state, buf.buf, buf.len); Py_END_ALLOW_THREADS } else { - _hacl_md5_update(self->state, buf.buf, buf.len); + _hacl_md5_update(new->hash_state, buf.buf, buf.len); } PyBuffer_Release(&buf); } - return (PyObject *)self; + return (PyObject *)new; error: - Py_XDECREF(self); + Py_XDECREF(new); return NULL; } @@ -347,13 +351,13 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, static struct PyMethodDef MD5_functions[] = { _MD5_MD5_METHODDEF - {NULL, NULL} /* sentinel */ + {NULL, NULL} /* Sentinel */ }; static int _md5_traverse(PyObject *module, visitproc visit, void *arg) { - md5module_state *state = get_md5module_state(module); + MD5State *state = md5_get_state(module); Py_VISIT(state->md5_type); return 0; } @@ -361,7 +365,7 @@ _md5_traverse(PyObject *module, visitproc visit, void *arg) static int _md5_clear(PyObject *module) { - md5module_state *state = get_md5module_state(module); + MD5State *state = md5_get_state(module); Py_CLEAR(state->md5_type); return 0; } @@ -376,7 +380,7 @@ _md5_free(void *module) static int md5_exec(PyObject *m) { - md5module_state *st = get_md5module_state(m); + MD5State *st = md5_get_state(m); st->md5_type = (PyTypeObject *)PyType_FromModuleAndSpec( m, &md5_type_spec, NULL); @@ -402,7 +406,7 @@ static PyModuleDef_Slot _md5_slots[] = { static struct PyModuleDef _md5module = { PyModuleDef_HEAD_INIT, .m_name = "_md5", - .m_size = sizeof(md5module_state), + .m_size = sizeof(MD5State), .m_methods = MD5_functions, .m_slots = _md5_slots, .m_traverse = _md5_traverse, From 77baa67c9e4b4cbb9bdec938360dc241b0f58034 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 15 Jun 2025 11:44:38 +0200 Subject: [PATCH 08/39] revert some constructor changes --- Modules/md5module.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/Modules/md5module.c b/Modules/md5module.c index db25c2bb971a05..a05fd9d591fdac 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -312,20 +312,30 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, } MD5object *new; + Py_buffer buf; + + if (string) { + GET_BUFFER_VIEW_OR_ERROUT(string, &buf); + } + MD5State *st = md5_get_state(module); if ((new = newMD5object(st)) == NULL) { + if (string) { + PyBuffer_Release(&buf); + } return NULL; } new->hash_state = Hacl_Hash_MD5_malloc(); if (new->hash_state == NULL) { Py_DECREF(new); + if (string) { + PyBuffer_Release(&buf); + } return PyErr_NoMemory(); } if (string) { - Py_buffer buf; - GET_BUFFER_VIEW_OR_ERROR(string, &buf, goto error); if (buf.len >= HASHLIB_GIL_MINSIZE) { /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ @@ -340,10 +350,6 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, } return (PyObject *)new; - -error: - Py_XDECREF(new); - return NULL; } From 902759fb854c40aeefb56f2d274ca7004a8ec476 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, 16 Jun 2025 12:15:23 +0200 Subject: [PATCH 09/39] unconditionally lock when performing HASH updates --- Lib/test/support/hashlib_helper.py | 19 -- Lib/test/test_hashlib.py | 36 +--- Lib/test/test_hmac.py | 24 --- Modules/_hashopenssl.c | 92 +++------ Modules/blake2module.c | 47 ++--- Modules/hashlib.h | 39 +--- Modules/hmacmodule.c | 307 ++++++++--------------------- Modules/md5module.c | 89 +++------ Modules/sha1module.c | 52 ++--- Modules/sha2module.c | 125 ++++-------- Modules/sha3module.c | 42 ++-- 11 files changed, 228 insertions(+), 644 deletions(-) diff --git a/Lib/test/support/hashlib_helper.py b/Lib/test/support/hashlib_helper.py index 7032257b06877a..c0d5da042d8bcf 100644 --- a/Lib/test/support/hashlib_helper.py +++ b/Lib/test/support/hashlib_helper.py @@ -308,22 +308,3 @@ def sha3_384(self): @property def sha3_512(self): return self._find_constructor_in("_sha3","sha3_512") - - -def find_gil_minsize(modules_names, default=2048): - """Get the largest GIL_MINSIZE value for the given cryptographic modules. - - The valid module names are the following: - - - _hashlib - - _md5, _sha1, _sha2, _sha3, _blake2 - - _hmac - """ - sizes = [] - for module_name in modules_names: - try: - module = importlib.import_module(module_name) - except ImportError: - continue - sizes.append(getattr(module, '_GIL_MINSIZE', default)) - return max(sizes, default=default) diff --git a/Lib/test/test_hashlib.py b/Lib/test/test_hashlib.py index b83ae181718b7a..a9469504b504ed 100644 --- a/Lib/test/test_hashlib.py +++ b/Lib/test/test_hashlib.py @@ -409,7 +409,7 @@ def test_large_update(self): aas = b'a' * 128 bees = b'b' * 127 cees = b'c' * 126 - dees = b'd' * 2048 # HASHLIB_GIL_MINSIZE + dees = b'd' * 2048 for cons in self.hash_constructors: m1 = cons(usedforsecurity=False) @@ -990,40 +990,6 @@ def test_case_shake256_vector(self): for msg, md in read_vectors('shake_256'): self.check('shake_256', msg, md, True) - def test_gil(self): - # Check things work fine with an input larger than the size required - # for multithreaded operation. Currently, all cryptographic modules - # have the same constant value (2048) but in the future it might not - # be the case. - mods = ['_md5', '_sha1', '_sha2', '_sha3', '_blake2', '_hashlib'] - gil_minsize = hashlib_helper.find_gil_minsize(mods) - for cons in self.hash_constructors: - # constructors belong to one of the above modules - m = cons(usedforsecurity=False) - m.update(b'1') - m.update(b'#' * gil_minsize) - m.update(b'1') - - m = cons(b'x' * gil_minsize, usedforsecurity=False) - m.update(b'1') - - def test_sha256_gil(self): - gil_minsize = hashlib_helper.find_gil_minsize(['_sha2', '_hashlib']) - m = hashlib.sha256() - m.update(b'1') - m.update(b'#' * gil_minsize) - m.update(b'1') - self.assertEqual( - m.hexdigest(), - '1cfceca95989f51f658e3f3ffe7f1cd43726c9e088c13ee10b46f57cef135b94' - ) - - m = hashlib.sha256(b'1' + b'#' * gil_minsize + b'1') - self.assertEqual( - m.hexdigest(), - '1cfceca95989f51f658e3f3ffe7f1cd43726c9e088c13ee10b46f57cef135b94' - ) - @threading_helper.reap_threads @threading_helper.requires_working_threading() def test_threaded_hashing(self): diff --git a/Lib/test/test_hmac.py b/Lib/test/test_hmac.py index ff6e1bce0ef801..d1f4662adbb618 100644 --- a/Lib/test/test_hmac.py +++ b/Lib/test/test_hmac.py @@ -1133,11 +1133,6 @@ def HMAC(self, key, msg=None): """Create a HMAC object.""" raise NotImplementedError - @property - def gil_minsize(self): - """Get the maximal input length for the GIL to be held.""" - raise NotImplementedError - def check_update(self, key, chunks): chunks = list(chunks) msg = b''.join(chunks) @@ -1155,13 +1150,6 @@ def test_update(self): with self.subTest(key=key, msg=msg): self.check_update(key, [msg]) - def test_update_large(self): - gil_minsize = self.gil_minsize - key = random.randbytes(16) - top = random.randbytes(gil_minsize + 1) - bot = random.randbytes(gil_minsize + 1) - self.check_update(key, [top, bot]) - def test_update_exceptions(self): h = self.HMAC(b"key") for msg in ['invalid msg', 123, (), []]: @@ -1175,10 +1163,6 @@ class PyUpdateTestCase(PyModuleMixin, UpdateTestCaseMixin, unittest.TestCase): def HMAC(self, key, msg=None): return self.hmac.HMAC(key, msg, digestmod='sha256') - @property - def gil_minsize(self): - return sha2._GIL_MINSIZE - @hashlib_helper.requires_openssl_hashdigest('sha256') class OpenSSLUpdateTestCase(UpdateTestCaseMixin, unittest.TestCase): @@ -1186,10 +1170,6 @@ class OpenSSLUpdateTestCase(UpdateTestCaseMixin, unittest.TestCase): def HMAC(self, key, msg=None): return _hashlib.hmac_new(key, msg, digestmod='sha256') - @property - def gil_minsize(self): - return _hashlib._GIL_MINSIZE - class BuiltinUpdateTestCase(BuiltinModuleMixin, UpdateTestCaseMixin, unittest.TestCase): @@ -1199,10 +1179,6 @@ def HMAC(self, key, msg=None): # are still built, making it possible to use SHA-2 hashes. return self.hmac.new(key, msg, digestmod='sha256') - @property - def gil_minsize(self): - return self.hmac._GIL_MINSIZE - class CopyBaseTestCase: diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 50cf3c57491049..d4c7cc02ccbea3 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -611,9 +611,9 @@ static int _hashlib_HASH_copy_locked(HASHobject *self, EVP_MD_CTX *new_ctx_p) { int result; - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); result = EVP_MD_CTX_copy(new_ctx_p, self->ctx); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); return result; } @@ -733,29 +733,16 @@ static PyObject * _hashlib_HASH_update_impl(HASHobject *self, PyObject *obj) /*[clinic end generated code: output=62ad989754946b86 input=aa1ce20e3f92ceb6]*/ { - int result; + int rc; Py_buffer view; - GET_BUFFER_VIEW_OR_ERROUT(obj, &view); - - if (!self->use_mutex && view.len >= HASHLIB_GIL_MINSIZE) { - self->use_mutex = true; - } - if (self->use_mutex) { - Py_BEGIN_ALLOW_THREADS - PyMutex_Lock(&self->mutex); - result = _hashlib_HASH_hash(self, view.buf, view.len); - PyMutex_Unlock(&self->mutex); - Py_END_ALLOW_THREADS - } else { - result = _hashlib_HASH_hash(self, view.buf, view.len); - } - + Py_BEGIN_ALLOW_THREADS + HASHLIB_ACQUIRE_LOCK(self); + rc = _hashlib_HASH_hash(self, view.buf, view.len); + HASHLIB_RELEASE_LOCK(self); + Py_END_ALLOW_THREADS PyBuffer_Release(&view); - - if (result == -1) - return NULL; - Py_RETURN_NONE; + return rc < 0 ? NULL : Py_None; } static PyMethodDef HASH_methods[] = { @@ -1060,15 +1047,11 @@ _hashlib_HASH(PyObject *module, const char *digestname, PyObject *data_obj, } if (view.buf && view.len) { - if (view.len >= HASHLIB_GIL_MINSIZE) { - /* We do not initialize self->lock here as this is the constructor - * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - result = _hashlib_HASH_hash(self, view.buf, view.len); - Py_END_ALLOW_THREADS - } else { + /* Do not use self->mutex here as this is the constructor + * where it is not yet possible to have concurrent access. */ + Py_BEGIN_ALLOW_THREADS result = _hashlib_HASH_hash(self, view.buf, view.len); - } + Py_END_ALLOW_THREADS if (result == -1) { assert(PyErr_Occurred()); Py_CLEAR(self); @@ -1701,7 +1684,7 @@ _hashlib_hmac_new_impl(PyObject *module, Py_buffer *key, PyObject *msg_obj, HASHLIB_INIT_MUTEX(self); if ((msg_obj != NULL) && (msg_obj != Py_None)) { - if (!_hmac_update(self, msg_obj)) { + if (_hmac_update(self, msg_obj) < 0) { goto error; } } @@ -1718,9 +1701,9 @@ static int locked_HMAC_CTX_copy(HMAC_CTX *new_ctx_p, HMACobject *self) { int result; - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); result = HMAC_CTX_copy(new_ctx_p, self->ctx); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); return result; } @@ -1743,35 +1726,26 @@ _hashlib_hmac_digest_size(HMACobject *self) static int _hmac_update(HMACobject *self, PyObject *obj) { - int r; + int r = 1; Py_buffer view = {0}; - GET_BUFFER_VIEW_OR_ERROR(obj, &view, return 0); - - if (!self->use_mutex && view.len >= HASHLIB_GIL_MINSIZE) { - self->use_mutex = true; - } - if (self->use_mutex) { + GET_BUFFER_VIEW_OR_ERROR(obj, &view, return -1); + if (view.len > 0) { Py_BEGIN_ALLOW_THREADS - PyMutex_Lock(&self->mutex); - r = HMAC_Update(self->ctx, - (const unsigned char *)view.buf, - (size_t)view.len); - PyMutex_Unlock(&self->mutex); + HASHLIB_ACQUIRE_LOCK(self); + r = HMAC_Update(self->ctx, + (const unsigned char *)view.buf, + (size_t)view.len); + HASHLIB_RELEASE_LOCK(self); Py_END_ALLOW_THREADS - } else { - r = HMAC_Update(self->ctx, - (const unsigned char *)view.buf, - (size_t)view.len); } - PyBuffer_Release(&view); if (r == 0) { notify_ssl_error_occurred(); - return 0; + return -1; } - return 1; + return 0; } /*[clinic input] @@ -1845,7 +1819,7 @@ static PyObject * _hashlib_HMAC_update_impl(HMACobject *self, PyObject *msg) /*[clinic end generated code: output=f31f0ace8c625b00 input=1829173bb3cfd4e6]*/ { - if (!_hmac_update(self, msg)) { + if (_hmac_update(self, msg) < 0) { return NULL; } Py_RETURN_NONE; @@ -2412,17 +2386,6 @@ hashlib_exception(PyObject *module) return 0; } -static int -hashlib_constants(PyObject *module) -{ - if (PyModule_AddIntConstant(module, "_GIL_MINSIZE", - HASHLIB_GIL_MINSIZE) < 0) - { - return -1; - } - return 0; -} - static PyModuleDef_Slot hashlib_slots[] = { {Py_mod_exec, hashlib_init_hashtable}, {Py_mod_exec, hashlib_init_HASH_type}, @@ -2431,7 +2394,6 @@ static PyModuleDef_Slot hashlib_slots[] = { {Py_mod_exec, hashlib_md_meth_names}, {Py_mod_exec, hashlib_init_constructors}, {Py_mod_exec, hashlib_exception}, - {Py_mod_exec, hashlib_constants}, {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED}, {Py_mod_gil, Py_MOD_GIL_NOT_USED}, {0, NULL} diff --git a/Modules/blake2module.c b/Modules/blake2module.c index 07aa89f573f05f..5f02344ab6e285 100644 --- a/Modules/blake2module.c +++ b/Modules/blake2module.c @@ -229,8 +229,6 @@ blake2_exec(PyObject *m) // good a place as any to probe the CPU flags. detect_cpu_features(&st->flags); - ADD_INT_CONST("_GIL_MINSIZE", HASHLIB_GIL_MINSIZE); - st->blake2b_type = (PyTypeObject *)PyType_FromModuleAndSpec( m, &blake2b_type_spec, NULL); @@ -422,7 +420,7 @@ new_Blake2Object(PyTypeObject *type) } while (0) static void -update(Blake2Object *self, uint8_t *buf, Py_ssize_t len) +blake2_update_state_unlocked(Blake2Object *self, uint8_t *buf, Py_ssize_t len) { switch (self->impl) { // These need to be ifdef'd out otherwise it's an unresolved symbol at @@ -634,15 +632,11 @@ py_blake2b_or_s_new(PyTypeObject *type, PyObject *data, int digest_size, /* Process initial data if any. */ if (data != NULL) { GET_BUFFER_VIEW_OR_ERROR(data, &buf, goto error); - - if (buf.len >= HASHLIB_GIL_MINSIZE) { + if (buf.len > 0) { Py_BEGIN_ALLOW_THREADS - update(self, buf.buf, buf.len); + blake2_update_state_unlocked(self, buf.buf, buf.len); Py_END_ALLOW_THREADS } - else { - update(self, buf.buf, buf.len); - } PyBuffer_Release(&buf); } @@ -793,9 +787,9 @@ _blake2_blake2b_copy_impl(Blake2Object *self) return NULL; } - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); rc = blake2_blake2b_copy_locked(self, cpy); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); if (rc < 0) { Py_DECREF(cpy); return NULL; @@ -817,24 +811,15 @@ _blake2_blake2b_update_impl(Blake2Object *self, PyObject *data) /*[clinic end generated code: output=99330230068e8c99 input=ffc4aa6a6a225d31]*/ { Py_buffer buf; - GET_BUFFER_VIEW_OR_ERROUT(data, &buf); - - if (!self->use_mutex && buf.len >= HASHLIB_GIL_MINSIZE) { - self->use_mutex = true; - } - if (self->use_mutex) { + if (buf.len > 0) { Py_BEGIN_ALLOW_THREADS - PyMutex_Lock(&self->mutex); - update(self, buf.buf, buf.len); - PyMutex_Unlock(&self->mutex); + HASHLIB_ACQUIRE_LOCK(self); + blake2_update_state_unlocked(self, buf.buf, buf.len); + HASHLIB_RELEASE_LOCK(self); Py_END_ALLOW_THREADS - } else { - update(self, buf.buf, buf.len); } - PyBuffer_Release(&buf); - Py_RETURN_NONE; } @@ -849,9 +834,9 @@ _blake2_blake2b_digest_impl(Blake2Object *self) /*[clinic end generated code: output=31ab8ad477f4a2f7 input=7d21659e9c5fff02]*/ { uint8_t digest[HACL_HASH_BLAKE2B_OUT_BYTES]; - - ENTER_HASHLIB(self); uint8_t digest_length = 0; + + HASHLIB_ACQUIRE_LOCK(self); switch (self->impl) { #if HACL_CAN_COMPILE_SIMD256 case Blake2b_256: @@ -870,9 +855,10 @@ _blake2_blake2b_digest_impl(Blake2Object *self) digest_length = Hacl_Hash_Blake2s_digest(self->blake2s_state, digest); break; default: + HASHLIB_RELEASE_LOCK(self); Py_UNREACHABLE(); } - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); return PyBytes_FromStringAndSize((const char *)digest, digest_length); } @@ -887,9 +873,9 @@ _blake2_blake2b_hexdigest_impl(Blake2Object *self) /*[clinic end generated code: output=5ef54b138db6610a input=76930f6946351f56]*/ { uint8_t digest[HACL_HASH_BLAKE2B_OUT_BYTES]; - - ENTER_HASHLIB(self); uint8_t digest_length = 0; + + HASHLIB_ACQUIRE_LOCK(self); switch (self->impl) { #if HACL_CAN_COMPILE_SIMD256 case Blake2b_256: @@ -908,9 +894,10 @@ _blake2_blake2b_hexdigest_impl(Blake2Object *self) digest_length = Hacl_Hash_Blake2s_digest(self->blake2s_state, digest); break; default: + HASHLIB_RELEASE_LOCK(self); Py_UNREACHABLE(); } - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); return _Py_strhex((const char *)digest, digest_length); } diff --git a/Modules/hashlib.h b/Modules/hashlib.h index fe12acb27ce036..43d53442666fb7 100644 --- a/Modules/hashlib.h +++ b/Modules/hashlib.h @@ -50,48 +50,17 @@ #include "pythread.h" -#define HASHLIB_LOCK_HEAD \ - /* - * Attributes to prevent undefined behaviors - * via multiple threads entering the C API. - */ \ - bool use_mutex; \ +#define HASHLIB_LOCK_HEAD \ + /* Guard against race conditions during incremental update(). */ \ PyMutex mutex; -#define HASHLIB_SET_MUTEX_POLICY(OBJ, VALUE) \ - _Py_atomic_store_int_relaxed((int *)&(OBJ)->use_mutex, (int)(VALUE)) +#define HASHLIB_ACQUIRE_LOCK(OBJ) PyMutex_Lock(&(OBJ)->mutex) +#define HASHLIB_RELEASE_LOCK(OBJ) PyMutex_Unlock(&(OBJ)->mutex) -#define ENTER_HASHLIB(OBJ) \ - do { \ - if (_Py_atomic_load_int_relaxed((const int *)&(OBJ)->use_mutex)) { \ - PyMutex_Lock(&(OBJ)->mutex); \ - } \ - } while (0) - -#define LEAVE_HASHLIB(OBJ) \ - do { \ - if (_Py_atomic_load_int_relaxed((const int *)&(OBJ)->use_mutex)) { \ - PyMutex_Unlock(&(OBJ)->mutex); \ - } \ - } while (0) - -#ifdef Py_GIL_DISABLED #define HASHLIB_INIT_MUTEX(OBJ) \ do { \ (OBJ)->mutex = (PyMutex){0}; \ - (OBJ)->use_mutex = true; \ } while (0) -#else -#define HASHLIB_INIT_MUTEX(OBJ) \ - do { \ - (OBJ)->mutex = (PyMutex){0}; \ - (OBJ)->use_mutex = false; \ - } while (0) -#endif - -/* TODO(gpshead): We should make this a module or class attribute - * to allow the user to optimize based on the platform they're using. */ -#define HASHLIB_GIL_MINSIZE 2048 static inline int _Py_hashlib_data_argument(PyObject **res, PyObject *data, PyObject *string) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index c7b49d4dee3d0a..36744afea6476e 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -215,105 +215,6 @@ typedef struct py_hmac_hacl_api { #define Py_CHECK_HACL_UINT32_T_LENGTH(LEN) #endif -/* - * Call the HACL* HMAC-HASH update function on the given data. - * - * The magnitude of 'LEN' is not checked and thus 'LEN' must be - * safely convertible to a uint32_t value. - */ -#define Py_HMAC_HACL_UPDATE_CALL(HACL_STATE, BUF, LEN) \ - Hacl_Streaming_HMAC_update(HACL_STATE, BUF, (uint32_t)(LEN)) - -/* - * Call the HACL* HMAC-HASH update function on the given data. - * - * On DEBUG builds, the 'ERRACTION' statements are executed if - * the update() call returned a non-successful HACL* exit code. - * - * The buffer 'BUF' and its length 'LEN' are left untouched. - * - * The formal signature of this macro is: - * - * (HACL_HMAC_state *, uint8_t *, uint32_t, PyObject *, (C statements)) - */ -#ifndef NDEBUG -#define Py_HMAC_HACL_UPDATE_ONCE( \ - HACL_STATE, BUF, LEN, \ - ALGORITHM, ERRACTION \ -) \ - do { \ - Py_CHECK_HACL_UINT32_T_LENGTH(LEN); \ - hacl_errno_t code = Py_HMAC_HACL_UPDATE_CALL(HACL_STATE, BUF, LEN); \ - if (_hacl_convert_errno(code, (ALGORITHM)) < 0) { \ - ERRACTION; \ - } \ - } while (0) -#else -#define Py_HMAC_HACL_UPDATE_ONCE( \ - HACL_STATE, BUF, LEN, \ - _ALGORITHM, _ERRACTION \ -) \ - do { \ - (void)Py_HMAC_HACL_UPDATE_CALL(HACL_STATE, BUF, (LEN)); \ - } while (0) -#endif - -/* - * Repetivively call the HACL* HMAC-HASH update function on the given - * data until the buffer length 'LEN' is strictly less than UINT32_MAX. - * - * On builds with PY_SSIZE_T_MAX <= UINT32_MAX, this is a no-op. - * - * The buffer 'BUF' (resp. 'LEN') is advanced (resp. decremented) - * by UINT32_MAX after each update. On DEBUG builds, each update() - * call is verified and the 'ERRACTION' statements are executed if - * a non-successful HACL* exit code is being returned. - * - * In particular, 'BUF' and 'LEN' must be variable names and not - * expressions on their own. - * - * The formal signature of this macro is: - * - * (HACL_HMAC_state *, uint8_t *, C integer, PyObject *, (C statements)) - */ -#ifdef Py_HMAC_SSIZE_LARGER_THAN_UINT32 -#define Py_HMAC_HACL_UPDATE_LOOP( \ - HACL_STATE, BUF, LEN, \ - ALGORITHM, ERRACTION \ -) \ - do { \ - while ((Py_ssize_t)LEN > UINT32_MAX_AS_SSIZE_T) { \ - Py_HMAC_HACL_UPDATE_ONCE(HACL_STATE, BUF, UINT32_MAX, \ - ALGORITHM, ERRACTION); \ - BUF += UINT32_MAX; \ - LEN -= UINT32_MAX; \ - } \ - } while (0) -#else -#define Py_HMAC_HACL_UPDATE_LOOP( \ - HACL_STATE, BUF, LEN, \ - _ALGORITHM, _ERRACTION \ -) -#endif - -/* - * Perform the HMAC-HASH update() operation in a streaming fashion. - * - * The formal signature of this macro is: - * - * (HACL_HMAC_state *, uint8_t *, C integer, PyObject *, (C statements)) - */ -#define Py_HMAC_HACL_UPDATE( \ - HACL_STATE, BUF, LEN, \ - ALGORITHM, ERRACTION \ -) \ - do { \ - Py_HMAC_HACL_UPDATE_LOOP(HACL_STATE, BUF, LEN, \ - ALGORITHM, ERRACTION); \ - Py_HMAC_HACL_UPDATE_ONCE(HACL_STATE, BUF, LEN, \ - ALGORITHM, ERRACTION); \ - } while (0) - /* * HMAC underlying hash function static information. */ @@ -491,17 +392,14 @@ narrow_hmac_hash_kind(hmacmodule_state *state, HMAC_Hash_Kind kind) * Otherwise, this sets an appropriate exception and returns -1. */ static int -_hacl_convert_errno(hacl_errno_t code, PyObject *algorithm) +_hacl_convert_errno(hacl_errno_t code) { switch (code) { case Hacl_Streaming_Types_Success: { return 0; } case Hacl_Streaming_Types_InvalidAlgorithm: { - // only makes sense if an algorithm is known at call time - assert(algorithm != NULL); - assert(PyUnicode_CheckExact(algorithm)); - PyErr_Format(PyExc_ValueError, "invalid algorithm: %U", algorithm); + PyErr_Format(PyExc_ValueError, "invalid HACL* algorithm"); return -1; } case Hacl_Streaming_Types_InvalidLength: { @@ -536,7 +434,7 @@ _hacl_hmac_state_new(HMAC_Hash_Kind kind, uint8_t *key, uint32_t len) assert(kind != Py_hmac_kind_hash_unknown); HACL_HMAC_state *state = NULL; hacl_errno_t retcode = Hacl_Streaming_HMAC_malloc_(kind, key, len, &state); - if (_hacl_convert_errno(retcode, NULL) < 0) { + if (_hacl_convert_errno(retcode) < 0) { assert(state == NULL); return NULL; } @@ -554,13 +452,60 @@ _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. +*/ +static int +_hacl_hmac_state_update_once(HACL_HMAC_state *state, + uint8_t *buf, Py_ssize_t len) +{ + assert(len >= 0); +#ifndef NDEBUG + Py_CHECK_HACL_UINT32_T_LENGTH(len); + hacl_errno_t code = Hacl_Streaming_HMAC_update(state, buf, (uint32_t)len); + return _hacl_convert_errno(code); +#else + (void)Hacl_Streaming_HMAC_update(state, buf, (uint32_t)len); + return 0; +#endif +} + +/* + * 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. + */ +static int +_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)) { + assert(PyErr_Occurred()); + return -1; + } + buf += UINT32_MAX; + len -= UINT32_MAX; + } +#endif + assert(len <= UINT32_MAX_AS_SSIZE_T); + return _hacl_hmac_state_update_once(state, buf, len); +} + /* Static information used to construct the hash table. */ static const py_hmac_hinfo py_hmac_static_hinfo[] = { -#define Py_HMAC_HINFO_HACL_API(HACL_HID) \ - { \ - /* one-shot helpers */ \ - .compute = &Py_hmac_## HACL_HID ##_compute_func, \ - .compute_py = &_hmac_compute_## HACL_HID ##_impl, \ +#define Py_HMAC_HINFO_HACL_API(HACL_HID) \ + { \ + /* one-shot helpers */ \ + .compute = &Py_hmac_## HACL_HID ##_compute_func, \ + .compute_py = &_hmac_compute_## HACL_HID ##_impl, \ } #define Py_HMAC_HINFO_ENTRY(HACL_HID, HLIB_NAME) \ @@ -798,29 +743,16 @@ hmac_feed_initial_data(HMACObject *self, uint8_t *msg, Py_ssize_t len) { assert(self->name != NULL); assert(self->state != NULL); - if (len == 0) { - // do nothing if the buffer is empty - return 0; - } - - if (len < HASHLIB_GIL_MINSIZE) { - Py_HMAC_HACL_UPDATE(self->state, msg, len, self->name, return -1); - return 0; + assert(len >= 0); + int rc = 0; + if (len > 0) { + /* Do not use self->mutex here as this is the constructor + * where it is not yet possible to have concurrent access. */ + Py_BEGIN_ALLOW_THREADS + rc = _hacl_hmac_state_update(self->state, msg, len); + Py_END_ALLOW_THREADS } - - int res = 0; - Py_BEGIN_ALLOW_THREADS - Py_HMAC_HACL_UPDATE(self->state, msg, len, self->name, goto error); - goto done; -#ifndef NDEBUG -error: - res = -1; -#else - Py_UNREACHABLE(); -#endif -done: - Py_END_ALLOW_THREADS - return res; + return rc; } /*[clinic input] @@ -946,12 +878,13 @@ _hmac_HMAC_copy_impl(HMACObject *self, PyTypeObject *cls) return NULL; } - ENTER_HASHLIB(self); + int rc = 0; + HASHLIB_ACQUIRE_LOCK(self); /* copy hash information */ hmac_copy_hinfo(copy, self); /* copy internal state */ - int rc = hmac_copy_state(copy, self); - LEAVE_HASHLIB(self); + rc = hmac_copy_state(copy, self); + HASHLIB_RELEASE_LOCK(self); if (rc < 0) { Py_DECREF(copy); @@ -963,78 +896,6 @@ _hmac_HMAC_copy_impl(HMACObject *self, PyTypeObject *cls) return (PyObject *)copy; } -/* - * Update the HMAC object with the given buffer. - * - * This unconditionally acquires the lock on the HMAC object. - * - * On DEBUG builds, each update() call is verified. - * - * Return 0 on success; otherwise, set an exception and return -1 on failure. - */ -static int -hmac_update_state_with_lock(HMACObject *self, uint8_t *buf, Py_ssize_t len) -{ - int res = 0; - Py_BEGIN_ALLOW_THREADS - PyMutex_Lock(&self->mutex); // unconditionally acquire a lock - Py_HMAC_HACL_UPDATE(self->state, buf, len, self->name, goto error); - goto done; -#ifndef NDEBUG -error: - res = -1; -#else - Py_UNREACHABLE(); -#endif -done: - PyMutex_Unlock(&self->mutex); - Py_END_ALLOW_THREADS - return res; -} - -/* - * Update the HMAC object with the given buffer. - * - * This conditionally acquires the lock on the HMAC object. - * - * On DEBUG builds, each update() call is verified. - * - * Return 0 on success; otherwise, set an exception and return -1 on failure. - */ -static int -hmac_update_state_cond_lock(HMACObject *self, uint8_t *buf, Py_ssize_t len) -{ - ENTER_HASHLIB(self); // conditionally acquire a lock - Py_HMAC_HACL_UPDATE(self->state, buf, len, self->name, goto error); - LEAVE_HASHLIB(self); - return 0; - -#ifndef NDEBUG -error: - LEAVE_HASHLIB(self); - return -1; -#else - Py_UNREACHABLE(); -#endif -} - -/* - * Update the internal HMAC state with the given buffer. - * - * Return 0 on success; otherwise, set an exception and return -1 on failure. - */ -static inline int -hmac_update_state(HMACObject *self, uint8_t *buf, Py_ssize_t len) -{ - assert(buf != 0); - assert(len >= 0); - return len == 0 - ? 0 /* nothing to do */ - : len < HASHLIB_GIL_MINSIZE - ? hmac_update_state_cond_lock(self, buf, len) - : hmac_update_state_with_lock(self, buf, len); -} - /*[clinic input] _hmac.HMAC.update @@ -1047,9 +908,16 @@ static PyObject * _hmac_HMAC_update_impl(HMACObject *self, PyObject *msgobj) /*[clinic end generated code: output=962134ada5e55985 input=7c0ea830efb03367]*/ { + int rc = 0; Py_buffer msg; GET_BUFFER_VIEW_OR_ERROUT(msgobj, &msg); - int rc = hmac_update_state(self, msg.buf, msg.len); + if (msg.len > 0) { + Py_BEGIN_ALLOW_THREADS + HASHLIB_ACQUIRE_LOCK(self); + rc = _hacl_hmac_state_update(self->state, msg.buf, msg.len); + HASHLIB_RELEASE_LOCK(self); + Py_END_ALLOW_THREADS + } PyBuffer_Release(&msg); return rc < 0 ? NULL : Py_None; } @@ -1069,14 +937,14 @@ hmac_digest_compute_cond_lock(HMACObject *self, uint8_t *digest) { assert(digest != NULL); hacl_errno_t rc; - ENTER_HASHLIB(self); // conditionally acquire a lock + HASHLIB_ACQUIRE_LOCK(self); rc = Hacl_Streaming_HMAC_digest(self->state, digest, self->digest_size); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); assert( rc == Hacl_Streaming_Types_Success || rc == Hacl_Streaming_Types_OutOfMemory ); - return _hacl_convert_errno(rc, NULL); + return _hacl_convert_errno(rc); } /*[clinic input] @@ -1692,20 +1560,6 @@ hmacmodule_init_strings(hmacmodule_state *state) return 0; } -static int -hmacmodule_init_globals(PyObject *module, hmacmodule_state *state) -{ -#define ADD_INT_CONST(NAME, VALUE) \ - do { \ - if (PyModule_AddIntConstant(module, (NAME), (VALUE)) < 0) { \ - return -1; \ - } \ - } while (0) - ADD_INT_CONST("_GIL_MINSIZE", HASHLIB_GIL_MINSIZE); -#undef ADD_INT_CONST - return 0; -} - static void hmacmodule_init_cpu_features(hmacmodule_state *state) { @@ -1796,9 +1650,6 @@ hmacmodule_exec(PyObject *module) if (hmacmodule_init_strings(state) < 0) { return -1; } - if (hmacmodule_init_globals(module, state) < 0) { - return -1; - } hmacmodule_init_cpu_features(state); return 0; } diff --git a/Modules/md5module.c b/Modules/md5module.c index a05fd9d591fdac..9aba1851dc1f4b 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -115,9 +115,9 @@ MD5Type_copy_impl(MD5object *self, PyTypeObject *cls) return NULL; } - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); newobj->hash_state = Hacl_Hash_MD5_copy(self->hash_state); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); if (newobj->hash_state == NULL) { Py_DECREF(self); return PyErr_NoMemory(); @@ -126,11 +126,11 @@ MD5Type_copy_impl(MD5object *self, PyTypeObject *cls) } static void -md5_digest_compute_cond_lock(MD5object *self, uint8_t *digest) +md5_digest_compute_with_lock(MD5object *self, uint8_t *digest) { - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); Hacl_Hash_MD5_digest(self->hash_state, digest); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); } /*[clinic input] @@ -144,7 +144,7 @@ MD5Type_digest_impl(MD5object *self) /*[clinic end generated code: output=eb691dc4190a07ec input=bc0c4397c2994be6]*/ { uint8_t digest[MD5_DIGESTSIZE]; - md5_digest_compute_cond_lock(self, digest); + md5_digest_compute_with_lock(self, digest); return PyBytes_FromStringAndSize((const char *)digest, MD5_DIGESTSIZE); } @@ -159,18 +159,20 @@ MD5Type_hexdigest_impl(MD5object *self) /*[clinic end generated code: output=17badced1f3ac932 input=b60b19de644798dd]*/ { uint8_t digest[MD5_DIGESTSIZE]; - md5_digest_compute_cond_lock(self, digest); + md5_digest_compute_with_lock(self, digest); return _Py_strhex((const char *)digest, MD5_DIGESTSIZE); } static void -_hacl_md5_update(Hacl_Hash_MD5_state_t *state, uint8_t *buf, Py_ssize_t len) +_hacl_md5_state_update(Hacl_Hash_MD5_state_t *state, + uint8_t *buf, Py_ssize_t len) { + assert(len >= 0); /* - * Note: we explicitly ignore the error code on the basis that it would - * take more than 1 billion years to overflow the maximum admissible length - * for MD5 (2^61 - 1). - */ + * Note: we explicitly ignore the error code on the basis that it would + * take more than 1 billion years to overflow the maximum admissible length + * for MD5 (2^61 - 1). + */ #if PY_SSIZE_T_MAX > UINT32_MAX while (len > UINT32_MAX) { (void)Hacl_Hash_MD5_update(state, buf, UINT32_MAX); @@ -182,42 +184,6 @@ _hacl_md5_update(Hacl_Hash_MD5_state_t *state, uint8_t *buf, Py_ssize_t len) (void)Hacl_Hash_MD5_update(state, buf, (uint32_t)len); } -static void -md5_update_state_with_lock(MD5object *self, uint8_t *buf, Py_ssize_t len) -{ - Py_BEGIN_ALLOW_THREADS - PyMutex_Lock(&self->mutex); // unconditionally acquire a lock - _hacl_md5_update(self->hash_state, buf, len); - PyMutex_Unlock(&self->mutex); - Py_END_ALLOW_THREADS -} - -static void -md5_update_state_cond_lock(MD5object *self, uint8_t *buf, Py_ssize_t len) -{ - ENTER_HASHLIB(self); // conditionally acquire a lock - _hacl_md5_update(self->hash_state, buf, len); - LEAVE_HASHLIB(self); -} - -static inline void -md5_update_state(MD5object *self, uint8_t *buf, Py_ssize_t len) -{ - assert(buf != 0); - assert(len >= 0); - if (len == 0) { - return; - } - if (len < HASHLIB_GIL_MINSIZE) { - md5_update_state_cond_lock(self, buf, len); - } - else { - HASHLIB_SET_MUTEX_POLICY(self, 1); - md5_update_state_with_lock(self, buf, len); - HASHLIB_SET_MUTEX_POLICY(self, 0); - } -} - /*[clinic input] MD5Type.update @@ -232,9 +198,14 @@ MD5Type_update_impl(MD5object *self, PyObject *obj) /*[clinic end generated code: output=b0fed9a7ce7ad253 input=6e1efcd9ecf17032]*/ { Py_buffer buf; - GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - md5_update_state(self, buf.buf, buf.len); + if (buf.len > 0) { + Py_BEGIN_ALLOW_THREADS + HASHLIB_ACQUIRE_LOCK(self); + _hacl_md5_state_update(self->hash_state, buf.buf, buf.len); + HASHLIB_RELEASE_LOCK(self); + Py_END_ALLOW_THREADS + } PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -336,16 +307,11 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, } if (string) { - if (buf.len >= HASHLIB_GIL_MINSIZE) { - /* Do not use self->mutex here as this is the constructor - * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - _hacl_md5_update(new->hash_state, buf.buf, buf.len); - Py_END_ALLOW_THREADS - } - else { - _hacl_md5_update(new->hash_state, buf.buf, buf.len); - } + /* Do not use self->mutex here as this is the constructor + * where it is not yet possible to have concurrent access. */ + Py_BEGIN_ALLOW_THREADS + _hacl_md5_state_update(new->hash_state, buf.buf, buf.len); + Py_END_ALLOW_THREADS PyBuffer_Release(&buf); } @@ -394,9 +360,6 @@ md5_exec(PyObject *m) if (PyModule_AddObjectRef(m, "MD5Type", (PyObject *)st->md5_type) < 0) { return -1; } - if (PyModule_AddIntConstant(m, "_GIL_MINSIZE", HASHLIB_GIL_MINSIZE) < 0) { - return -1; - } return 0; } diff --git a/Modules/sha1module.c b/Modules/sha1module.c index a746bf74f8d4c1..a95a9af5fe848c 100644 --- a/Modules/sha1module.c +++ b/Modules/sha1module.c @@ -121,9 +121,9 @@ SHA1Type_copy_impl(SHA1object *self, PyTypeObject *cls) return NULL; } - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); newobj->hash_state = Hacl_Hash_SHA1_copy(self->hash_state); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); if (newobj->hash_state == NULL) { Py_DECREF(newobj); return PyErr_NoMemory(); @@ -142,9 +142,9 @@ SHA1Type_digest_impl(SHA1object *self) /*[clinic end generated code: output=2f05302a7aa2b5cb input=13824b35407444bd]*/ { unsigned char digest[SHA1_DIGESTSIZE]; - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); Hacl_Hash_SHA1_digest(self->hash_state, digest); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); return PyBytes_FromStringAndSize((const char *)digest, SHA1_DIGESTSIZE); } @@ -159,14 +159,15 @@ SHA1Type_hexdigest_impl(SHA1object *self) /*[clinic end generated code: output=4161fd71e68c6659 input=97691055c0c74ab0]*/ { unsigned char digest[SHA1_DIGESTSIZE]; - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); Hacl_Hash_SHA1_digest(self->hash_state, digest); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); return _Py_strhex((const char *)digest, SHA1_DIGESTSIZE); } static void -update(Hacl_Hash_SHA1_state_t *state, uint8_t *buf, Py_ssize_t len) +_hacl_sha1_state_update(Hacl_Hash_SHA1_state_t *state, + uint8_t *buf, Py_ssize_t len) { /* * Note: we explicitly ignore the error code on the basis that it would @@ -198,22 +199,14 @@ SHA1Type_update_impl(SHA1object *self, PyObject *obj) /*[clinic end generated code: output=cdc8e0e106dbec5f input=aad8e07812edbba3]*/ { Py_buffer buf; - GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - - if (!self->use_mutex && buf.len >= HASHLIB_GIL_MINSIZE) { - self->use_mutex = true; - } - if (self->use_mutex) { + if (buf.len > 0) { Py_BEGIN_ALLOW_THREADS - PyMutex_Lock(&self->mutex); - update(self->hash_state, buf.buf, buf.len); - PyMutex_Unlock(&self->mutex); + HASHLIB_ACQUIRE_LOCK(self); + _hacl_sha1_state_update(self->hash_state, buf.buf, buf.len); + HASHLIB_RELEASE_LOCK(self); Py_END_ALLOW_THREADS - } else { - update(self->hash_state, buf.buf, buf.len); } - PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -314,16 +307,11 @@ _sha1_sha1_impl(PyObject *module, PyObject *data, int usedforsecurity, return PyErr_NoMemory(); } if (string) { - if (buf.len >= HASHLIB_GIL_MINSIZE) { - /* We do not initialize self->lock here as this is the constructor - * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - update(new->hash_state, buf.buf, buf.len); - Py_END_ALLOW_THREADS - } - else { - update(new->hash_state, buf.buf, buf.len); - } + /* Do not use self->mutex here as this is the constructor + * where it is not yet possible to have concurrent access. */ + Py_BEGIN_ALLOW_THREADS + _hacl_sha1_state_update(new->hash_state, buf.buf, buf.len); + Py_END_ALLOW_THREADS PyBuffer_Release(&buf); } @@ -373,12 +361,6 @@ _sha1_exec(PyObject *module) { return -1; } - if (PyModule_AddIntConstant(module, - "_GIL_MINSIZE", - HASHLIB_GIL_MINSIZE) < 0) - { - return -1; - } return 0; } diff --git a/Modules/sha2module.c b/Modules/sha2module.c index 72931910c5d720..1595fb9eca56e4 100644 --- a/Modules/sha2module.c +++ b/Modules/sha2module.c @@ -206,7 +206,8 @@ SHA512_dealloc(PyObject *op) * 64 bits so we loop in <4gig chunks when needed. */ static void -update_256(Hacl_Hash_SHA2_state_t_256 *state, uint8_t *buf, Py_ssize_t len) +_hacl_sha2_state_update_256(Hacl_Hash_SHA2_state_t_256 *state, + uint8_t *buf, Py_ssize_t len) { /* * Note: we explicitly ignore the error code on the basis that it would @@ -225,7 +226,8 @@ update_256(Hacl_Hash_SHA2_state_t_256 *state, uint8_t *buf, Py_ssize_t len) } static void -update_512(Hacl_Hash_SHA2_state_t_512 *state, uint8_t *buf, Py_ssize_t len) +_hacl_sha2_state_update_512(Hacl_Hash_SHA2_state_t_512 *state, + uint8_t *buf, Py_ssize_t len) { /* * Note: we explicitly ignore the error code on the basis that it would @@ -272,9 +274,9 @@ SHA256Type_copy_impl(SHA256object *self, PyTypeObject *cls) } } - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); rc = SHA256copy(self, newobj); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); if (rc < 0) { Py_DECREF(newobj); return NULL; @@ -309,9 +311,9 @@ SHA512Type_copy_impl(SHA512object *self, PyTypeObject *cls) } } - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); rc = SHA512copy(self, newobj); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); if (rc < 0) { Py_DECREF(newobj); return NULL; @@ -331,11 +333,11 @@ SHA256Type_digest_impl(SHA256object *self) { uint8_t digest[SHA256_DIGESTSIZE]; assert(self->digestsize <= SHA256_DIGESTSIZE); - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); // HACL* performs copies under the hood so that self->state remains valid // after this call. Hacl_Hash_SHA2_digest_256(self->state, digest); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); return PyBytes_FromStringAndSize((const char *)digest, self->digestsize); } @@ -351,11 +353,11 @@ SHA512Type_digest_impl(SHA512object *self) { uint8_t digest[SHA512_DIGESTSIZE]; assert(self->digestsize <= SHA512_DIGESTSIZE); - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); // HACL* performs copies under the hood so that self->state remains valid // after this call. Hacl_Hash_SHA2_digest_512(self->state, digest); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); return PyBytes_FromStringAndSize((const char *)digest, self->digestsize); } @@ -371,9 +373,9 @@ SHA256Type_hexdigest_impl(SHA256object *self) { uint8_t digest[SHA256_DIGESTSIZE]; assert(self->digestsize <= SHA256_DIGESTSIZE); - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); Hacl_Hash_SHA2_digest_256(self->state, digest); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); return _Py_strhex((const char *)digest, self->digestsize); } @@ -389,9 +391,9 @@ SHA512Type_hexdigest_impl(SHA512object *self) { uint8_t digest[SHA512_DIGESTSIZE]; assert(self->digestsize <= SHA512_DIGESTSIZE); - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); Hacl_Hash_SHA2_digest_512(self->state, digest); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); return _Py_strhex((const char *)digest, self->digestsize); } @@ -409,22 +411,14 @@ SHA256Type_update_impl(SHA256object *self, PyObject *obj) /*[clinic end generated code: output=dc58a580cf8905a5 input=b2d449d5b30f0f5a]*/ { Py_buffer buf; - GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - - if (!self->use_mutex && buf.len >= HASHLIB_GIL_MINSIZE) { - self->use_mutex = true; - } - if (self->use_mutex) { + if (buf.len > 0) { Py_BEGIN_ALLOW_THREADS - PyMutex_Lock(&self->mutex); - update_256(self->state, buf.buf, buf.len); - PyMutex_Unlock(&self->mutex); + HASHLIB_ACQUIRE_LOCK(self); + _hacl_sha2_state_update_256(self->state, buf.buf, buf.len); + HASHLIB_RELEASE_LOCK(self); Py_END_ALLOW_THREADS - } else { - update_256(self->state, buf.buf, buf.len); } - PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -443,22 +437,14 @@ SHA512Type_update_impl(SHA512object *self, PyObject *obj) /*[clinic end generated code: output=9af211766c0b7365 input=ded2b46656566283]*/ { Py_buffer buf; - GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - - if (!self->use_mutex && buf.len >= HASHLIB_GIL_MINSIZE) { - self->use_mutex = true; - } - if (self->use_mutex) { + if (buf.len > 0) { Py_BEGIN_ALLOW_THREADS - PyMutex_Lock(&self->mutex); - update_512(self->state, buf.buf, buf.len); - PyMutex_Unlock(&self->mutex); + HASHLIB_ACQUIRE_LOCK(self); + _hacl_sha2_state_update_512(self->state, buf.buf, buf.len); + HASHLIB_RELEASE_LOCK(self); Py_END_ALLOW_THREADS - } else { - update_512(self->state, buf.buf, buf.len); } - PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -638,16 +624,11 @@ _sha2_sha256_impl(PyObject *module, PyObject *data, int usedforsecurity, return PyErr_NoMemory(); } if (string) { - if (buf.len >= HASHLIB_GIL_MINSIZE) { - /* We do not initialize self->lock here as this is the constructor - * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - update_256(new->state, buf.buf, buf.len); - Py_END_ALLOW_THREADS - } - else { - update_256(new->state, buf.buf, buf.len); - } + /* Do not use self->mutex here as this is the constructor + * where it is not yet possible to have concurrent access. */ + Py_BEGIN_ALLOW_THREADS + _hacl_sha2_state_update_256(new->state, buf.buf, buf.len); + Py_END_ALLOW_THREADS PyBuffer_Release(&buf); } @@ -700,16 +681,11 @@ _sha2_sha224_impl(PyObject *module, PyObject *data, int usedforsecurity, return PyErr_NoMemory(); } if (string) { - if (buf.len >= HASHLIB_GIL_MINSIZE) { - /* We do not initialize self->lock here as this is the constructor - * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - update_256(new->state, buf.buf, buf.len); - Py_END_ALLOW_THREADS - } - else { - update_256(new->state, buf.buf, buf.len); - } + /* Do not use self->mutex here as this is the constructor + * where it is not yet possible to have concurrent access. */ + Py_BEGIN_ALLOW_THREADS + _hacl_sha2_state_update_256(new->state, buf.buf, buf.len); + Py_END_ALLOW_THREADS PyBuffer_Release(&buf); } @@ -763,16 +739,11 @@ _sha2_sha512_impl(PyObject *module, PyObject *data, int usedforsecurity, return PyErr_NoMemory(); } if (string) { - if (buf.len >= HASHLIB_GIL_MINSIZE) { - /* We do not initialize self->lock here as this is the constructor - * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - update_512(new->state, buf.buf, buf.len); - Py_END_ALLOW_THREADS - } - else { - update_512(new->state, buf.buf, buf.len); - } + /* Do not use self->mutex here as this is the constructor + * where it is not yet possible to have concurrent access. */ + Py_BEGIN_ALLOW_THREADS + _hacl_sha2_state_update_512(new->state, buf.buf, buf.len); + Py_END_ALLOW_THREADS PyBuffer_Release(&buf); } @@ -826,16 +797,13 @@ _sha2_sha384_impl(PyObject *module, PyObject *data, int usedforsecurity, return PyErr_NoMemory(); } if (string) { - if (buf.len >= HASHLIB_GIL_MINSIZE) { - /* We do not initialize self->lock here as this is the constructor - * where it is not yet possible to have concurrent access. */ + /* Do not use self->mutex here as this is the constructor + * where it is not yet possible to have concurrent access. */ + if (buf.len > 0) { Py_BEGIN_ALLOW_THREADS - update_512(new->state, buf.buf, buf.len); + _hacl_sha2_state_update_512(new->state, buf.buf, buf.len); Py_END_ALLOW_THREADS } - else { - update_512(new->state, buf.buf, buf.len); - } PyBuffer_Release(&buf); } @@ -919,13 +887,6 @@ static int sha2_exec(PyObject *module) return -1; } - if (PyModule_AddIntConstant(module, - "_GIL_MINSIZE", - HASHLIB_GIL_MINSIZE) < 0) - { - return -1; - } - return 0; } diff --git a/Modules/sha3module.c b/Modules/sha3module.c index cfbf0cbcc042c5..b66af6dbcd1c5e 100644 --- a/Modules/sha3module.c +++ b/Modules/sha3module.c @@ -163,16 +163,13 @@ py_sha3_new_impl(PyTypeObject *type, PyObject *data_obj, int usedforsecurity, if (data) { GET_BUFFER_VIEW_OR_ERROR(data, &buf, goto error); - if (buf.len >= HASHLIB_GIL_MINSIZE) { - /* We do not initialize self->lock here as this is the constructor - * where it is not yet possible to have concurrent access. */ + /* Do not use self->mutex here as this is the constructor + * where it is not yet possible to have concurrent access. */ + if (buf.len > 0) { Py_BEGIN_ALLOW_THREADS - sha3_update(self->hash_state, buf.buf, buf.len); + sha3_update(self->hash_state, buf.buf, buf.len); Py_END_ALLOW_THREADS } - else { - sha3_update(self->hash_state, buf.buf, buf.len); - } } PyBuffer_Release(&buf); @@ -238,9 +235,9 @@ _sha3_sha3_224_copy_impl(SHA3object *self) if ((newobj = newSHA3object(Py_TYPE(self))) == NULL) { return NULL; } - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); newobj->hash_state = Hacl_Hash_SHA3_copy(self->hash_state); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); if (newobj->hash_state == NULL) { Py_DECREF(newobj); return PyErr_NoMemory(); @@ -262,9 +259,9 @@ _sha3_sha3_224_digest_impl(SHA3object *self) unsigned char digest[SHA3_MAX_DIGESTSIZE]; // This function errors out if the algorithm is SHAKE. Here, we know this // not to be the case, and therefore do not perform error checking. - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); (void)Hacl_Hash_SHA3_digest(self->hash_state, digest); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); return PyBytes_FromStringAndSize((const char *)digest, Hacl_Hash_SHA3_hash_len(self->hash_state)); } @@ -281,9 +278,9 @@ _sha3_sha3_224_hexdigest_impl(SHA3object *self) /*[clinic end generated code: output=75ad03257906918d input=2d91bb6e0d114ee3]*/ { unsigned char digest[SHA3_MAX_DIGESTSIZE]; - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); (void)Hacl_Hash_SHA3_digest(self->hash_state, digest); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); return _Py_strhex((const char *)digest, Hacl_Hash_SHA3_hash_len(self->hash_state)); } @@ -303,22 +300,14 @@ _sha3_sha3_224_update_impl(SHA3object *self, PyObject *data) /*[clinic end generated code: output=390b7abf7c9795a5 input=a887f54dcc4ae227]*/ { Py_buffer buf; - GET_BUFFER_VIEW_OR_ERROUT(data, &buf); - - if (!self->use_mutex && buf.len >= HASHLIB_GIL_MINSIZE) { - self->use_mutex = true; - } - if (self->use_mutex) { + if (buf.len > 0) { Py_BEGIN_ALLOW_THREADS - PyMutex_Lock(&self->mutex); - sha3_update(self->hash_state, buf.buf, buf.len); - PyMutex_Unlock(&self->mutex); + HASHLIB_ACQUIRE_LOCK(self); + sha3_update(self->hash_state, buf.buf, buf.len); + HASHLIB_RELEASE_LOCK(self); Py_END_ALLOW_THREADS - } else { - sha3_update(self->hash_state, buf.buf, buf.len); } - PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -647,9 +636,6 @@ _sha3_exec(PyObject *m) if (PyModule_AddStringConstant(m, "implementation", "HACL") < 0) { return -1; } - if (PyModule_AddIntConstant(m, "_GIL_MINSIZE", HASHLIB_GIL_MINSIZE) < 0) { - return -1; - } return 0; } From 05c1e6631268b3e04d853ef74213b1dbbe2420e4 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, 16 Jun 2025 12:26:51 +0200 Subject: [PATCH 10/39] post-merge --- Modules/blake2module.c | 4 +--- Modules/hashlib.h | 3 ++- Modules/hmacmodule.c | 9 ++------- Modules/md5module.c | 4 +--- Modules/sha1module.c | 6 +----- Modules/sha2module.c | 10 ++-------- Modules/sha3module.c | 5 +---- 7 files changed, 10 insertions(+), 31 deletions(-) diff --git a/Modules/blake2module.c b/Modules/blake2module.c index 23487fcbd07be4..0d0fd3881d6687 100644 --- a/Modules/blake2module.c +++ b/Modules/blake2module.c @@ -350,7 +350,7 @@ type_to_impl(PyTypeObject *type) } typedef struct { - PyObject_HEAD + PyObject_HASHLIB_HEAD union { Hacl_Hash_Blake2s_state_t *blake2s_state; Hacl_Hash_Blake2b_state_t *blake2b_state; @@ -362,8 +362,6 @@ typedef struct { #endif }; blake2_impl impl; - bool use_mutex; - PyMutex mutex; } Blake2Object; #define _Blake2Object_CAST(op) ((Blake2Object *)(op)) diff --git a/Modules/hashlib.h b/Modules/hashlib.h index 43d53442666fb7..d33f144d79f0a9 100644 --- a/Modules/hashlib.h +++ b/Modules/hashlib.h @@ -50,7 +50,8 @@ #include "pythread.h" -#define HASHLIB_LOCK_HEAD \ +#define PyObject_HASHLIB_HEAD \ + PyObject_HEAD \ /* Guard against race conditions during incremental update(). */ \ PyMutex mutex; diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index cb53c1dcd7b42c..d3b22cb41ca119 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -283,11 +283,7 @@ get_hmacmodule_state_by_cls(PyTypeObject *cls) typedef Hacl_Streaming_HMAC_agile_state HACL_HMAC_state; typedef struct HMACObject { - PyObject_HEAD - - bool use_mutex; - PyMutex mutex; - + PyObject_HASHLIB_HEAD // Hash function information PyObject *name; // rendered name (exact unicode object) HMAC_Hash_Kind kind; // can be used for runtime dispatch (must be known) @@ -878,12 +874,11 @@ _hmac_HMAC_copy_impl(HMACObject *self, PyTypeObject *cls) return NULL; } - int rc = 0; HASHLIB_ACQUIRE_LOCK(self); /* copy hash information */ hmac_copy_hinfo(copy, self); /* copy internal state */ - rc = hmac_copy_state(copy, self); + int rc = hmac_copy_state(copy, self); HASHLIB_RELEASE_LOCK(self); if (rc < 0) { diff --git a/Modules/md5module.c b/Modules/md5module.c index bfc269c80b9a5e..38b6bbe5ad687c 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -37,10 +37,8 @@ class MD5Type "MD5object *" "&PyType_Type" #include "_hacl/Hacl_Hash_MD5.h" - typedef struct { - PyObject_HEAD - HASHLIB_LOCK_HEAD + PyObject_HASHLIB_HEAD Hacl_Hash_MD5_state_t *hash_state; } MD5object; diff --git a/Modules/sha1module.c b/Modules/sha1module.c index a95a9af5fe848c..c68caeb6a3ca93 100644 --- a/Modules/sha1module.c +++ b/Modules/sha1module.c @@ -38,11 +38,7 @@ class SHA1Type "SHA1object *" "&PyType_Type" #include "_hacl/Hacl_Hash_SHA1.h" typedef struct { - PyObject_HEAD - // Prevents undefined behavior via multiple threads entering the C API. - bool use_mutex; - PyMutex mutex; - PyThread_type_lock lock; + PyObject_HASHLIB_HEAD Hacl_Hash_SHA1_state_t *hash_state; } SHA1object; diff --git a/Modules/sha2module.c b/Modules/sha2module.c index 1595fb9eca56e4..cf2ca265c921b5 100644 --- a/Modules/sha2module.c +++ b/Modules/sha2module.c @@ -50,20 +50,14 @@ class SHA512Type "SHA512object *" "&PyType_Type" // TODO: Get rid of int digestsize in favor of Hacl state info? typedef struct { - PyObject_HEAD + PyObject_HASHLIB_HEAD int digestsize; - // Prevents undefined behavior via multiple threads entering the C API. - bool use_mutex; - PyMutex mutex; Hacl_Hash_SHA2_state_t_256 *state; } SHA256object; typedef struct { - PyObject_HEAD + PyObject_HASHLIB_HEAD int digestsize; - // Prevents undefined behavior via multiple threads entering the C API. - bool use_mutex; - PyMutex mutex; Hacl_Hash_SHA2_state_t_512 *state; } SHA512object; diff --git a/Modules/sha3module.c b/Modules/sha3module.c index b66af6dbcd1c5e..e12aaa9966a02f 100644 --- a/Modules/sha3module.c +++ b/Modules/sha3module.c @@ -59,10 +59,7 @@ class _sha3.shake_256 "SHA3object *" "&SHAKE256type" #include "_hacl/Hacl_Hash_SHA3.h" typedef struct { - PyObject_HEAD - // Prevents undefined behavior via multiple threads entering the C API. - bool use_mutex; - PyMutex mutex; + PyObject_HASHLIB_HEAD Hacl_Hash_SHA3_state_t *hash_state; } SHA3object; From db5727853bf6dea49a13f1e1b0135b940f519a6b 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, 16 Jun 2025 12:33:23 +0200 Subject: [PATCH 11/39] do not guard against empty buffers for now --- Modules/_hashopenssl.c | 24 +++++++++++------------- Modules/blake2module.c | 20 ++++++++------------ Modules/hmacmodule.c | 25 ++++++++++--------------- Modules/md5module.c | 28 +++++++++++----------------- Modules/sha1module.c | 12 +++++------- Modules/sha2module.c | 32 +++++++++++++------------------- Modules/sha3module.c | 20 ++++++++------------ 7 files changed, 66 insertions(+), 95 deletions(-) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 4abcfe2219433e..1a8691035295bb 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -800,16 +800,16 @@ static PyObject * _hashlib_HASH_update_impl(HASHobject *self, PyObject *obj) /*[clinic end generated code: output=62ad989754946b86 input=aa1ce20e3f92ceb6]*/ { - int rc; + int result; Py_buffer view; GET_BUFFER_VIEW_OR_ERROUT(obj, &view); Py_BEGIN_ALLOW_THREADS HASHLIB_ACQUIRE_LOCK(self); - rc = _hashlib_HASH_hash(self, view.buf, view.len); + result = _hashlib_HASH_hash(self, view.buf, view.len); HASHLIB_RELEASE_LOCK(self); Py_END_ALLOW_THREADS PyBuffer_Release(&view); - return rc < 0 ? NULL : Py_None; + return result < 0 ? NULL : Py_None; } static PyMethodDef HASH_methods[] = { @@ -1806,19 +1806,17 @@ _hashlib_hmac_digest_size(HMACobject *self) static int _hmac_update(HMACobject *self, PyObject *obj) { - int r = 1; + int r; Py_buffer view = {0}; GET_BUFFER_VIEW_OR_ERROR(obj, &view, return 0); - if (view.len > 0) { - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - r = HMAC_Update(self->ctx, - (const unsigned char *)view.buf, - (size_t)view.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS - } + Py_BEGIN_ALLOW_THREADS + HASHLIB_ACQUIRE_LOCK(self); + r = HMAC_Update(self->ctx, + (const unsigned char *)view.buf, + (size_t)view.len); + HASHLIB_RELEASE_LOCK(self); + Py_END_ALLOW_THREADS PyBuffer_Release(&view); if (r == 0) { diff --git a/Modules/blake2module.c b/Modules/blake2module.c index 0d0fd3881d6687..f7b44625666b99 100644 --- a/Modules/blake2module.c +++ b/Modules/blake2module.c @@ -642,11 +642,9 @@ py_blake2_new(PyTypeObject *type, PyObject *data, int digest_size, if (data != NULL) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROR(data, &buf, goto error); - if (buf.len > 0) { - Py_BEGIN_ALLOW_THREADS - blake2_update_state_unlocked(self, buf.buf, buf.len); - Py_END_ALLOW_THREADS - } + Py_BEGIN_ALLOW_THREADS + blake2_update_state_unlocked(self, buf.buf, buf.len); + Py_END_ALLOW_THREADS PyBuffer_Release(&buf); } @@ -819,13 +817,11 @@ _blake2_blake2b_update_impl(Blake2Object *self, PyObject *data) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(data, &buf); - if (buf.len > 0) { - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - blake2_update_state_unlocked(self, buf.buf, buf.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS - } + Py_BEGIN_ALLOW_THREADS + HASHLIB_ACQUIRE_LOCK(self); + blake2_update_state_unlocked(self, buf.buf, buf.len); + HASHLIB_RELEASE_LOCK(self); + Py_END_ALLOW_THREADS PyBuffer_Release(&buf); Py_RETURN_NONE; } diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index d3b22cb41ca119..f4b72fb832f052 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -739,15 +739,12 @@ hmac_feed_initial_data(HMACObject *self, uint8_t *msg, Py_ssize_t len) { assert(self->name != NULL); assert(self->state != NULL); - assert(len >= 0); int rc = 0; - if (len > 0) { - /* Do not use self->mutex here as this is the constructor - * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - rc = _hacl_hmac_state_update(self->state, msg, len); - Py_END_ALLOW_THREADS - } + /* Do not use self->mutex here as this is the constructor + * where it is not yet possible to have concurrent access. */ + Py_BEGIN_ALLOW_THREADS + rc = _hacl_hmac_state_update(self->state, msg, len); + Py_END_ALLOW_THREADS return rc; } @@ -906,13 +903,11 @@ _hmac_HMAC_update_impl(HMACObject *self, PyObject *msgobj) int rc = 0; Py_buffer msg; GET_BUFFER_VIEW_OR_ERROUT(msgobj, &msg); - if (msg.len > 0) { - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - rc = _hacl_hmac_state_update(self->state, msg.buf, msg.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS - } + Py_BEGIN_ALLOW_THREADS + HASHLIB_ACQUIRE_LOCK(self); + rc = _hacl_hmac_state_update(self->state, msg.buf, msg.len); + HASHLIB_RELEASE_LOCK(self); + Py_END_ALLOW_THREADS PyBuffer_Release(&msg); return rc < 0 ? NULL : Py_None; } diff --git a/Modules/md5module.c b/Modules/md5module.c index 38b6bbe5ad687c..7f94f6bf6ad9f1 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -123,14 +123,6 @@ MD5Type_copy_impl(MD5object *self, PyTypeObject *cls) return (PyObject *)newobj; } -static void -md5_digest_compute_with_lock(MD5object *self, uint8_t *digest) -{ - HASHLIB_ACQUIRE_LOCK(self); - Hacl_Hash_MD5_digest(self->hash_state, digest); - HASHLIB_RELEASE_LOCK(self); -} - /*[clinic input] MD5Type.digest @@ -142,7 +134,9 @@ MD5Type_digest_impl(MD5object *self) /*[clinic end generated code: output=eb691dc4190a07ec input=bc0c4397c2994be6]*/ { uint8_t digest[MD5_DIGESTSIZE]; - md5_digest_compute_with_lock(self, digest); + HASHLIB_ACQUIRE_LOCK(self); + Hacl_Hash_MD5_digest(self->hash_state, digest); + HASHLIB_RELEASE_LOCK(self); return PyBytes_FromStringAndSize((const char *)digest, MD5_DIGESTSIZE); } @@ -157,7 +151,9 @@ MD5Type_hexdigest_impl(MD5object *self) /*[clinic end generated code: output=17badced1f3ac932 input=b60b19de644798dd]*/ { uint8_t digest[MD5_DIGESTSIZE]; - md5_digest_compute_with_lock(self, digest); + HASHLIB_ACQUIRE_LOCK(self); + Hacl_Hash_MD5_digest(self->hash_state, digest); + HASHLIB_RELEASE_LOCK(self); return _Py_strhex((const char *)digest, MD5_DIGESTSIZE); } @@ -197,13 +193,11 @@ MD5Type_update_impl(MD5object *self, PyObject *obj) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - if (buf.len > 0) { - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - _hacl_md5_state_update(self->hash_state, buf.buf, buf.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS - } + Py_BEGIN_ALLOW_THREADS + HASHLIB_ACQUIRE_LOCK(self); + _hacl_md5_state_update(self->hash_state, buf.buf, buf.len); + HASHLIB_RELEASE_LOCK(self); + Py_END_ALLOW_THREADS PyBuffer_Release(&buf); Py_RETURN_NONE; } diff --git a/Modules/sha1module.c b/Modules/sha1module.c index c68caeb6a3ca93..806dbab59e9337 100644 --- a/Modules/sha1module.c +++ b/Modules/sha1module.c @@ -196,13 +196,11 @@ SHA1Type_update_impl(SHA1object *self, PyObject *obj) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - if (buf.len > 0) { - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - _hacl_sha1_state_update(self->hash_state, buf.buf, buf.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS - } + Py_BEGIN_ALLOW_THREADS + HASHLIB_ACQUIRE_LOCK(self); + _hacl_sha1_state_update(self->hash_state, buf.buf, buf.len); + HASHLIB_RELEASE_LOCK(self); + Py_END_ALLOW_THREADS PyBuffer_Release(&buf); Py_RETURN_NONE; } diff --git a/Modules/sha2module.c b/Modules/sha2module.c index cf2ca265c921b5..d8530284d258e7 100644 --- a/Modules/sha2module.c +++ b/Modules/sha2module.c @@ -406,13 +406,11 @@ SHA256Type_update_impl(SHA256object *self, PyObject *obj) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - if (buf.len > 0) { - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - _hacl_sha2_state_update_256(self->state, buf.buf, buf.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS - } + Py_BEGIN_ALLOW_THREADS + HASHLIB_ACQUIRE_LOCK(self); + _hacl_sha2_state_update_256(self->state, buf.buf, buf.len); + HASHLIB_RELEASE_LOCK(self); + Py_END_ALLOW_THREADS PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -432,13 +430,11 @@ SHA512Type_update_impl(SHA512object *self, PyObject *obj) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - if (buf.len > 0) { - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - _hacl_sha2_state_update_512(self->state, buf.buf, buf.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS - } + Py_BEGIN_ALLOW_THREADS + HASHLIB_ACQUIRE_LOCK(self); + _hacl_sha2_state_update_512(self->state, buf.buf, buf.len); + HASHLIB_RELEASE_LOCK(self); + Py_END_ALLOW_THREADS PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -793,11 +789,9 @@ _sha2_sha384_impl(PyObject *module, PyObject *data, int usedforsecurity, if (string) { /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - if (buf.len > 0) { - Py_BEGIN_ALLOW_THREADS - _hacl_sha2_state_update_512(new->state, buf.buf, buf.len); - Py_END_ALLOW_THREADS - } + Py_BEGIN_ALLOW_THREADS + _hacl_sha2_state_update_512(new->state, buf.buf, buf.len); + Py_END_ALLOW_THREADS PyBuffer_Release(&buf); } diff --git a/Modules/sha3module.c b/Modules/sha3module.c index e12aaa9966a02f..c83caa920c4bd8 100644 --- a/Modules/sha3module.c +++ b/Modules/sha3module.c @@ -162,11 +162,9 @@ py_sha3_new_impl(PyTypeObject *type, PyObject *data_obj, int usedforsecurity, GET_BUFFER_VIEW_OR_ERROR(data, &buf, goto error); /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - if (buf.len > 0) { - Py_BEGIN_ALLOW_THREADS - sha3_update(self->hash_state, buf.buf, buf.len); - Py_END_ALLOW_THREADS - } + Py_BEGIN_ALLOW_THREADS + sha3_update(self->hash_state, buf.buf, buf.len); + Py_END_ALLOW_THREADS } PyBuffer_Release(&buf); @@ -298,13 +296,11 @@ _sha3_sha3_224_update_impl(SHA3object *self, PyObject *data) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(data, &buf); - if (buf.len > 0) { - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - sha3_update(self->hash_state, buf.buf, buf.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS - } + Py_BEGIN_ALLOW_THREADS + HASHLIB_ACQUIRE_LOCK(self); + sha3_update(self->hash_state, buf.buf, buf.len); + HASHLIB_RELEASE_LOCK(self); + Py_END_ALLOW_THREADS PyBuffer_Release(&buf); Py_RETURN_NONE; } From ead20a1b6fa840fc0f2eef216dc3cd17e06adeb9 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, 16 Jun 2025 12:38:09 +0200 Subject: [PATCH 12/39] consistency fixes --- Modules/blake2module.c | 8 +++++--- Modules/sha3module.c | 7 ++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Modules/blake2module.c b/Modules/blake2module.c index f7b44625666b99..3ad68a8de2f080 100644 --- a/Modules/blake2module.c +++ b/Modules/blake2module.c @@ -418,7 +418,7 @@ new_Blake2Object(PyTypeObject *type) } while (0) static void -blake2_update_state_unlocked(Blake2Object *self, uint8_t *buf, Py_ssize_t len) +blake2_update_unlocked(Blake2Object *self, uint8_t *buf, Py_ssize_t len) { switch (self->impl) { // blake2b_256_state and blake2s_128_state must be if'd since @@ -642,8 +642,10 @@ py_blake2_new(PyTypeObject *type, PyObject *data, int digest_size, if (data != NULL) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROR(data, &buf, goto error); + /* Do not use self->mutex here as this is the constructor + * where it is not yet possible to have concurrent access. */ Py_BEGIN_ALLOW_THREADS - blake2_update_state_unlocked(self, buf.buf, buf.len); + blake2_update_unlocked(self, buf.buf, buf.len); Py_END_ALLOW_THREADS PyBuffer_Release(&buf); } @@ -819,7 +821,7 @@ _blake2_blake2b_update_impl(Blake2Object *self, PyObject *data) GET_BUFFER_VIEW_OR_ERROUT(data, &buf); Py_BEGIN_ALLOW_THREADS HASHLIB_ACQUIRE_LOCK(self); - blake2_update_state_unlocked(self, buf.buf, buf.len); + blake2_update_unlocked(self, buf.buf, buf.len); HASHLIB_RELEASE_LOCK(self); Py_END_ALLOW_THREADS PyBuffer_Release(&buf); diff --git a/Modules/sha3module.c b/Modules/sha3module.c index c83caa920c4bd8..09b280eca43985 100644 --- a/Modules/sha3module.c +++ b/Modules/sha3module.c @@ -81,7 +81,8 @@ newSHA3object(PyTypeObject *type) } static void -sha3_update(Hacl_Hash_SHA3_state_t *state, uint8_t *buf, Py_ssize_t len) +_hacl_sha3_state_update(Hacl_Hash_SHA3_state_t *state, + uint8_t *buf, Py_ssize_t len) { /* * Note: we explicitly ignore the error code on the basis that it would @@ -163,7 +164,7 @@ py_sha3_new_impl(PyTypeObject *type, PyObject *data_obj, int usedforsecurity, /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ Py_BEGIN_ALLOW_THREADS - sha3_update(self->hash_state, buf.buf, buf.len); + _hacl_sha3_state_update(self->hash_state, buf.buf, buf.len); Py_END_ALLOW_THREADS } @@ -298,7 +299,7 @@ _sha3_sha3_224_update_impl(SHA3object *self, PyObject *data) GET_BUFFER_VIEW_OR_ERROUT(data, &buf); Py_BEGIN_ALLOW_THREADS HASHLIB_ACQUIRE_LOCK(self); - sha3_update(self->hash_state, buf.buf, buf.len); + _hacl_sha3_state_update(self->hash_state, buf.buf, buf.len); HASHLIB_RELEASE_LOCK(self); Py_END_ALLOW_THREADS PyBuffer_Release(&buf); From 68a6bbcc11c85b0beb78a239123ff9052201ec50 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, 16 Jun 2025 12:41:26 +0200 Subject: [PATCH 13/39] remove unused import --- Lib/test/support/hashlib_helper.py | 1 - Lib/test/test_hashlib.py | 1 - 2 files changed, 2 deletions(-) diff --git a/Lib/test/support/hashlib_helper.py b/Lib/test/support/hashlib_helper.py index c0d5da042d8bcf..da318a307e8f42 100644 --- a/Lib/test/support/hashlib_helper.py +++ b/Lib/test/support/hashlib_helper.py @@ -1,6 +1,5 @@ import functools import hashlib -import importlib import unittest from test.support.import_helper import import_module diff --git a/Lib/test/test_hashlib.py b/Lib/test/test_hashlib.py index a9469504b504ed..44ebb884109efd 100644 --- a/Lib/test/test_hashlib.py +++ b/Lib/test/test_hashlib.py @@ -20,7 +20,6 @@ import unittest from test import support from test.support import _4G, bigmemtest -from test.support import hashlib_helper from test.support.import_helper import import_fresh_module from test.support import requires_resource from test.support import threading_helper From 68f297e0c3a82581788c33c5a1ab53ed068d49b4 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, 16 Jun 2025 13:11:07 +0200 Subject: [PATCH 14/39] correct naming for locked/unlocked versions --- Modules/blake2module.c | 4 ++-- Modules/hmacmodule.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Modules/blake2module.c b/Modules/blake2module.c index 3ad68a8de2f080..07920d5d3da53d 100644 --- a/Modules/blake2module.c +++ b/Modules/blake2module.c @@ -737,7 +737,7 @@ py_blake2s_new_impl(PyTypeObject *type, PyObject *data_obj, int digest_size, } static int -blake2_blake2b_copy_locked(Blake2Object *self, Blake2Object *cpy) +blake2_blake2b_copy_unlocked(Blake2Object *self, Blake2Object *cpy) { assert(cpy != NULL); #define BLAKE2_COPY(TYPE, STATE_ATTR) \ @@ -795,7 +795,7 @@ _blake2_blake2b_copy_impl(Blake2Object *self) } HASHLIB_ACQUIRE_LOCK(self); - rc = blake2_blake2b_copy_locked(self, cpy); + rc = blake2_blake2b_copy_unlocked(self, cpy); HASHLIB_RELEASE_LOCK(self); if (rc < 0) { Py_DECREF(cpy); diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index f4b72fb832f052..59de3c1525778c 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -923,7 +923,7 @@ _hmac_HMAC_update_impl(HMACObject *self, PyObject *msgobj) * Note: this function may raise a MemoryError. */ static int -hmac_digest_compute_cond_lock(HMACObject *self, uint8_t *digest) +hmac_digest_compute_locked(HMACObject *self, uint8_t *digest) { assert(digest != NULL); hacl_errno_t rc; @@ -951,7 +951,7 @@ _hmac_HMAC_digest_impl(HMACObject *self) { assert(self->digest_size <= Py_hmac_hash_max_digest_size); uint8_t digest[Py_hmac_hash_max_digest_size]; - if (hmac_digest_compute_cond_lock(self, digest) < 0) { + if (hmac_digest_compute_locked(self, digest) < 0) { return NULL; } return PyBytes_FromStringAndSize((const char *)digest, self->digest_size); @@ -974,7 +974,7 @@ _hmac_HMAC_hexdigest_impl(HMACObject *self) { assert(self->digest_size <= Py_hmac_hash_max_digest_size); uint8_t digest[Py_hmac_hash_max_digest_size]; - if (hmac_digest_compute_cond_lock(self, digest) < 0) { + if (hmac_digest_compute_locked(self, digest) < 0) { return NULL; } return _Py_strhex((const char *)digest, self->digest_size); From 9817c3dc0123693d722b8c157ab9ea5b582296cd 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, 16 Jun 2025 13:13:25 +0200 Subject: [PATCH 15/39] debug? --- Modules/hmacmodule.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index 59de3c1525778c..ae5063b53f5c8b 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -491,7 +491,10 @@ _hacl_hmac_state_update(HACL_HMAC_state *state, uint8_t *buf, Py_ssize_t len) len -= UINT32_MAX; } #endif - assert(len <= UINT32_MAX_AS_SSIZE_T); + if (len > UINT32_MAX_AS_SSIZE_T) { + PyErr_Format(PyExc_ValueError, "invalid length: %zd (max: %ju)", len, UINT32_MAX); + return -1; + } return _hacl_hmac_state_update_once(state, buf, len); } From c14c87d2e7fe20619ee724654f2f9336e6cbc511 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, 16 Jun 2025 13:30:52 +0200 Subject: [PATCH 16/39] simplify HMAC --- Modules/hmacmodule.c | 29 +++++------------------------ 1 file changed, 5 insertions(+), 24 deletions(-) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index ae5063b53f5c8b..99ed022abce940 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -728,29 +728,6 @@ hmac_new_initial_state(HMACObject *self, uint8_t *key, Py_ssize_t len) return self->state == NULL ? -1 : 0; } -/* - * Feed initial data. - * - * This function MUST only be called by the HMAC object constructor - * and after hmac_set_hinfo() and hmac_new_initial_state() have been - * called, lest the behaviour is undefined. - * - * Return 0 on success; otherwise, set an exception and return -1 on failure. - */ -static int -hmac_feed_initial_data(HMACObject *self, uint8_t *msg, Py_ssize_t len) -{ - assert(self->name != NULL); - assert(self->state != NULL); - int rc = 0; - /* Do not use self->mutex here as this is the constructor - * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - rc = _hacl_hmac_state_update(self->state, msg, len); - Py_END_ALLOW_THREADS - return rc; -} - /*[clinic input] _hmac.new @@ -797,7 +774,11 @@ _hmac_new_impl(PyObject *module, PyObject *keyobj, PyObject *msgobj, if (msgobj != NULL && msgobj != Py_None) { Py_buffer msg; GET_BUFFER_VIEW_OR_ERROR(msgobj, &msg, goto error); - rc = hmac_feed_initial_data(self, msg.buf, msg.len); + /* Do not use self->mutex here as this is the constructor + * where it is not yet possible to have concurrent access. */ + Py_BEGIN_ALLOW_THREADS + rc = _hacl_hmac_state_update(self->state, msg.buf, msg.len); + Py_END_ALLOW_THREADS PyBuffer_Release(&msg); #ifndef NDEBUG if (rc < 0) { From bfb543622d0cfd9c820183815bd43059fbb642d0 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, 16 Jun 2025 13:48:53 +0200 Subject: [PATCH 17/39] release the GIL for large buffers --- Modules/blake2module.c | 16 +++++++-------- Modules/hashlib.h | 23 +++++++++++++++++++++ Modules/hmacmodule.c | 16 +++++++-------- Modules/md5module.c | 16 +++++++-------- Modules/sha1module.c | 16 +++++++-------- Modules/sha2module.c | 46 ++++++++++++++++++++++-------------------- Modules/sha3module.c | 16 +++++++-------- 7 files changed, 87 insertions(+), 62 deletions(-) diff --git a/Modules/blake2module.c b/Modules/blake2module.c index 07920d5d3da53d..0f071958d2db23 100644 --- a/Modules/blake2module.c +++ b/Modules/blake2module.c @@ -644,9 +644,10 @@ py_blake2_new(PyTypeObject *type, PyObject *data, int digest_size, GET_BUFFER_VIEW_OR_ERROR(data, &buf, goto error); /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - blake2_update_unlocked(self, buf.buf, buf.len); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS( + buf.len, + blake2_update_unlocked(self, buf.buf, buf.len) + ) PyBuffer_Release(&buf); } @@ -819,11 +820,10 @@ _blake2_blake2b_update_impl(Blake2Object *self, PyObject *data) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(data, &buf); - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - blake2_update_unlocked(self, buf.buf, buf.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX( + self, buf.len, + blake2_update_unlocked(self, buf.buf, buf.len) + ) PyBuffer_Release(&buf); Py_RETURN_NONE; } diff --git a/Modules/hashlib.h b/Modules/hashlib.h index d33f144d79f0a9..35f2ee1a607a79 100644 --- a/Modules/hashlib.h +++ b/Modules/hashlib.h @@ -63,6 +63,29 @@ (OBJ)->mutex = (PyMutex){0}; \ } while (0) +#define HASHLIB_GIL_MINSIZE 2048 +#define HASHLIB_EXTERNAL_INSTRUCTIONS(SIZE, STATEMENTS) \ + if ((SIZE) > HASHLIB_GIL_MINSIZE) { \ + Py_BEGIN_ALLOW_THREADS \ + STATEMENTS; \ + Py_END_ALLOW_THREADS \ + } \ + else { \ + STATEMENTS; \ + } + +#define HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX(OBJ, SIZE, STATEMENTS) \ + if ((SIZE) > HASHLIB_GIL_MINSIZE) { \ + Py_BEGIN_ALLOW_THREADS \ + HASHLIB_ACQUIRE_LOCK(OBJ); \ + STATEMENTS; \ + HASHLIB_RELEASE_LOCK(OBJ); \ + Py_END_ALLOW_THREADS \ + } \ + else { \ + STATEMENTS; \ + } + static inline int _Py_hashlib_data_argument(PyObject **res, PyObject *data, PyObject *string) { diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index 99ed022abce940..a98b0ead82867c 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -776,9 +776,10 @@ _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. */ - Py_BEGIN_ALLOW_THREADS - rc = _hacl_hmac_state_update(self->state, msg.buf, msg.len); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS( + msg.len, + _hacl_hmac_state_update(self->state, msg.buf, msg.len) + ); PyBuffer_Release(&msg); #ifndef NDEBUG if (rc < 0) { @@ -887,11 +888,10 @@ _hmac_HMAC_update_impl(HMACObject *self, PyObject *msgobj) int rc = 0; Py_buffer msg; GET_BUFFER_VIEW_OR_ERROUT(msgobj, &msg); - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - rc = _hacl_hmac_state_update(self->state, msg.buf, msg.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX( + self, msg.len, + rc = _hacl_hmac_state_update(self->state, msg.buf, msg.len) + ) PyBuffer_Release(&msg); return rc < 0 ? NULL : Py_None; } diff --git a/Modules/md5module.c b/Modules/md5module.c index 7f94f6bf6ad9f1..a5c0dc350fb5ea 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -193,11 +193,10 @@ MD5Type_update_impl(MD5object *self, PyObject *obj) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - _hacl_md5_state_update(self->hash_state, buf.buf, buf.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX( + self, buf.len, + _hacl_md5_state_update(self->hash_state, buf.buf, buf.len) + ) PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -301,9 +300,10 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, if (string) { /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - _hacl_md5_state_update(new->hash_state, buf.buf, buf.len); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS( + buf.len, + _hacl_md5_state_update(new->hash_state, buf.buf, buf.len) + ) PyBuffer_Release(&buf); } diff --git a/Modules/sha1module.c b/Modules/sha1module.c index 806dbab59e9337..bdeae1e36f0f7a 100644 --- a/Modules/sha1module.c +++ b/Modules/sha1module.c @@ -196,11 +196,10 @@ SHA1Type_update_impl(SHA1object *self, PyObject *obj) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - _hacl_sha1_state_update(self->hash_state, buf.buf, buf.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX( + self, buf.len, + _hacl_sha1_state_update(self->hash_state, buf.buf, buf.len) + ) PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -303,9 +302,10 @@ _sha1_sha1_impl(PyObject *module, PyObject *data, int usedforsecurity, if (string) { /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - _hacl_sha1_state_update(new->hash_state, buf.buf, buf.len); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS( + buf.len, + _hacl_sha1_state_update(new->hash_state, buf.buf, buf.len) + ) PyBuffer_Release(&buf); } diff --git a/Modules/sha2module.c b/Modules/sha2module.c index d8530284d258e7..12026888bc6537 100644 --- a/Modules/sha2module.c +++ b/Modules/sha2module.c @@ -406,11 +406,10 @@ SHA256Type_update_impl(SHA256object *self, PyObject *obj) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - _hacl_sha2_state_update_256(self->state, buf.buf, buf.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX( + self, buf.len, + _hacl_sha2_state_update_256(self->state, buf.buf, buf.len) + ) PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -430,11 +429,10 @@ SHA512Type_update_impl(SHA512object *self, PyObject *obj) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - _hacl_sha2_state_update_512(self->state, buf.buf, buf.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX( + self, buf.len, + _hacl_sha2_state_update_512(self->state, buf.buf, buf.len) + ) PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -616,9 +614,10 @@ _sha2_sha256_impl(PyObject *module, PyObject *data, int usedforsecurity, if (string) { /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - _hacl_sha2_state_update_256(new->state, buf.buf, buf.len); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS( + buf.len, + _hacl_sha2_state_update_256(new->state, buf.buf, buf.len) + ) PyBuffer_Release(&buf); } @@ -673,9 +672,10 @@ _sha2_sha224_impl(PyObject *module, PyObject *data, int usedforsecurity, if (string) { /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - _hacl_sha2_state_update_256(new->state, buf.buf, buf.len); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS( + buf.len, + _hacl_sha2_state_update_256(new->state, buf.buf, buf.len) + ) PyBuffer_Release(&buf); } @@ -731,9 +731,10 @@ _sha2_sha512_impl(PyObject *module, PyObject *data, int usedforsecurity, if (string) { /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - _hacl_sha2_state_update_512(new->state, buf.buf, buf.len); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS( + buf.len, + _hacl_sha2_state_update_512(new->state, buf.buf, buf.len) + ) PyBuffer_Release(&buf); } @@ -789,9 +790,10 @@ _sha2_sha384_impl(PyObject *module, PyObject *data, int usedforsecurity, if (string) { /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - _hacl_sha2_state_update_512(new->state, buf.buf, buf.len); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS( + buf.len, + _hacl_sha2_state_update_512(new->state, buf.buf, buf.len) + ) PyBuffer_Release(&buf); } diff --git a/Modules/sha3module.c b/Modules/sha3module.c index 09b280eca43985..3d65ff472f573d 100644 --- a/Modules/sha3module.c +++ b/Modules/sha3module.c @@ -163,9 +163,10 @@ py_sha3_new_impl(PyTypeObject *type, PyObject *data_obj, int usedforsecurity, GET_BUFFER_VIEW_OR_ERROR(data, &buf, goto error); /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - _hacl_sha3_state_update(self->hash_state, buf.buf, buf.len); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS( + buf.len, + _hacl_sha3_state_update(self->hash_state, buf.buf, buf.len) + ) } PyBuffer_Release(&buf); @@ -297,11 +298,10 @@ _sha3_sha3_224_update_impl(SHA3object *self, PyObject *data) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(data, &buf); - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - _hacl_sha3_state_update(self->hash_state, buf.buf, buf.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX( + self, buf.len, + _hacl_sha3_state_update(self->hash_state, buf.buf, buf.len) + ) PyBuffer_Release(&buf); Py_RETURN_NONE; } From 923c05f772eb752f8babc6464bb3c7d29dff1a10 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, 16 Jun 2025 13:53:25 +0200 Subject: [PATCH 18/39] restore GIL_MINSIZE --- Lib/test/support/hashlib_helper.py | 20 ++++++++++++++++ Lib/test/test_hashlib.py | 37 +++++++++++++++++++++++++++++- Lib/test/test_hmac.py | 24 +++++++++++++++++++ Modules/_hashopenssl.c | 12 ++++++++++ Modules/blake2module.c | 2 ++ Modules/hmacmodule.c | 17 ++++++++++++++ Modules/md5module.c | 3 +++ Modules/sha1module.c | 6 +++++ Modules/sha2module.c | 7 ++++++ Modules/sha3module.c | 3 +++ 10 files changed, 130 insertions(+), 1 deletion(-) diff --git a/Lib/test/support/hashlib_helper.py b/Lib/test/support/hashlib_helper.py index da318a307e8f42..7032257b06877a 100644 --- a/Lib/test/support/hashlib_helper.py +++ b/Lib/test/support/hashlib_helper.py @@ -1,5 +1,6 @@ import functools import hashlib +import importlib import unittest from test.support.import_helper import import_module @@ -307,3 +308,22 @@ def sha3_384(self): @property def sha3_512(self): return self._find_constructor_in("_sha3","sha3_512") + + +def find_gil_minsize(modules_names, default=2048): + """Get the largest GIL_MINSIZE value for the given cryptographic modules. + + The valid module names are the following: + + - _hashlib + - _md5, _sha1, _sha2, _sha3, _blake2 + - _hmac + """ + sizes = [] + for module_name in modules_names: + try: + module = importlib.import_module(module_name) + except ImportError: + continue + sizes.append(getattr(module, '_GIL_MINSIZE', default)) + return max(sizes, default=default) diff --git a/Lib/test/test_hashlib.py b/Lib/test/test_hashlib.py index 44ebb884109efd..b83ae181718b7a 100644 --- a/Lib/test/test_hashlib.py +++ b/Lib/test/test_hashlib.py @@ -20,6 +20,7 @@ import unittest from test import support from test.support import _4G, bigmemtest +from test.support import hashlib_helper from test.support.import_helper import import_fresh_module from test.support import requires_resource from test.support import threading_helper @@ -408,7 +409,7 @@ def test_large_update(self): aas = b'a' * 128 bees = b'b' * 127 cees = b'c' * 126 - dees = b'd' * 2048 + dees = b'd' * 2048 # HASHLIB_GIL_MINSIZE for cons in self.hash_constructors: m1 = cons(usedforsecurity=False) @@ -989,6 +990,40 @@ def test_case_shake256_vector(self): for msg, md in read_vectors('shake_256'): self.check('shake_256', msg, md, True) + def test_gil(self): + # Check things work fine with an input larger than the size required + # for multithreaded operation. Currently, all cryptographic modules + # have the same constant value (2048) but in the future it might not + # be the case. + mods = ['_md5', '_sha1', '_sha2', '_sha3', '_blake2', '_hashlib'] + gil_minsize = hashlib_helper.find_gil_minsize(mods) + for cons in self.hash_constructors: + # constructors belong to one of the above modules + m = cons(usedforsecurity=False) + m.update(b'1') + m.update(b'#' * gil_minsize) + m.update(b'1') + + m = cons(b'x' * gil_minsize, usedforsecurity=False) + m.update(b'1') + + def test_sha256_gil(self): + gil_minsize = hashlib_helper.find_gil_minsize(['_sha2', '_hashlib']) + m = hashlib.sha256() + m.update(b'1') + m.update(b'#' * gil_minsize) + m.update(b'1') + self.assertEqual( + m.hexdigest(), + '1cfceca95989f51f658e3f3ffe7f1cd43726c9e088c13ee10b46f57cef135b94' + ) + + m = hashlib.sha256(b'1' + b'#' * gil_minsize + b'1') + self.assertEqual( + m.hexdigest(), + '1cfceca95989f51f658e3f3ffe7f1cd43726c9e088c13ee10b46f57cef135b94' + ) + @threading_helper.reap_threads @threading_helper.requires_working_threading() def test_threaded_hashing(self): diff --git a/Lib/test/test_hmac.py b/Lib/test/test_hmac.py index d1f4662adbb618..ff6e1bce0ef801 100644 --- a/Lib/test/test_hmac.py +++ b/Lib/test/test_hmac.py @@ -1133,6 +1133,11 @@ def HMAC(self, key, msg=None): """Create a HMAC object.""" raise NotImplementedError + @property + def gil_minsize(self): + """Get the maximal input length for the GIL to be held.""" + raise NotImplementedError + def check_update(self, key, chunks): chunks = list(chunks) msg = b''.join(chunks) @@ -1150,6 +1155,13 @@ def test_update(self): with self.subTest(key=key, msg=msg): self.check_update(key, [msg]) + def test_update_large(self): + gil_minsize = self.gil_minsize + key = random.randbytes(16) + top = random.randbytes(gil_minsize + 1) + bot = random.randbytes(gil_minsize + 1) + self.check_update(key, [top, bot]) + def test_update_exceptions(self): h = self.HMAC(b"key") for msg in ['invalid msg', 123, (), []]: @@ -1163,6 +1175,10 @@ class PyUpdateTestCase(PyModuleMixin, UpdateTestCaseMixin, unittest.TestCase): def HMAC(self, key, msg=None): return self.hmac.HMAC(key, msg, digestmod='sha256') + @property + def gil_minsize(self): + return sha2._GIL_MINSIZE + @hashlib_helper.requires_openssl_hashdigest('sha256') class OpenSSLUpdateTestCase(UpdateTestCaseMixin, unittest.TestCase): @@ -1170,6 +1186,10 @@ class OpenSSLUpdateTestCase(UpdateTestCaseMixin, unittest.TestCase): def HMAC(self, key, msg=None): return _hashlib.hmac_new(key, msg, digestmod='sha256') + @property + def gil_minsize(self): + return _hashlib._GIL_MINSIZE + class BuiltinUpdateTestCase(BuiltinModuleMixin, UpdateTestCaseMixin, unittest.TestCase): @@ -1179,6 +1199,10 @@ def HMAC(self, key, msg=None): # are still built, making it possible to use SHA-2 hashes. return self.hmac.new(key, msg, digestmod='sha256') + @property + def gil_minsize(self): + return self.hmac._GIL_MINSIZE + class CopyBaseTestCase: diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 1a8691035295bb..08248af71da16d 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -2458,6 +2458,17 @@ hashlib_exception(PyObject *module) return 0; } +static int +hashlib_constants(PyObject *module) +{ + if (PyModule_AddIntConstant(module, "_GIL_MINSIZE", + HASHLIB_GIL_MINSIZE) < 0) + { + return -1; + } + return 0; +} + static PyModuleDef_Slot hashlib_slots[] = { {Py_mod_exec, hashlib_init_hashtable}, {Py_mod_exec, hashlib_init_HASH_type}, @@ -2466,6 +2477,7 @@ static PyModuleDef_Slot hashlib_slots[] = { {Py_mod_exec, hashlib_md_meth_names}, {Py_mod_exec, hashlib_init_constructors}, {Py_mod_exec, hashlib_exception}, + {Py_mod_exec, hashlib_constants}, {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED}, {Py_mod_gil, Py_MOD_GIL_NOT_USED}, {0, NULL} diff --git a/Modules/blake2module.c b/Modules/blake2module.c index 0f071958d2db23..ecff6467f7b37f 100644 --- a/Modules/blake2module.c +++ b/Modules/blake2module.c @@ -227,6 +227,8 @@ blake2_exec(PyObject *m) } \ } while (0) + ADD_INT_CONST("_GIL_MINSIZE", HASHLIB_GIL_MINSIZE); + st->blake2b_type = (PyTypeObject *)PyType_FromModuleAndSpec( m, &blake2b_type_spec, NULL); diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index a98b0ead82867c..df53c4ef0d69a6 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -1534,6 +1534,20 @@ hmacmodule_init_strings(hmacmodule_state *state) return 0; } +static int +hmacmodule_init_globals(PyObject *module, hmacmodule_state *state) +{ +#define ADD_INT_CONST(NAME, VALUE) \ + do { \ + if (PyModule_AddIntConstant(module, (NAME), (VALUE)) < 0) { \ + return -1; \ + } \ + } while (0) + ADD_INT_CONST("_GIL_MINSIZE", HASHLIB_GIL_MINSIZE); +#undef ADD_INT_CONST + return 0; +} + static void hmacmodule_init_cpu_features(hmacmodule_state *state) { @@ -1624,6 +1638,9 @@ hmacmodule_exec(PyObject *module) if (hmacmodule_init_strings(state) < 0) { return -1; } + if (hmacmodule_init_globals(module, state) < 0) { + return -1; + } hmacmodule_init_cpu_features(state); return 0; } diff --git a/Modules/md5module.c b/Modules/md5module.c index a5c0dc350fb5ea..94a070ba4ba5c6 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -352,6 +352,9 @@ md5_exec(PyObject *m) if (PyModule_AddObjectRef(m, "MD5Type", (PyObject *)st->md5_type) < 0) { return -1; } + if (PyModule_AddIntConstant(m, "_GIL_MINSIZE", HASHLIB_GIL_MINSIZE) < 0) { + return -1; + } return 0; } diff --git a/Modules/sha1module.c b/Modules/sha1module.c index bdeae1e36f0f7a..fd940aa5c6988a 100644 --- a/Modules/sha1module.c +++ b/Modules/sha1module.c @@ -355,6 +355,12 @@ _sha1_exec(PyObject *module) { return -1; } + if (PyModule_AddIntConstant(module, + "_GIL_MINSIZE", + HASHLIB_GIL_MINSIZE) < 0) + { + return -1; + } return 0; } diff --git a/Modules/sha2module.c b/Modules/sha2module.c index 12026888bc6537..7eb397858495a6 100644 --- a/Modules/sha2module.c +++ b/Modules/sha2module.c @@ -877,6 +877,13 @@ static int sha2_exec(PyObject *module) return -1; } + if (PyModule_AddIntConstant(module, + "_GIL_MINSIZE", + HASHLIB_GIL_MINSIZE) < 0) + { + return -1; + } + return 0; } diff --git a/Modules/sha3module.c b/Modules/sha3module.c index 3d65ff472f573d..26e6692e0ad5fd 100644 --- a/Modules/sha3module.c +++ b/Modules/sha3module.c @@ -630,6 +630,9 @@ _sha3_exec(PyObject *m) if (PyModule_AddStringConstant(m, "implementation", "HACL") < 0) { return -1; } + if (PyModule_AddIntConstant(m, "_GIL_MINSIZE", HASHLIB_GIL_MINSIZE) < 0) { + return -1; + } return 0; } From 55b2afabcd516207125c4774111a5c4288b5c890 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, 16 Jun 2025 14:17:21 +0200 Subject: [PATCH 19/39] correctly lock objects --- Lib/test/test_hashlib.py | 10 +++++++-- Modules/_hashopenssl.c | 39 ++++++++++++++-------------------- Modules/blake2module.c | 8 +++---- Modules/hashlib.h | 46 +++++++++++++++++++++++----------------- Modules/hmacmodule.c | 6 +++--- Modules/md5module.c | 8 +++---- Modules/sha1module.c | 8 +++---- Modules/sha2module.c | 24 ++++++++++----------- Modules/sha3module.c | 8 +++---- 9 files changed, 81 insertions(+), 76 deletions(-) diff --git a/Lib/test/test_hashlib.py b/Lib/test/test_hashlib.py index b83ae181718b7a..ca9c561ea803f3 100644 --- a/Lib/test/test_hashlib.py +++ b/Lib/test/test_hashlib.py @@ -1027,17 +1027,23 @@ def test_sha256_gil(self): @threading_helper.reap_threads @threading_helper.requires_working_threading() def test_threaded_hashing(self): + for constructor in self.hash_constructors: + if constructor().name not in self.shakes: + with self.subTest(constructor=constructor): + self.do_test_threaded_hashing(constructor) + + def do_test_threaded_hashing(self, constructor): # Updating the same hash object from several threads at once # using data chunk sizes containing the same byte sequences. # # If the internal locks are working to prevent multiple # updates on the same object from running at once, the resulting # hash will be the same as doing it single threaded upfront. - hasher = hashlib.sha1() + hasher = constructor() num_threads = 5 smallest_data = b'swineflu' data = smallest_data * 200000 - expected_hash = hashlib.sha1(data*num_threads).hexdigest() + expected_hash = constructor(data*num_threads).hexdigest() def hash_in_chunks(chunk_size): index = 0 diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 08248af71da16d..3f454578411d26 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -278,21 +278,15 @@ get_hashlib_state(PyObject *module) } typedef struct { - PyObject_HEAD + PyObject_HASHLIB_HEAD EVP_MD_CTX *ctx; /* OpenSSL message digest context */ - // Prevents undefined behavior via multiple threads entering the C API. - bool use_mutex; - PyMutex mutex; /* OpenSSL context lock */ } HASHobject; #define HASHobject_CAST(op) ((HASHobject *)(op)) typedef struct { - PyObject_HEAD + PyObject_HASHLIB_HEAD HMAC_CTX *ctx; /* OpenSSL hmac context */ - // Prevents undefined behavior via multiple threads entering the C API. - bool use_mutex; - PyMutex mutex; /* HMAC context lock */ } HMACobject; #define HMACobject_CAST(op) ((HMACobject *)(op)) @@ -803,11 +797,10 @@ _hashlib_HASH_update_impl(HASHobject *self, PyObject *obj) int result; Py_buffer view; GET_BUFFER_VIEW_OR_ERROUT(obj, &view); - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - result = _hashlib_HASH_hash(self, view.buf, view.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( + self, HASHLIB_GIL_MINSIZE, + result = _hashlib_HASH_hash(self, view.buf, view.len) + ); PyBuffer_Release(&view); return result < 0 ? NULL : Py_None; } @@ -1114,9 +1107,10 @@ _hashlib_HASH(PyObject *module, const char *digestname, PyObject *data_obj, if (view.buf && view.len) { /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - Py_BEGIN_ALLOW_THREADS - result = _hashlib_HASH_hash(self, view.buf, view.len); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( + view.len, + result = _hashlib_HASH_hash(self, view.buf, view.len) + ); if (result == -1) { assert(PyErr_Occurred()); Py_CLEAR(self); @@ -1810,13 +1804,12 @@ _hmac_update(HMACobject *self, PyObject *obj) Py_buffer view = {0}; GET_BUFFER_VIEW_OR_ERROR(obj, &view, return 0); - Py_BEGIN_ALLOW_THREADS - HASHLIB_ACQUIRE_LOCK(self); - r = HMAC_Update(self->ctx, - (const unsigned char *)view.buf, - (size_t)view.len); - HASHLIB_RELEASE_LOCK(self); - Py_END_ALLOW_THREADS + HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( + self, view.len, + r = HMAC_Update( + self->ctx, (const unsigned char *)view.buf, (size_t)view.len + ) + ); PyBuffer_Release(&view); if (r == 0) { diff --git a/Modules/blake2module.c b/Modules/blake2module.c index ecff6467f7b37f..199b01e5b5c1ca 100644 --- a/Modules/blake2module.c +++ b/Modules/blake2module.c @@ -646,10 +646,10 @@ py_blake2_new(PyTypeObject *type, PyObject *data, int digest_size, GET_BUFFER_VIEW_OR_ERROR(data, &buf, goto error); /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - HASHLIB_EXTERNAL_INSTRUCTIONS( + HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( buf.len, blake2_update_unlocked(self, buf.buf, buf.len) - ) + ); PyBuffer_Release(&buf); } @@ -822,10 +822,10 @@ _blake2_blake2b_update_impl(Blake2Object *self, PyObject *data) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(data, &buf); - HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX( + HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( self, buf.len, blake2_update_unlocked(self, buf.buf, buf.len) - ) + ); PyBuffer_Release(&buf); Py_RETURN_NONE; } diff --git a/Modules/hashlib.h b/Modules/hashlib.h index 35f2ee1a607a79..bb7e50ed6fc794 100644 --- a/Modules/hashlib.h +++ b/Modules/hashlib.h @@ -64,27 +64,33 @@ } while (0) #define HASHLIB_GIL_MINSIZE 2048 -#define HASHLIB_EXTERNAL_INSTRUCTIONS(SIZE, STATEMENTS) \ - if ((SIZE) > HASHLIB_GIL_MINSIZE) { \ - Py_BEGIN_ALLOW_THREADS \ - STATEMENTS; \ - Py_END_ALLOW_THREADS \ - } \ - else { \ - STATEMENTS; \ - } +#define HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED(SIZE, STATEMENTS) \ + do { \ + if ((SIZE) > HASHLIB_GIL_MINSIZE) { \ + Py_BEGIN_ALLOW_THREADS \ + STATEMENTS; \ + Py_END_ALLOW_THREADS \ + } \ + else { \ + STATEMENTS; \ + } \ + } while (0) -#define HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX(OBJ, SIZE, STATEMENTS) \ - if ((SIZE) > HASHLIB_GIL_MINSIZE) { \ - Py_BEGIN_ALLOW_THREADS \ - HASHLIB_ACQUIRE_LOCK(OBJ); \ - STATEMENTS; \ - HASHLIB_RELEASE_LOCK(OBJ); \ - Py_END_ALLOW_THREADS \ - } \ - else { \ - STATEMENTS; \ - } +#define HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED(OBJ, SIZE, STATEMENTS) \ + do { \ + if ((SIZE) > HASHLIB_GIL_MINSIZE) { \ + Py_BEGIN_ALLOW_THREADS \ + HASHLIB_ACQUIRE_LOCK(OBJ); \ + STATEMENTS; \ + HASHLIB_RELEASE_LOCK(OBJ); \ + Py_END_ALLOW_THREADS \ + } \ + else { \ + HASHLIB_ACQUIRE_LOCK(OBJ); \ + STATEMENTS; \ + HASHLIB_RELEASE_LOCK(OBJ); \ + } \ + } while (0) static inline int _Py_hashlib_data_argument(PyObject **res, PyObject *data, PyObject *string) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index df53c4ef0d69a6..18673b98e9c88d 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -776,7 +776,7 @@ _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. */ - HASHLIB_EXTERNAL_INSTRUCTIONS( + HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( msg.len, _hacl_hmac_state_update(self->state, msg.buf, msg.len) ); @@ -888,10 +888,10 @@ _hmac_HMAC_update_impl(HMACObject *self, PyObject *msgobj) int rc = 0; Py_buffer msg; GET_BUFFER_VIEW_OR_ERROUT(msgobj, &msg); - HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX( + HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( self, msg.len, rc = _hacl_hmac_state_update(self->state, msg.buf, msg.len) - ) + ); PyBuffer_Release(&msg); return rc < 0 ? NULL : Py_None; } diff --git a/Modules/md5module.c b/Modules/md5module.c index 94a070ba4ba5c6..e3398541189411 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -193,10 +193,10 @@ MD5Type_update_impl(MD5object *self, PyObject *obj) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX( + HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( self, buf.len, _hacl_md5_state_update(self->hash_state, buf.buf, buf.len) - ) + ); PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -300,10 +300,10 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, if (string) { /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - HASHLIB_EXTERNAL_INSTRUCTIONS( + HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( buf.len, _hacl_md5_state_update(new->hash_state, buf.buf, buf.len) - ) + ); PyBuffer_Release(&buf); } diff --git a/Modules/sha1module.c b/Modules/sha1module.c index fd940aa5c6988a..cb7bcd5a078326 100644 --- a/Modules/sha1module.c +++ b/Modules/sha1module.c @@ -196,10 +196,10 @@ SHA1Type_update_impl(SHA1object *self, PyObject *obj) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX( + HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( self, buf.len, _hacl_sha1_state_update(self->hash_state, buf.buf, buf.len) - ) + ); PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -302,10 +302,10 @@ _sha1_sha1_impl(PyObject *module, PyObject *data, int usedforsecurity, if (string) { /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - HASHLIB_EXTERNAL_INSTRUCTIONS( + HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( buf.len, _hacl_sha1_state_update(new->hash_state, buf.buf, buf.len) - ) + ); PyBuffer_Release(&buf); } diff --git a/Modules/sha2module.c b/Modules/sha2module.c index 7eb397858495a6..f4c4ba3254849b 100644 --- a/Modules/sha2module.c +++ b/Modules/sha2module.c @@ -406,10 +406,10 @@ SHA256Type_update_impl(SHA256object *self, PyObject *obj) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX( + HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( self, buf.len, _hacl_sha2_state_update_256(self->state, buf.buf, buf.len) - ) + ); PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -429,10 +429,10 @@ SHA512Type_update_impl(SHA512object *self, PyObject *obj) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); - HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX( + HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( self, buf.len, _hacl_sha2_state_update_512(self->state, buf.buf, buf.len) - ) + ); PyBuffer_Release(&buf); Py_RETURN_NONE; } @@ -614,10 +614,10 @@ _sha2_sha256_impl(PyObject *module, PyObject *data, int usedforsecurity, if (string) { /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - HASHLIB_EXTERNAL_INSTRUCTIONS( + HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( buf.len, _hacl_sha2_state_update_256(new->state, buf.buf, buf.len) - ) + ); PyBuffer_Release(&buf); } @@ -672,10 +672,10 @@ _sha2_sha224_impl(PyObject *module, PyObject *data, int usedforsecurity, if (string) { /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - HASHLIB_EXTERNAL_INSTRUCTIONS( + HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( buf.len, _hacl_sha2_state_update_256(new->state, buf.buf, buf.len) - ) + ); PyBuffer_Release(&buf); } @@ -731,10 +731,10 @@ _sha2_sha512_impl(PyObject *module, PyObject *data, int usedforsecurity, if (string) { /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - HASHLIB_EXTERNAL_INSTRUCTIONS( + HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( buf.len, _hacl_sha2_state_update_512(new->state, buf.buf, buf.len) - ) + ); PyBuffer_Release(&buf); } @@ -790,10 +790,10 @@ _sha2_sha384_impl(PyObject *module, PyObject *data, int usedforsecurity, if (string) { /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - HASHLIB_EXTERNAL_INSTRUCTIONS( + HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( buf.len, _hacl_sha2_state_update_512(new->state, buf.buf, buf.len) - ) + ); PyBuffer_Release(&buf); } diff --git a/Modules/sha3module.c b/Modules/sha3module.c index 26e6692e0ad5fd..f9983fe509b36c 100644 --- a/Modules/sha3module.c +++ b/Modules/sha3module.c @@ -163,10 +163,10 @@ py_sha3_new_impl(PyTypeObject *type, PyObject *data_obj, int usedforsecurity, GET_BUFFER_VIEW_OR_ERROR(data, &buf, goto error); /* Do not use self->mutex here as this is the constructor * where it is not yet possible to have concurrent access. */ - HASHLIB_EXTERNAL_INSTRUCTIONS( + HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( buf.len, _hacl_sha3_state_update(self->hash_state, buf.buf, buf.len) - ) + ); } PyBuffer_Release(&buf); @@ -298,10 +298,10 @@ _sha3_sha3_224_update_impl(SHA3object *self, PyObject *data) { Py_buffer buf; GET_BUFFER_VIEW_OR_ERROUT(data, &buf); - HASHLIB_EXTERNAL_INSTRUCTIONS_WITH_MUTEX( + HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( self, buf.len, _hacl_sha3_state_update(self->hash_state, buf.buf, buf.len) - ) + ); PyBuffer_Release(&buf); Py_RETURN_NONE; } From 5cd60d1556b5632359ba43d8b6871690db94ceb0 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, 16 Jun 2025 14:27:22 +0200 Subject: [PATCH 20/39] improve tests --- Lib/test/test_hashlib.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_hashlib.py b/Lib/test/test_hashlib.py index ca9c561ea803f3..1955d0b2a9e9c5 100644 --- a/Lib/test/test_hashlib.py +++ b/Lib/test/test_hashlib.py @@ -1027,28 +1027,30 @@ def test_sha256_gil(self): @threading_helper.reap_threads @threading_helper.requires_working_threading() def test_threaded_hashing(self): - for constructor in self.hash_constructors: - if constructor().name not in self.shakes: - with self.subTest(constructor=constructor): - self.do_test_threaded_hashing(constructor) + for algorithm, constructors in self.constructors_to_test.items(): + is_shake = algorithm in self.shakes + for constructor in constructors: + with self.subTest(constructor=constructor, is_shake=is_shake): + self.do_test_threaded_hashing(constructor, is_shake) - def do_test_threaded_hashing(self, constructor): + def do_test_threaded_hashing(self, constructor, is_shake): # Updating the same hash object from several threads at once # using data chunk sizes containing the same byte sequences. # # If the internal locks are working to prevent multiple # updates on the same object from running at once, the resulting # hash will be the same as doing it single threaded upfront. - hasher = constructor() num_threads = 5 - smallest_data = b'swineflu' + smallest_data = os.urandom(16) data = smallest_data * 200000 - expected_hash = constructor(data*num_threads).hexdigest() + + h1 = constructor() + h2 = constructor(data * num_threads) def hash_in_chunks(chunk_size): index = 0 while index < len(data): - hasher.update(data[index:index + chunk_size]) + h1.update(data[index:index + chunk_size]) index += chunk_size threads = [] @@ -1065,7 +1067,10 @@ def hash_in_chunks(chunk_size): for thread in threads: thread.join() - self.assertEqual(expected_hash, hasher.hexdigest()) + if is_shake: + self.assertEqual(h1.hexdigest(16), h2.hexdigest(16)) + else: + self.assertEqual(h1.hexdigest(), h2.hexdigest()) def test_get_fips_mode(self): fips_mode = self.is_fips_mode From a2fcbd5f8a3e354a2b5d26d2c802b11fafdb3c47 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, 16 Jun 2025 14:29:02 +0200 Subject: [PATCH 21/39] fixup HMAC --- Modules/hmacmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index 18673b98e9c88d..a80aa1cb5b4103 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -778,7 +778,7 @@ _hmac_new_impl(PyObject *module, PyObject *keyobj, PyObject *msgobj, * where it is not yet possible to have concurrent access. */ HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( msg.len, - _hacl_hmac_state_update(self->state, msg.buf, msg.len) + rc = _hacl_hmac_state_update(self->state, msg.buf, msg.len) ); PyBuffer_Release(&msg); #ifndef NDEBUG From 417cee1587b79462fd39842ed1f2a7f1f5d0dc6f 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, 16 Jun 2025 14:49:57 +0200 Subject: [PATCH 22/39] fixup --- Modules/hmacmodule.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index a80aa1cb5b4103..116374adc773f1 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -390,33 +390,39 @@ narrow_hmac_hash_kind(hmacmodule_state *state, HMAC_Hash_Kind kind) static int _hacl_convert_errno(hacl_errno_t code) { + int res = -1; + PyGILState_STATE gstate = PyGILState_Ensure(); switch (code) { case Hacl_Streaming_Types_Success: { - return 0; + res = 0; + goto finally; } case Hacl_Streaming_Types_InvalidAlgorithm: { - PyErr_Format(PyExc_ValueError, "invalid HACL* algorithm"); - return -1; + PyErr_SetString(PyExc_ValueError, "invalid HACL* algorithm"); + goto finally; } case Hacl_Streaming_Types_InvalidLength: { PyErr_SetString(PyExc_ValueError, "invalid length"); - return -1; + goto finally; } case Hacl_Streaming_Types_MaximumLengthExceeded: { PyErr_SetString(PyExc_OverflowError, "maximum length exceeded"); - return -1; + goto finally; } case Hacl_Streaming_Types_OutOfMemory: { PyErr_NoMemory(); - return -1; + goto finally; } default: { PyErr_Format(PyExc_RuntimeError, "HACL* internal routine failed with error code: %d", code); - return -1; + goto finally; } } +finally: + PyGILState_Release(gstate); + return res; } /* @@ -483,7 +489,7 @@ _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)) { + if (_hacl_hmac_state_update_once(state, buf, UINT32_MAX) < 0) { assert(PyErr_Occurred()); return -1; } @@ -492,7 +498,9 @@ _hacl_hmac_state_update(HACL_HMAC_state *state, uint8_t *buf, Py_ssize_t len) } #endif if (len > UINT32_MAX_AS_SSIZE_T) { + PyGILState_STATE gstate = PyGILState_Ensure(); PyErr_Format(PyExc_ValueError, "invalid length: %zd (max: %ju)", len, UINT32_MAX); + PyGILState_Release(gstate); return -1; } return _hacl_hmac_state_update_once(state, buf, len); @@ -781,13 +789,9 @@ _hmac_new_impl(PyObject *module, PyObject *keyobj, PyObject *msgobj, rc = _hacl_hmac_state_update(self->state, msg.buf, msg.len) ); PyBuffer_Release(&msg); -#ifndef NDEBUG if (rc < 0) { goto error; } -#else - (void)rc; -#endif } assert(rc == 0); PyObject_GC_Track(self); From f350501c4b1264d2e63e9448026c9964cfdfaae9 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, 16 Jun 2025 15:09:58 +0200 Subject: [PATCH 23/39] GIL protection --- Modules/hmacmodule.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index 116374adc773f1..7c8b44c84230a3 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -390,39 +390,36 @@ narrow_hmac_hash_kind(hmacmodule_state *state, HMAC_Hash_Kind kind) static int _hacl_convert_errno(hacl_errno_t code) { - int res = -1; + if (code == Hacl_Streaming_Types_Success) { + return 0; + } PyGILState_STATE gstate = PyGILState_Ensure(); switch (code) { - case Hacl_Streaming_Types_Success: { - res = 0; - goto finally; - } case Hacl_Streaming_Types_InvalidAlgorithm: { PyErr_SetString(PyExc_ValueError, "invalid HACL* algorithm"); - goto finally; + break; } case Hacl_Streaming_Types_InvalidLength: { PyErr_SetString(PyExc_ValueError, "invalid length"); - goto finally; + break; } case Hacl_Streaming_Types_MaximumLengthExceeded: { PyErr_SetString(PyExc_OverflowError, "maximum length exceeded"); - goto finally; + break; } case Hacl_Streaming_Types_OutOfMemory: { PyErr_NoMemory(); - goto finally; + break; } default: { PyErr_Format(PyExc_RuntimeError, "HACL* internal routine failed with error code: %d", code); - goto finally; + break; } } -finally: PyGILState_Release(gstate); - return res; + return -1; } /* From 5c4009dacef9473e8aa783dba50c2bc771c5d231 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, 16 Jun 2025 15:10:08 +0200 Subject: [PATCH 24/39] show WASI errors --- Modules/hmacmodule.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index 7c8b44c84230a3..7cb377e892b1ee 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -496,7 +496,8 @@ _hacl_hmac_state_update(HACL_HMAC_state *state, uint8_t *buf, Py_ssize_t len) #endif if (len > UINT32_MAX_AS_SSIZE_T) { PyGILState_STATE gstate = PyGILState_Ensure(); - PyErr_Format(PyExc_ValueError, "invalid length: %zd (max: %ju)", len, UINT32_MAX); + PyErr_Format(PyExc_ValueError, "invalid length: %zd (max: %u)", + len, UINT32_MAX); PyGILState_Release(gstate); return -1; } From 8aec797072562522f7dc3b618d076a941bc02c3b 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, 16 Jun 2025 15:22:28 +0200 Subject: [PATCH 25/39] fix WASI --- Modules/hmacmodule.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index 7cb377e892b1ee..6d02869ac77c1e 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -494,14 +494,8 @@ _hacl_hmac_state_update(HACL_HMAC_state *state, uint8_t *buf, Py_ssize_t len) len -= UINT32_MAX; } #endif - if (len > UINT32_MAX_AS_SSIZE_T) { - PyGILState_STATE gstate = PyGILState_Ensure(); - PyErr_Format(PyExc_ValueError, "invalid length: %zd (max: %u)", - len, UINT32_MAX); - PyGILState_Release(gstate); - return -1; - } - return _hacl_hmac_state_update_once(state, buf, len); + assert(Py_CHECK_HACL_UINT32_T_LENGTH(len)); + return _hacl_hmac_state_update_once(state, buf, (uint32_t)len); } /* Static information used to construct the hash table. */ @@ -787,9 +781,13 @@ _hmac_new_impl(PyObject *module, PyObject *keyobj, PyObject *msgobj, rc = _hacl_hmac_state_update(self->state, msg.buf, msg.len) ); PyBuffer_Release(&msg); +#ifndef NDEBUG if (rc < 0) { goto error; } +#else + (void)rc; +#endif } assert(rc == 0); PyObject_GC_Track(self); From 6db58dc3b9e0593fca76586b5e54a03d29d4f659 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, 16 Jun 2025 15:24:21 +0200 Subject: [PATCH 26/39] fix compilation --- Modules/hmacmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index 6d02869ac77c1e..c99939961de5fe 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -494,7 +494,7 @@ _hacl_hmac_state_update(HACL_HMAC_state *state, uint8_t *buf, Py_ssize_t len) len -= UINT32_MAX; } #endif - assert(Py_CHECK_HACL_UINT32_T_LENGTH(len)); + Py_CHECK_HACL_UINT32_T_LENGTH(len); return _hacl_hmac_state_update_once(state, buf, (uint32_t)len); } From b1f94635c918dd44676ac4ee0003bef65f3e8310 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, 16 Jun 2025 15:24:44 +0200 Subject: [PATCH 27/39] fix compilation --- Modules/hmacmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index c99939961de5fe..268b6747141f55 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -463,8 +463,8 @@ _hacl_hmac_state_update_once(HACL_HMAC_state *state, uint8_t *buf, Py_ssize_t len) { assert(len >= 0); -#ifndef NDEBUG Py_CHECK_HACL_UINT32_T_LENGTH(len); +#ifndef NDEBUG hacl_errno_t code = Hacl_Streaming_HMAC_update(state, buf, (uint32_t)len); return _hacl_convert_errno(code); #else From 491b9221275e069945bbcf778e9321853704ed72 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, 16 Jun 2025 15:27:06 +0200 Subject: [PATCH 28/39] fix warnings --- Modules/hmacmodule.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index 268b6747141f55..f28e14892b2b00 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -460,15 +460,13 @@ _hacl_hmac_state_free(HACL_HMAC_state *state) */ static int _hacl_hmac_state_update_once(HACL_HMAC_state *state, - uint8_t *buf, Py_ssize_t len) + uint8_t *buf, uint32_t len) { - assert(len >= 0); - Py_CHECK_HACL_UINT32_T_LENGTH(len); #ifndef NDEBUG - hacl_errno_t code = Hacl_Streaming_HMAC_update(state, buf, (uint32_t)len); + hacl_errno_t code = Hacl_Streaming_HMAC_update(state, buf, len); return _hacl_convert_errno(code); #else - (void)Hacl_Streaming_HMAC_update(state, buf, (uint32_t)len); + (void)Hacl_Streaming_HMAC_update(state, buf, len); return 0; #endif } From c048975cd32b714b18f3e5ad6f750113ac24e622 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 17 Jun 2025 11:48:26 +0200 Subject: [PATCH 29/39] sync --- Modules/hmacmodule.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index f28e14892b2b00..7f32bb42744fc2 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -390,9 +390,11 @@ narrow_hmac_hash_kind(hmacmodule_state *state, HMAC_Hash_Kind kind) static int _hacl_convert_errno(hacl_errno_t code) { + assert(PyGILState_GetThisThreadState() != NULL); if (code == Hacl_Streaming_Types_Success) { return 0; } + PyGILState_STATE gstate = PyGILState_Ensure(); switch (code) { case Hacl_Streaming_Types_InvalidAlgorithm: { From c9044d26ea9ae5be780bd21128a66ec0fadcd6ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 17 Jun 2025 11:49:00 +0200 Subject: [PATCH 30/39] fixup format string --- Modules/hmacmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index 7f32bb42744fc2..0e5f7c852dbbfe 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -415,7 +415,7 @@ _hacl_convert_errno(hacl_errno_t code) } default: { PyErr_Format(PyExc_RuntimeError, - "HACL* internal routine failed with error code: %d", + "HACL* internal routine failed with error code: %u", code); break; } From 6c08f0d2fb15f8ec9ac6568851e683bd8275ad36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 17 Jun 2025 18:20:10 +0200 Subject: [PATCH 31/39] address review --- Modules/_hashopenssl.c | 4 +-- Modules/blake2module.c | 2 +- Modules/hashlib.h | 55 +++++++++++++++++++++++++++--------------- Modules/hmacmodule.c | 2 +- Modules/md5module.c | 2 +- Modules/sha1module.c | 2 +- Modules/sha2module.c | 4 +-- Modules/sha3module.c | 2 +- 8 files changed, 44 insertions(+), 29 deletions(-) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 3f454578411d26..4ca5b5698cff3c 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -278,14 +278,14 @@ get_hashlib_state(PyObject *module) } typedef struct { - PyObject_HASHLIB_HEAD + HASHLIB_OBJECT_HEAD EVP_MD_CTX *ctx; /* OpenSSL message digest context */ } HASHobject; #define HASHobject_CAST(op) ((HASHobject *)(op)) typedef struct { - PyObject_HASHLIB_HEAD + HASHLIB_OBJECT_HEAD HMAC_CTX *ctx; /* OpenSSL hmac context */ } HMACobject; diff --git a/Modules/blake2module.c b/Modules/blake2module.c index 199b01e5b5c1ca..295bca07650916 100644 --- a/Modules/blake2module.c +++ b/Modules/blake2module.c @@ -352,7 +352,7 @@ type_to_impl(PyTypeObject *type) } typedef struct { - PyObject_HASHLIB_HEAD + HASHLIB_OBJECT_HEAD union { Hacl_Hash_Blake2s_state_t *blake2s_state; Hacl_Hash_Blake2b_state_t *blake2b_state; diff --git a/Modules/hashlib.h b/Modules/hashlib.h index bb7e50ed6fc794..5800772a374440 100644 --- a/Modules/hashlib.h +++ b/Modules/hashlib.h @@ -3,9 +3,9 @@ #include "pycore_lock.h" // PyMutex /* - * Given a PyObject* obj, fill in the Py_buffer* viewp with the result - * of PyObject_GetBuffer. Sets an exception and issues the erraction - * on any errors, e.g. 'return NULL' or 'goto error'. + * Given a PyObject* 'obj', fill in the Py_buffer* 'viewp' with the result + * of PyObject_GetBuffer. Sets an exception and issues the 'erraction' + * on any errors, e.g., 'return NULL' or 'goto error'. */ #define GET_BUFFER_VIEW_OR_ERROR(obj, viewp, erraction) do { \ if (PyUnicode_Check((obj))) { \ @@ -34,36 +34,46 @@ /* * Helper code to synchronize access to the hash object when the GIL is - * released around a CPU consuming hashlib operation. All code paths that - * access a mutable part of obj must be enclosed in an ENTER_HASHLIB / - * LEAVE_HASHLIB block or explicitly acquire and release the lock inside - * a PY_BEGIN / END_ALLOW_THREADS block if they wish to release the GIL for - * an operation. + * released around a CPU consuming hashlib operation. * - * These only drop the GIL if the lock acquisition itself is likely to - * block. Thus the non-blocking acquire gating the GIL release for a - * blocking lock acquisition. The intent of these macros is to surround - * the assumed always "fast" operations that you aren't releasing the - * GIL around. Otherwise use code similar to what you see in hash - * function update() methods. + * Code accessing a mutable part of the hash object must be enclosed in + * an HASHLIB_{ACQUIRE,RELEASE}_LOCK block or explicitly acquire and release + * the mutex inside a Py_BEGIN_ALLOW_THREADS -- Py_END_ALLOW_THREADS block if + * they wish to release the GIL for an operation. */ -#include "pythread.h" - -#define PyObject_HASHLIB_HEAD \ +#define HASHLIB_OBJECT_HEAD \ PyObject_HEAD \ /* Guard against race conditions during incremental update(). */ \ PyMutex mutex; -#define HASHLIB_ACQUIRE_LOCK(OBJ) PyMutex_Lock(&(OBJ)->mutex) -#define HASHLIB_RELEASE_LOCK(OBJ) PyMutex_Unlock(&(OBJ)->mutex) - #define HASHLIB_INIT_MUTEX(OBJ) \ do { \ (OBJ)->mutex = (PyMutex){0}; \ } while (0) +#define HASHLIB_ACQUIRE_LOCK(OBJ) PyMutex_Lock(&(OBJ)->mutex) +#define HASHLIB_RELEASE_LOCK(OBJ) PyMutex_Unlock(&(OBJ)->mutex) + +/* + * Message length above which the GIL is to be released + * when performing hashing operations. + */ #define HASHLIB_GIL_MINSIZE 2048 + +// Macros for executing code while conditionally holding the GIL. +// +// These only drop the GIL if the lock acquisition itself is likely to +// block. Thus the non-blocking acquire gating the GIL release for a +// blocking lock acquisition. The intent of these macros is to surround +// the assumed always "fast" operations that you aren't releasing the +// GIL around. + +/* + * Execute a suite of C statements 'STATEMENTS'. + * + * The GIL is held if 'SIZE' is below the HASHLIB_GIL_MINSIZE threshold. + */ #define HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED(SIZE, STATEMENTS) \ do { \ if ((SIZE) > HASHLIB_GIL_MINSIZE) { \ @@ -76,6 +86,11 @@ } \ } while (0) +/* + * Lock 'OBJ' and execute a suite of C statements 'STATEMENTS'. + * + * The GIL is held if 'SIZE' is below the HASHLIB_GIL_MINSIZE threshold. + */ #define HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED(OBJ, SIZE, STATEMENTS) \ do { \ if ((SIZE) > HASHLIB_GIL_MINSIZE) { \ diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index 0e5f7c852dbbfe..fa00a481918b5d 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -283,7 +283,7 @@ get_hmacmodule_state_by_cls(PyTypeObject *cls) typedef Hacl_Streaming_HMAC_agile_state HACL_HMAC_state; typedef struct HMACObject { - PyObject_HASHLIB_HEAD + HASHLIB_OBJECT_HEAD // Hash function information PyObject *name; // rendered name (exact unicode object) HMAC_Hash_Kind kind; // can be used for runtime dispatch (must be known) diff --git a/Modules/md5module.c b/Modules/md5module.c index e3398541189411..d5f66751258d08 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -38,7 +38,7 @@ class MD5Type "MD5object *" "&PyType_Type" #include "_hacl/Hacl_Hash_MD5.h" typedef struct { - PyObject_HASHLIB_HEAD + HASHLIB_OBJECT_HEAD Hacl_Hash_MD5_state_t *hash_state; } MD5object; diff --git a/Modules/sha1module.c b/Modules/sha1module.c index cb7bcd5a078326..251fb089a62860 100644 --- a/Modules/sha1module.c +++ b/Modules/sha1module.c @@ -38,7 +38,7 @@ class SHA1Type "SHA1object *" "&PyType_Type" #include "_hacl/Hacl_Hash_SHA1.h" typedef struct { - PyObject_HASHLIB_HEAD + HASHLIB_OBJECT_HEAD Hacl_Hash_SHA1_state_t *hash_state; } SHA1object; diff --git a/Modules/sha2module.c b/Modules/sha2module.c index f4c4ba3254849b..51dbdc7b7b6823 100644 --- a/Modules/sha2module.c +++ b/Modules/sha2module.c @@ -50,13 +50,13 @@ class SHA512Type "SHA512object *" "&PyType_Type" // TODO: Get rid of int digestsize in favor of Hacl state info? typedef struct { - PyObject_HASHLIB_HEAD + HASHLIB_OBJECT_HEAD int digestsize; Hacl_Hash_SHA2_state_t_256 *state; } SHA256object; typedef struct { - PyObject_HASHLIB_HEAD + HASHLIB_OBJECT_HEAD int digestsize; Hacl_Hash_SHA2_state_t_512 *state; } SHA512object; diff --git a/Modules/sha3module.c b/Modules/sha3module.c index f9983fe509b36c..ee7d33116c10e4 100644 --- a/Modules/sha3module.c +++ b/Modules/sha3module.c @@ -59,7 +59,7 @@ class _sha3.shake_256 "SHA3object *" "&SHAKE256type" #include "_hacl/Hacl_Hash_SHA3.h" typedef struct { - PyObject_HASHLIB_HEAD + HASHLIB_OBJECT_HEAD Hacl_Hash_SHA3_state_t *hash_state; } SHA3object; From 7fd139625b7b63dd664bf9694969577e9175a44a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Fri, 20 Jun 2025 09:57:26 +0200 Subject: [PATCH 32/39] reudce diff --- Modules/md5module.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/Modules/md5module.c b/Modules/md5module.c index d5f66751258d08..b64cbdaf86673e 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -22,7 +22,6 @@ #include "Python.h" #include "hashlib.h" -#include "pycore_strhex.h" // _Py_strhex() /*[clinic input] module _md5 @@ -133,7 +132,7 @@ static PyObject * MD5Type_digest_impl(MD5object *self) /*[clinic end generated code: output=eb691dc4190a07ec input=bc0c4397c2994be6]*/ { - uint8_t digest[MD5_DIGESTSIZE]; + unsigned char digest[MD5_DIGESTSIZE]; HASHLIB_ACQUIRE_LOCK(self); Hacl_Hash_MD5_digest(self->hash_state, digest); HASHLIB_RELEASE_LOCK(self); @@ -150,11 +149,20 @@ static PyObject * MD5Type_hexdigest_impl(MD5object *self) /*[clinic end generated code: output=17badced1f3ac932 input=b60b19de644798dd]*/ { - uint8_t digest[MD5_DIGESTSIZE]; + unsigned char digest[MD5_DIGESTSIZE]; HASHLIB_ACQUIRE_LOCK(self); Hacl_Hash_MD5_digest(self->hash_state, digest); HASHLIB_RELEASE_LOCK(self); - return _Py_strhex((const char *)digest, MD5_DIGESTSIZE); + + const char *hexdigits = "0123456789abcdef"; + char digest_hex[MD5_DIGESTSIZE * 2]; + char *str = digest_hex; + for (size_t i=0; i < MD5_DIGESTSIZE; i++) { + unsigned char byte = digest[i]; + *str++ = hexdigits[byte >> 4]; + *str++ = hexdigits[byte & 0x0f]; + } + return PyUnicode_FromStringAndSize(digest_hex, sizeof(digest_hex)); } static void From 977c8078ac236c8f60e87f91e77b4ab4bf7d3c97 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, 21 Jun 2025 14:38:20 +0200 Subject: [PATCH 33/39] fixup --- Modules/_hashopenssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 71ca86f43ad91f..0fe69ee8fac835 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -798,7 +798,7 @@ _hashlib_HASH_update_impl(HASHobject *self, PyObject *obj) Py_buffer view; GET_BUFFER_VIEW_OR_ERROUT(obj, &view); HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( - self, HASHLIB_GIL_MINSIZE, + self, view.len, result = _hashlib_HASH_hash(self, view.buf, view.len) ); PyBuffer_Release(&view); From 6d66fefbc2fe517925d22ee1ae4a83cd2a589db0 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, 21 Jun 2025 14:50:36 +0200 Subject: [PATCH 34/39] fixup --- Modules/sha3module.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Modules/sha3module.c b/Modules/sha3module.c index a57d8b281cb1dc..7f0ff31008a672 100644 --- a/Modules/sha3module.c +++ b/Modules/sha3module.c @@ -514,9 +514,9 @@ _sha3_shake_128_digest_impl(SHA3object *self, Py_ssize_t length) CHECK_HACL_UINT32_T_LENGTH(length); PyObject *digest = PyBytes_FromStringAndSize(NULL, length); uint8_t *buffer = (uint8_t *)PyBytes_AS_STRING(digest); - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); (void)Hacl_Hash_SHA3_squeeze(self->hash_state, buffer, (uint32_t)length); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); return digest; } @@ -548,9 +548,9 @@ _sha3_shake_128_hexdigest_impl(SHA3object *self, Py_ssize_t length) return PyErr_NoMemory(); } - ENTER_HASHLIB(self); + HASHLIB_ACQUIRE_LOCK(self); (void)Hacl_Hash_SHA3_squeeze(self->hash_state, buffer, (uint32_t)length); - LEAVE_HASHLIB(self); + HASHLIB_RELEASE_LOCK(self); PyObject *digest = _Py_strhex((const char *)buffer, length); PyMem_Free(buffer); return digest; From c9db0b197457bb5cdd391115b141d13547cdceca 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, 21 Jun 2025 15:34:53 +0200 Subject: [PATCH 35/39] make the test suite less slow --- Lib/test/test_hashlib.py | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_hashlib.py b/Lib/test/test_hashlib.py index 5cda1efb66608f..f17c44122c11de 100644 --- a/Lib/test/test_hashlib.py +++ b/Lib/test/test_hashlib.py @@ -1060,11 +1060,24 @@ def test_sha256_gil(self): @threading_helper.reap_threads @threading_helper.requires_working_threading() - def test_threaded_hashing(self): + def test_threaded_hashing_fast(self): + # Same as test_threaded_hashing_slow() but only tests "fast" functions + # since otherwise test_hashlib.py becomes too slow during development. + for name in ['md5', 'sha1', 'sha256', 'sha3_256', 'blake2s']: + if constructor := getattr(hashlib, name, None): + with self.subTest(name): + self.do_test_threaded_hashing(constructor, is_shake=False) + if shake_128 := getattr(hashlib, 'shake_128', None): + self.do_test_threaded_hashing(shake_128, is_shake=True) + + @requires_resource('cpu') + @threading_helper.reap_threads + @threading_helper.requires_working_threading() + def test_threaded_hashing_slow(self): for algorithm, constructors in self.constructors_to_test.items(): is_shake = algorithm in self.shakes for constructor in constructors: - with self.subTest(constructor=constructor, is_shake=is_shake): + with self.subTest(constructor.__name__, is_shake=is_shake): self.do_test_threaded_hashing(constructor, is_shake) def do_test_threaded_hashing(self, constructor, is_shake): @@ -1074,8 +1087,12 @@ def do_test_threaded_hashing(self, constructor, is_shake): # If the internal locks are working to prevent multiple # updates on the same object from running at once, the resulting # hash will be the same as doing it single threaded upfront. + # + # Be careful when choosing num_threads, len(smallest_data) + # and len(data) // len(smallest_data) as the obtained chunk + # size needs to satisfy some conditions below. num_threads = 5 - smallest_data = os.urandom(16) + smallest_data = os.urandom(8) data = smallest_data * 200000 h1 = constructor() From 6ffdd1cd2ef8dec6a679e56a3bbfc207b01eb532 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, 21 Jun 2025 15:37:22 +0200 Subject: [PATCH 36/39] fix test when GIL_MINSIZE is changed --- Lib/test/test_hashlib.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_hashlib.py b/Lib/test/test_hashlib.py index f17c44122c11de..2689bb0de882c1 100644 --- a/Lib/test/test_hashlib.py +++ b/Lib/test/test_hashlib.py @@ -1043,20 +1043,14 @@ def test_gil(self): def test_sha256_gil(self): gil_minsize = hashlib_helper.find_gil_minsize(['_sha2', '_hashlib']) + data = b'1' + b'#' * gil_minsize + b'1' + expected = hashlib.sha256(data).hexdigest() + m = hashlib.sha256() m.update(b'1') m.update(b'#' * gil_minsize) m.update(b'1') - self.assertEqual( - m.hexdigest(), - '1cfceca95989f51f658e3f3ffe7f1cd43726c9e088c13ee10b46f57cef135b94' - ) - - m = hashlib.sha256(b'1' + b'#' * gil_minsize + b'1') - self.assertEqual( - m.hexdigest(), - '1cfceca95989f51f658e3f3ffe7f1cd43726c9e088c13ee10b46f57cef135b94' - ) + self.assertEqual(m.hexdigest(), expected) @threading_helper.reap_threads @threading_helper.requires_working_threading() From 98ec9152b776a60df1f45a4fd3dc727430d93dae 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, 21 Jun 2025 15:43:30 +0200 Subject: [PATCH 37/39] defer cosmetics --- Modules/hashlib.h | 6 +++--- Modules/hmacmodule.c | 10 +++++----- Modules/md5module.c | 17 ++++++++--------- Modules/sha1module.c | 7 +++---- Modules/sha2module.c | 18 ++++++++---------- Modules/sha3module.c | 7 +++---- 6 files changed, 30 insertions(+), 35 deletions(-) diff --git a/Modules/hashlib.h b/Modules/hashlib.h index 5800772a374440..9a7e72f34a7f9d 100644 --- a/Modules/hashlib.h +++ b/Modules/hashlib.h @@ -3,9 +3,9 @@ #include "pycore_lock.h" // PyMutex /* - * Given a PyObject* 'obj', fill in the Py_buffer* 'viewp' with the result - * of PyObject_GetBuffer. Sets an exception and issues the 'erraction' - * on any errors, e.g., 'return NULL' or 'goto error'. + * Given a PyObject* obj, fill in the Py_buffer* viewp with the result + * of PyObject_GetBuffer. Sets an exception and issues the erraction + * on any errors, e.g. 'return NULL' or 'goto error'. */ #define GET_BUFFER_VIEW_OR_ERROR(obj, viewp, erraction) do { \ if (PyUnicode_Check((obj))) { \ diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index fa00a481918b5d..e7a5ccbb19b45c 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -500,11 +500,11 @@ _hacl_hmac_state_update(HACL_HMAC_state *state, uint8_t *buf, Py_ssize_t len) /* Static information used to construct the hash table. */ static const py_hmac_hinfo py_hmac_static_hinfo[] = { -#define Py_HMAC_HINFO_HACL_API(HACL_HID) \ - { \ - /* one-shot helpers */ \ - .compute = &Py_hmac_## HACL_HID ##_compute_func, \ - .compute_py = &_hmac_compute_## HACL_HID ##_impl, \ +#define Py_HMAC_HINFO_HACL_API(HACL_HID) \ + { \ + /* one-shot helpers */ \ + .compute = &Py_hmac_## HACL_HID ##_compute_func, \ + .compute_py = &_hmac_compute_## HACL_HID ##_impl, \ } #define Py_HMAC_HINFO_ENTRY(HACL_HID, HLIB_NAME) \ diff --git a/Modules/md5module.c b/Modules/md5module.c index de305f7febf766..5dac3a91f4f09d 100644 --- a/Modules/md5module.c +++ b/Modules/md5module.c @@ -159,15 +159,14 @@ MD5Type_hexdigest_impl(MD5object *self) } static void -_hacl_md5_state_update(Hacl_Hash_MD5_state_t *state, - uint8_t *buf, Py_ssize_t len) +update(Hacl_Hash_MD5_state_t *state, uint8_t *buf, Py_ssize_t len) { - assert(len >= 0); /* - * Note: we explicitly ignore the error code on the basis that it would - * take more than 1 billion years to overflow the maximum admissible length - * for MD5 (2^61 - 1). - */ + * Note: we explicitly ignore the error code on the basis that it would + * take more than 1 billion years to overflow the maximum admissible length + * for MD5 (2^61 - 1). + */ + assert(len >= 0); #if PY_SSIZE_T_MAX > UINT32_MAX while (len > UINT32_MAX) { (void)Hacl_Hash_MD5_update(state, buf, UINT32_MAX); @@ -196,7 +195,7 @@ MD5Type_update_impl(MD5object *self, PyObject *obj) GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( self, buf.len, - _hacl_md5_state_update(self->hash_state, buf.buf, buf.len) + update(self->hash_state, buf.buf, buf.len) ); PyBuffer_Release(&buf); Py_RETURN_NONE; @@ -303,7 +302,7 @@ _md5_md5_impl(PyObject *module, PyObject *data, int usedforsecurity, * where it is not yet possible to have concurrent access. */ HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( buf.len, - _hacl_md5_state_update(new->hash_state, buf.buf, buf.len) + update(new->hash_state, buf.buf, buf.len) ); PyBuffer_Release(&buf); } diff --git a/Modules/sha1module.c b/Modules/sha1module.c index 251fb089a62860..3bc83b674f1bc1 100644 --- a/Modules/sha1module.c +++ b/Modules/sha1module.c @@ -162,8 +162,7 @@ SHA1Type_hexdigest_impl(SHA1object *self) } static void -_hacl_sha1_state_update(Hacl_Hash_SHA1_state_t *state, - uint8_t *buf, Py_ssize_t len) +update(Hacl_Hash_SHA1_state_t *state, uint8_t *buf, Py_ssize_t len) { /* * Note: we explicitly ignore the error code on the basis that it would @@ -198,7 +197,7 @@ SHA1Type_update_impl(SHA1object *self, PyObject *obj) GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( self, buf.len, - _hacl_sha1_state_update(self->hash_state, buf.buf, buf.len) + update(self->hash_state, buf.buf, buf.len) ); PyBuffer_Release(&buf); Py_RETURN_NONE; @@ -304,7 +303,7 @@ _sha1_sha1_impl(PyObject *module, PyObject *data, int usedforsecurity, * where it is not yet possible to have concurrent access. */ HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( buf.len, - _hacl_sha1_state_update(new->hash_state, buf.buf, buf.len) + update(new->hash_state, buf.buf, buf.len) ); PyBuffer_Release(&buf); } diff --git a/Modules/sha2module.c b/Modules/sha2module.c index 51dbdc7b7b6823..6643b7e6b02654 100644 --- a/Modules/sha2module.c +++ b/Modules/sha2module.c @@ -200,8 +200,7 @@ SHA512_dealloc(PyObject *op) * 64 bits so we loop in <4gig chunks when needed. */ static void -_hacl_sha2_state_update_256(Hacl_Hash_SHA2_state_t_256 *state, - uint8_t *buf, Py_ssize_t len) +update_256(Hacl_Hash_SHA2_state_t_256 *state, uint8_t *buf, Py_ssize_t len) { /* * Note: we explicitly ignore the error code on the basis that it would @@ -220,8 +219,7 @@ _hacl_sha2_state_update_256(Hacl_Hash_SHA2_state_t_256 *state, } static void -_hacl_sha2_state_update_512(Hacl_Hash_SHA2_state_t_512 *state, - uint8_t *buf, Py_ssize_t len) +update_512(Hacl_Hash_SHA2_state_t_512 *state, uint8_t *buf, Py_ssize_t len) { /* * Note: we explicitly ignore the error code on the basis that it would @@ -408,7 +406,7 @@ SHA256Type_update_impl(SHA256object *self, PyObject *obj) GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( self, buf.len, - _hacl_sha2_state_update_256(self->state, buf.buf, buf.len) + update_256(self->state, buf.buf, buf.len) ); PyBuffer_Release(&buf); Py_RETURN_NONE; @@ -431,7 +429,7 @@ SHA512Type_update_impl(SHA512object *self, PyObject *obj) GET_BUFFER_VIEW_OR_ERROUT(obj, &buf); HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( self, buf.len, - _hacl_sha2_state_update_512(self->state, buf.buf, buf.len) + update_512(self->state, buf.buf, buf.len) ); PyBuffer_Release(&buf); Py_RETURN_NONE; @@ -616,7 +614,7 @@ _sha2_sha256_impl(PyObject *module, PyObject *data, int usedforsecurity, * where it is not yet possible to have concurrent access. */ HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( buf.len, - _hacl_sha2_state_update_256(new->state, buf.buf, buf.len) + update_256(new->state, buf.buf, buf.len) ); PyBuffer_Release(&buf); } @@ -674,7 +672,7 @@ _sha2_sha224_impl(PyObject *module, PyObject *data, int usedforsecurity, * where it is not yet possible to have concurrent access. */ HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( buf.len, - _hacl_sha2_state_update_256(new->state, buf.buf, buf.len) + update_256(new->state, buf.buf, buf.len) ); PyBuffer_Release(&buf); } @@ -733,7 +731,7 @@ _sha2_sha512_impl(PyObject *module, PyObject *data, int usedforsecurity, * where it is not yet possible to have concurrent access. */ HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( buf.len, - _hacl_sha2_state_update_512(new->state, buf.buf, buf.len) + update_512(new->state, buf.buf, buf.len) ); PyBuffer_Release(&buf); } @@ -792,7 +790,7 @@ _sha2_sha384_impl(PyObject *module, PyObject *data, int usedforsecurity, * where it is not yet possible to have concurrent access. */ HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( buf.len, - _hacl_sha2_state_update_512(new->state, buf.buf, buf.len) + update_512(new->state, buf.buf, buf.len) ); PyBuffer_Release(&buf); } diff --git a/Modules/sha3module.c b/Modules/sha3module.c index 7f0ff31008a672..68b239352769a1 100644 --- a/Modules/sha3module.c +++ b/Modules/sha3module.c @@ -92,8 +92,7 @@ newSHA3object(PyTypeObject *type) } static void -_hacl_sha3_state_update(Hacl_Hash_SHA3_state_t *state, - uint8_t *buf, Py_ssize_t len) +sha3_update(Hacl_Hash_SHA3_state_t *state, uint8_t *buf, Py_ssize_t len) { /* * Note: we explicitly ignore the error code on the basis that it would @@ -176,7 +175,7 @@ py_sha3_new_impl(PyTypeObject *type, PyObject *data_obj, int usedforsecurity, * where it is not yet possible to have concurrent access. */ HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED( buf.len, - _hacl_sha3_state_update(self->hash_state, buf.buf, buf.len) + sha3_update(self->hash_state, buf.buf, buf.len) ); } @@ -311,7 +310,7 @@ _sha3_sha3_224_update_impl(SHA3object *self, PyObject *data) GET_BUFFER_VIEW_OR_ERROUT(data, &buf); HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED( self, buf.len, - _hacl_sha3_state_update(self->hash_state, buf.buf, buf.len) + sha3_update(self->hash_state, buf.buf, buf.len) ); PyBuffer_Release(&buf); Py_RETURN_NONE; From 398ddb396ec34407b5b172c4f790de405ee30666 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, 21 Jun 2025 21:23:40 +0200 Subject: [PATCH 38/39] Update Lib/test/test_hashlib.py --- Lib/test/test_hashlib.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_hashlib.py b/Lib/test/test_hashlib.py index 2689bb0de882c1..aa8abdbe6bccb1 100644 --- a/Lib/test/test_hashlib.py +++ b/Lib/test/test_hashlib.py @@ -1089,8 +1089,8 @@ def do_test_threaded_hashing(self, constructor, is_shake): smallest_data = os.urandom(8) data = smallest_data * 200000 - h1 = constructor() - h2 = constructor(data * num_threads) + h1 = constructor(usedforsecurity=False) + h2 = constructor(data * num_threads, usedforsecurity=False) def hash_in_chunks(chunk_size): index = 0 From 0ae70e9c3f3cac460a414ea8f9e77bbdd4c80748 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 22 Jun 2025 15:29:23 +0200 Subject: [PATCH 39/39] improve test --- Lib/test/test_hashlib.py | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_hashlib.py b/Lib/test/test_hashlib.py index aa8abdbe6bccb1..6e4e41e4fbb4f1 100644 --- a/Lib/test/test_hashlib.py +++ b/Lib/test/test_hashlib.py @@ -1055,7 +1055,7 @@ def test_sha256_gil(self): @threading_helper.reap_threads @threading_helper.requires_working_threading() def test_threaded_hashing_fast(self): - # Same as test_threaded_hashing_slow() but only tests "fast" functions + # Same as test_threaded_hashing_slow() but only tests some functions # since otherwise test_hashlib.py becomes too slow during development. for name in ['md5', 'sha1', 'sha256', 'sha3_256', 'blake2s']: if constructor := getattr(hashlib, name, None): @@ -1081,30 +1081,29 @@ def do_test_threaded_hashing(self, constructor, is_shake): # If the internal locks are working to prevent multiple # updates on the same object from running at once, the resulting # hash will be the same as doing it single threaded upfront. - # - # Be careful when choosing num_threads, len(smallest_data) - # and len(data) // len(smallest_data) as the obtained chunk - # size needs to satisfy some conditions below. - num_threads = 5 - smallest_data = os.urandom(8) - data = smallest_data * 200000 + + # The data to hash has length s|M|q^N and the chunk size for the i-th + # thread is s|M|q^(N-i), where N is the number of threads, M is a fixed + # message of small length, and s >= 1 and q >= 2 are small integers. + smallest_size, num_threads, s, q = 8, 5, 2, 10 + + smallest_data = os.urandom(smallest_size) + data = s * smallest_data * (q ** num_threads) h1 = constructor(usedforsecurity=False) h2 = constructor(data * num_threads, usedforsecurity=False) - def hash_in_chunks(chunk_size): - index = 0 - while index < len(data): + def update(chunk_size): + for index in range(0, len(data), chunk_size): h1.update(data[index:index + chunk_size]) - index += chunk_size threads = [] - for threadnum in range(num_threads): - chunk_size = len(data) // (10 ** threadnum) + for thread_num in range(num_threads): + # chunk_size = len(data) // (q ** thread_num) + chunk_size = s * smallest_size * q ** (num_threads - thread_num) self.assertGreater(chunk_size, 0) - self.assertEqual(chunk_size % len(smallest_data), 0) - thread = threading.Thread(target=hash_in_chunks, - args=(chunk_size,)) + self.assertEqual(chunk_size % smallest_size, 0) + thread = threading.Thread(target=update, args=(chunk_size,)) threads.append(thread) for thread in threads: