Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor coverity fixes #315

Merged
merged 2 commits into from
Nov 28, 2023
Merged

Minor coverity fixes #315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d5aeab4
Use context lock to protect only changed items
simo5 Nov 28, 2023
2e2c187
Remove lockless peak from sessions managment
simo5 Nov 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 19 additions & 19 deletions src/provider.c
View file
Open in desktop
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
22 changes: 8 additions & 14 deletions src/session.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -957,22 +957,16 @@ void p11prov_return_session(P11PROV_SESSION *session)
pool = session->pool;

if (pool) {
/* peek at the pool lockless worst case we waste some time */
if (pool->open_sessions >= pool->max_cached_sessions) {
/* not much we can do if this fails,
* but only accounting will be a bit off */

/* LOCKED SECTION ------------- */
if (MUTEX_LOCK(pool) == CKR_OK) {
if (pool->open_sessions >= pool->max_cached_sessions
&& session != pool->login_session) {
token_session_close(session);
pool->open_sessions--;
}
(void)MUTEX_UNLOCK(pool);
/* LOCKED SECTION ------------- */
if (MUTEX_LOCK(pool) == CKR_OK) {
if (pool->open_sessions >= pool->max_cached_sessions
&& session != pool->login_session) {
token_session_close(session);
pool->open_sessions--;
}
/* ------------- LOCKED SECTION */
(void)MUTEX_UNLOCK(pool);
}
/* ------------- LOCKED SECTION */
}

ret = MUTEX_LOCK(session);
Expand Down
Loading