Skip to content

Commit 0719c9e

Browse files
kaabiajhedberg
authored andcommitted
crypto: mbedtls_shim: Fix concurrency and deadlock issues
- **Fix session mutex handling:** Ensure the `mtls_sessions_lock` is always released in `mtls_get_unused_session_index` on failure to prevent deadlocks. - **Protect `in_use` flag:** Added mutex protection when setting `in_use = false` in free functions. - **Cleanup on setup failure:** Added calls to `mbedtls_*_free()` in `mtls_session_setup()` when key initialization fails. - **Free logic fix:** Corrected `if/else` structure in `mtls_session_free()` to ensure the correct context is freed. Signed-off-by: Badr Bacem KAABIA <[email protected]>
1 parent 577f60d commit 0719c9e

File tree

1 file changed

+32
-18
lines changed

1 file changed

+32
-18
lines changed

drivers/crypto/crypto_mtls_shim.c

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ struct mtls_shim_session {
5757

5858
struct mtls_shim_session mtls_sessions[CRYPTO_MAX_SESSION];
5959

60+
static K_MUTEX_DEFINE(mtls_sessions_lock);
61+
6062
#if defined(MBEDTLS_MEMORY_BUFFER_ALLOC_C)
6163
#include "mbedtls/memory_buffer_alloc.h"
6264
#else
@@ -295,13 +297,17 @@ static int mtls_get_unused_session_index(void)
295297
{
296298
int i;
297299

300+
k_mutex_lock(&mtls_sessions_lock, K_FOREVER);
301+
298302
for (i = 0; i < CRYPTO_MAX_SESSION; i++) {
299303
if (!mtls_sessions[i].in_use) {
300304
mtls_sessions[i].in_use = true;
305+
k_mutex_unlock(&mtls_sessions_lock);
301306
return i;
302307
}
303308
}
304309

310+
k_mutex_unlock(&mtls_sessions_lock);
305311
return -1;
306312
}
307313

@@ -316,7 +322,7 @@ static int mtls_session_setup(const struct device *dev,
316322
mbedtls_gcm_context *gcm_ctx;
317323
#endif
318324
int ctx_idx;
319-
int ret;
325+
int ret = 0;
320326

321327
if (ctx->flags & ~(MTLS_SUPPORT)) {
322328
LOG_ERR("Unsupported flag");
@@ -349,6 +355,8 @@ static int mtls_session_setup(const struct device *dev,
349355
return -ENOSPC;
350356
}
351357

358+
mtls_sessions[ctx_idx].mode = mode;
359+
ctx->drv_sessn_state = &mtls_sessions[ctx_idx];
352360

353361
switch (mode) {
354362
case CRYPTO_CIPHER_MODE_ECB:
@@ -365,9 +373,8 @@ static int mtls_session_setup(const struct device *dev,
365373
}
366374
if (ret) {
367375
LOG_ERR("AES_ECB: failed at setkey (%d)", ret);
368-
ctx->ops.block_crypt_hndlr = NULL;
369-
mtls_sessions[ctx_idx].in_use = false;
370-
return -EINVAL;
376+
mbedtls_aes_free(aes_ctx);
377+
ret = -EINVAL;
371378
}
372379
break;
373380
case CRYPTO_CIPHER_MODE_CBC:
@@ -384,9 +391,8 @@ static int mtls_session_setup(const struct device *dev,
384391
}
385392
if (ret) {
386393
LOG_ERR("AES_CBC: failed at setkey (%d)", ret);
387-
ctx->ops.cbc_crypt_hndlr = NULL;
388-
mtls_sessions[ctx_idx].in_use = false;
389-
return -EINVAL;
394+
mbedtls_aes_init(aes_ctx);
395+
ret = -EINVAL;
390396
}
391397
break;
392398
case CRYPTO_CIPHER_MODE_CCM:
@@ -396,9 +402,9 @@ static int mtls_session_setup(const struct device *dev,
396402
ctx->key.bit_stream, ctx->keylen * 8U);
397403
if (ret) {
398404
LOG_ERR("AES_CCM: failed at setkey (%d)", ret);
399-
mtls_sessions[ctx_idx].in_use = false;
400-
401-
return -EINVAL;
405+
mbedtls_ccm_free(ccm_ctx);
406+
ret = -EINVAL;
407+
break;
402408
}
403409
if (op_type == CRYPTO_CIPHER_OP_ENCRYPT) {
404410
ctx->ops.ccm_crypt_hndlr = mtls_ccm_encrypt_auth;
@@ -414,9 +420,9 @@ static int mtls_session_setup(const struct device *dev,
414420
ctx->key.bit_stream, ctx->keylen * 8U);
415421
if (ret) {
416422
LOG_ERR("AES_GCM: failed at setkey (%d)", ret);
417-
mtls_sessions[ctx_idx].in_use = false;
418-
419-
return -EINVAL;
423+
mbedtls_gcm_free(gcm_ctx);
424+
ret = -EINVAL;
425+
break;
420426
}
421427
if (op_type == CRYPTO_CIPHER_OP_ENCRYPT) {
422428
ctx->ops.gcm_crypt_hndlr = mtls_gcm_encrypt_auth;
@@ -427,13 +433,17 @@ static int mtls_session_setup(const struct device *dev,
427433
#endif /* CONFIG_MBEDTLS_CIPHER_GCM_ENABLED */
428434
default:
429435
LOG_ERR("Unhandled mode");
430-
mtls_sessions[ctx_idx].in_use = false;
431-
return -EINVAL;
436+
ret = -EINVAL;
432437
}
433438

434-
mtls_sessions[ctx_idx].mode = mode;
435-
ctx->drv_sessn_state = &mtls_sessions[ctx_idx];
436-
439+
/* Centralized cleanup of the session slot if an error occurred
440+
* during configuration (ret != 0).
441+
*/
442+
if (ret != 0) {
443+
k_mutex_lock(&mtls_sessions_lock, K_FOREVER);
444+
mtls_sessions[ctx_idx].in_use = false;
445+
k_mutex_unlock(&mtls_sessions_lock);
446+
}
437447
return ret;
438448
}
439449

@@ -451,7 +461,9 @@ static int mtls_session_free(const struct device *dev, struct cipher_ctx *ctx)
451461
} else {
452462
mbedtls_aes_free(&mtls_session->mtls_aes);
453463
}
464+
k_mutex_lock(&mtls_sessions_lock, K_FOREVER);
454465
mtls_session->in_use = false;
466+
k_mutex_unlock(&mtls_sessions_lock);
455467

456468
return 0;
457469
}
@@ -582,7 +594,9 @@ static int mtls_hash_session_free(const struct device *dev, struct hash_ctx *ctx
582594
} else {
583595
mbedtls_sha512_free(&mtls_session->mtls_sha512);
584596
}
597+
k_mutex_lock(&mtls_sessions_lock, K_FOREVER);
585598
mtls_session->in_use = false;
599+
k_mutex_unlock(&mtls_sessions_lock);
586600

587601
return 0;
588602
}

0 commit comments

Comments
 (0)