From 98e1150e6a389c61360e2b0322e7dead01895492 Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Tue, 28 Nov 2023 13:55:49 -0500
Subject: [PATCH] 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 <simo@redhat.com>
---
 src/provider.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/src/provider.c b/src/provider.c
index 6f950da8..f4c33107 100644
--- 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);