Use context lock to protect only changed items

OpenSSL is in charge of making sure the context is not used when it is freed. Repurpose the lock to only cover parts of the context that can be changed by independent threads after the context creation. Fixes: CID 468683 CID 468679 Signed-off-by: Simo Sorce <[email protected]>
simo5 · Nov 28, 2023 · d5aeab4 · d5aeab4
1 parent 6843795
commit d5aeab4
Showing 1 changed file with 19 additions and 19 deletions.
diff --git a/src/provider.c b/src/provider.c
@@ -18,8 +18,6 @@ struct p11prov_ctx {
         P11PROV_IN_ERROR,
     } status;
 
-    pthread_rwlock_t rwlock;
-
     /* Provider handles */
     const OSSL_CORE_HANDLE *handle;
     OSSL_LIB_CTX *libctx;
@@ -51,6 +49,7 @@ struct p11prov_ctx {
     OSSL_ALGORITHM *op_asym_cipher;
     OSSL_ALGORITHM *op_encoder;
 
+    pthread_rwlock_t quirk_lock;
     struct quirk *quirks;
     int nquirks;
 };
@@ -240,7 +239,7 @@ CK_RV p11prov_ctx_get_quirk(P11PROV_CTX *ctx, CK_SLOT_ID id, const char *name,
     int lock;
     CK_RV ret;
 
-    lock = pthread_rwlock_rdlock(&ctx->rwlock);
+    lock = pthread_rwlock_rdlock(&ctx->quirk_lock);
     if (lock != 0) {
         ret = CKR_CANT_LOCK;
         P11PROV_raise(ctx, ret, "Failure to rdlock! (%d)", errno);
@@ -281,7 +280,7 @@ CK_RV p11prov_ctx_get_quirk(P11PROV_CTX *ctx, CK_SLOT_ID id, const char *name,
     ret = CKR_OK;
 
 done:
-    lock = pthread_rwlock_unlock(&ctx->rwlock);
+    lock = pthread_rwlock_unlock(&ctx->quirk_lock);
     if (lock != 0) {
         P11PROV_raise(ctx, CKR_CANT_LOCK, "Failure to unlock! (%d)", errno);
         /* we do not return an error in this case, as we got the info */
@@ -329,7 +328,7 @@ CK_RV p11prov_ctx_set_quirk(P11PROV_CTX *ctx, CK_SLOT_ID id, const char *name,
         memcpy(_data, data, _size);
     }
 
-    lock = pthread_rwlock_wrlock(&ctx->rwlock);
+    lock = pthread_rwlock_wrlock(&ctx->quirk_lock);
     if (lock != 0) {
         ret = CKR_CANT_LOCK;
         P11PROV_raise(ctx, ret, "Failure to wrlock! (%d)", errno);
@@ -380,7 +379,7 @@ CK_RV p11prov_ctx_set_quirk(P11PROV_CTX *ctx, CK_SLOT_ID id, const char *name,
 
 done:
     P11PROV_debug("Set quirk '%s' of size %lu", name, size);
-    lock = pthread_rwlock_unlock(&ctx->rwlock);
+    lock = pthread_rwlock_unlock(&ctx->quirk_lock);
     if (lock != 0) {
         P11PROV_raise(ctx, CKR_CANT_LOCK, "Failure to unlock! (%d)", errno);
         /* we do not return an error in this case, as we got the info */
@@ -515,13 +514,6 @@ static void p11prov_ctx_free(P11PROV_CTX *ctx)
 {
     int ret;
 
-    ret = pthread_rwlock_wrlock(&ctx->rwlock);
-    if (ret != 0) {
-        P11PROV_raise(ctx, CKR_CANT_LOCK,
-                      "Failure to wrlock! Data corruption may happen (%d)",
-                      errno);
-    }
-
     if (ctx->no_deinit) {
         ctx->status = P11PROV_NO_DEINIT;
     }
@@ -549,6 +541,13 @@ static void p11prov_ctx_free(P11PROV_CTX *ctx)
     p11prov_module_free(ctx->module);
     ctx->module = NULL;
 
+    ret = pthread_rwlock_wrlock(&ctx->quirk_lock);
+    if (ret != 0) {
+        P11PROV_raise(ctx, CKR_CANT_LOCK,
+                      "Failure to wrlock! Data corruption may happen (%d)",
+                      errno);
+    }
+
     if (ctx->quirks) {
         for (int i = 0; i < ctx->nquirks; i++) {
             OPENSSL_free(ctx->quirks[i].name);
@@ -560,22 +559,23 @@ static void p11prov_ctx_free(P11PROV_CTX *ctx)
         OPENSSL_free(ctx->quirks);
     }
 
-    /* remove from pool */
-    context_rm_pool(ctx);
-
-    ret = pthread_rwlock_unlock(&ctx->rwlock);
+    ret = pthread_rwlock_unlock(&ctx->quirk_lock);
     if (ret != 0) {
         P11PROV_raise(ctx, CKR_CANT_LOCK,
                       "Failure to unlock! Data corruption may happen (%d)",
                       errno);
     }
 
-    ret = pthread_rwlock_destroy(&ctx->rwlock);
+    ret = pthread_rwlock_destroy(&ctx->quirk_lock);
     if (ret != 0) {
         P11PROV_raise(ctx, CKR_CANT_LOCK,
                       "Failure to free lock! Data corruption may happen (%d)",
                       errno);
     }
+
+    /* remove from pool */
+    context_rm_pool(ctx);
+
     OPENSSL_clear_free(ctx, sizeof(P11PROV_CTX));
 }
 
@@ -1311,7 +1311,7 @@ int OSSL_provider_init(const OSSL_CORE_HANDLE *handle, const OSSL_DISPATCH *in,
     }
     ctx->handle = handle;
 
-    ret = pthread_rwlock_init(&ctx->rwlock, NULL);
+    ret = pthread_rwlock_init(&ctx->quirk_lock, NULL);
     if (ret != 0) {
         ret = errno;
         P11PROV_debug("rwlock init failed (%d)", ret);