Skip to content

Commit

Permalink
Use context lock to protect only changed items
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
simo5 committed Nov 28, 2023
1 parent de52d24 commit 98e1150
Showing 1 changed file with 19 additions and 19 deletions.
38 changes: 19 additions & 19 deletions src/provider.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
};
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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));
}

Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 98e1150

Please sign in to comment.