correctly lock objects

Author: picnixz

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

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) {

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;
 }

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)

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;
 }

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);
     }
 

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);
     }
 

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);
     }
 

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;
 }