Skip to content

Commit

Permalink
nrf_security: Refactor mutexes
Browse files Browse the repository at this point in the history
This refactors the mutexes in nrf_security.

The main issue with the current design is that
that it doesn't use the Zephyr macro K_MUTEX_DEFINE
to define the mutexes. If this macro is not used
the Zephyr mutexes need to be initialized at runtime,
but the Cracen code doesn't initalize them and so
they don't work properly.

This simplifies the nrf_security mutexes by removing
the flag. This keep the design simpler and less
error prone. An extra flag added to the Zephyr mutexes
doesn't seem to be necessary. The nrf_security mutexes
were added in order to be used by the Cracen driver and
reduce the ifdef conditions inside the driver code
for tfm/non tfm builds. No initialization is required
for these mutexes because there are always meant to be
statically defined and this removes the need for
initialization as long as we use the Zephyr macros
for this.

The mbedtls prefixed mutexes used by the PSA core are now
changed to direcly use the zephyr mutexes. This makes the
design simpler again, since we remove the connection with the
nrf_security mutexes. The mbedlts prefixed mutexes are always
under ifdef conditions inside the PSA core. So using the mechanism
of the nrf_security mutexes doesn't improve anything.

Since both the nrf_security mutexes and the mbedtls prefixed
mutexes indide PSA core are basically just a front end which
use the Zephyr mutexes its better to keep them separated.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
  • Loading branch information
Vge0rge authored and rlubos committed Nov 1, 2024
1 parent 2272c54 commit 16654ce
Show file tree
Hide file tree
Showing 11 changed files with 69 additions and 141 deletions.
8 changes: 4 additions & 4 deletions subsys/nrf_security/src/drivers/cracen/cracenpsa/src/cracen.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ static void cracen_load_microcode(void)

void cracen_acquire(void)
{
nrf_security_mutex_lock(&cracen_mutex);
nrf_security_mutex_lock(cracen_mutex);

if (users++ == 0) {
nrf_cracen_module_enable(NRF_CRACEN, CRACEN_ENABLE_CRYPTOMASTER_Msk |
Expand All @@ -61,12 +61,12 @@ void cracen_acquire(void)
LOG_DBG_MSG("Powered on CRACEN.");
}

nrf_security_mutex_unlock(&cracen_mutex);
nrf_security_mutex_unlock(cracen_mutex);
}

void cracen_release(void)
{
nrf_security_mutex_lock(&cracen_mutex);
nrf_security_mutex_lock(cracen_mutex);

if (--users == 0) {
/* Disable IRQs in the ARM NVIC as the first operation to be
Expand Down Expand Up @@ -102,7 +102,7 @@ void cracen_release(void)
LOG_DBG_MSG("Powered off CRACEN.");
}

nrf_security_mutex_unlock(&cracen_mutex);
nrf_security_mutex_unlock(cracen_mutex);
}

#define CRACEN_NOT_INITIALIZED 0x207467
Expand Down
12 changes: 8 additions & 4 deletions subsys/nrf_security/src/drivers/cracen/cracenpsa/src/ctr_drbg.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ psa_status_t cracen_init_random(cracen_prng_context_t *context)
return PSA_SUCCESS;
}

nrf_security_mutex_lock(&cracen_prng_context_mutex);
nrf_security_mutex_lock(cracen_prng_context_mutex);
safe_memset(&prng, sizeof(prng), 0, sizeof(prng));

/* Get the entropy used to seed the DRBG */
Expand All @@ -153,7 +153,7 @@ psa_status_t cracen_init_random(cracen_prng_context_t *context)
prng.initialized = CRACEN_PRNG_INITIALIZED;

exit:
nrf_security_mutex_unlock(&cracen_prng_context_mutex);
nrf_security_mutex_unlock(cracen_prng_context_mutex);

return silex_statuscodes_to_psa(sx_err);
}
Expand All @@ -173,9 +173,13 @@ psa_status_t cracen_get_random(cracen_prng_context_t *context, uint8_t *output,
return PSA_ERROR_INVALID_ARGUMENT;
}

nrf_security_mutex_lock(&cracen_prng_context_mutex);
nrf_security_mutex_lock(cracen_prng_context_mutex);

if (prng.reseed_counter == 0) {
/* Zephyr mutexes allow the same thread to lock a
* mutex multiple times. So we can call cracen_init_random
* here even though we hold the mutex.
*/
status = cracen_init_random(context);

if (status != PSA_SUCCESS) {
Expand Down Expand Up @@ -238,7 +242,7 @@ psa_status_t cracen_get_random(cracen_prng_context_t *context, uint8_t *output,
prng.reseed_counter += 1;

exit:
nrf_security_mutex_unlock(&cracen_prng_context_mutex);
nrf_security_mutex_unlock(cracen_prng_context_mutex);
return status;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

extern const uint8_t cracen_N3072[384];

extern mbedtls_threading_mutex_t cracen_mutex_symmetric;
extern nrf_security_mutex_t cracen_mutex_symmetric;

#define DEFAULT_KEY_SIZE(bits) (bits), PSA_BITS_TO_BYTES(bits), (1 + 2 * PSA_BITS_TO_BYTES(bits))
static struct {
Expand Down Expand Up @@ -1341,15 +1341,15 @@ psa_status_t cracen_export_key(const psa_key_attributes_t *attributes, const uin
* use case. Here the decision was to avoid defining another mutex to handle the
* push buffer for the rest of the use cases.
*/
nrf_security_mutex_lock(&cracen_mutex_symmetric);
nrf_security_mutex_lock(cracen_mutex_symmetric);
status = cracen_kmu_prepare_key(key_buffer);
if (status == SX_OK) {
memcpy(data, kmu_push_area, key_out_size);
*data_length = key_out_size;
}

(void)cracen_kmu_clean_key(key_buffer);
nrf_security_mutex_unlock(&cracen_mutex_symmetric);
nrf_security_mutex_unlock(cracen_mutex_symmetric);

return silex_statuscodes_to_psa(status);
}
Expand Down Expand Up @@ -1385,7 +1385,7 @@ psa_status_t cracen_copy_key(psa_key_attributes_t *attributes, const uint8_t *so
psa_status_t psa_status;
size_t key_size = PSA_BITS_TO_BYTES(psa_get_key_bits(attributes));

nrf_security_mutex_lock(&cracen_mutex_symmetric);
nrf_security_mutex_lock(cracen_mutex_symmetric);
status = cracen_kmu_prepare_key(source_key);

if (status == SX_OK) {
Expand All @@ -1397,7 +1397,7 @@ psa_status_t cracen_copy_key(psa_key_attributes_t *attributes, const uint8_t *so
}

(void)cracen_kmu_clean_key(source_key);
nrf_security_mutex_unlock(&cracen_mutex_symmetric);
nrf_security_mutex_unlock(cracen_mutex_symmetric);

if (status != SX_OK) {
return silex_statuscodes_to_psa(status);
Expand Down
6 changes: 3 additions & 3 deletions subsys/nrf_security/src/drivers/cracen/cracenpsa/src/kmu.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

#define SECONDARY_SLOT_METADATA_VALUE UINT32_MAX

extern mbedtls_threading_mutex_t cracen_mutex_symmetric;
extern nrf_security_mutex_t cracen_mutex_symmetric;

/* The section .nrf_kmu_reserved_push_area is placed at the top RAM address
* by the linker scripts. We do that for both the secure and non-secure builds.
Expand Down Expand Up @@ -844,13 +844,13 @@ static psa_status_t push_kmu_key_to_ram(uint8_t *key_buffer, size_t key_buffer_s
* Here the decision was to avoid defining another mutex to handle the push buffer for the
* rest of the use cases.
*/
nrf_security_mutex_lock(&cracen_mutex_symmetric);
nrf_security_mutex_lock(cracen_mutex_symmetric);
status = silex_statuscodes_to_psa(cracen_kmu_prepare_key(key_buffer));
if (status == PSA_SUCCESS) {
memcpy(key_buffer, kmu_push_area, key_buffer_size);
safe_memzero(kmu_push_area, sizeof(kmu_push_area));
}
nrf_security_mutex_unlock(&cracen_mutex_symmetric);
nrf_security_mutex_unlock(cracen_mutex_symmetric);

return status;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ int cracen_prng_value_from_pool(uint32_t *prng_value)
{
int status = SX_OK;

nrf_security_mutex_lock(&cracen_prng_pool_mutex);
nrf_security_mutex_lock(cracen_prng_pool_mutex);

if (prng_pool_remaining == 0) {
psa_status_t psa_status =
Expand All @@ -47,6 +47,6 @@ int cracen_prng_value_from_pool(uint32_t *prng_value)
prng_pool_remaining--;

exit:
nrf_security_mutex_unlock(&cracen_prng_pool_mutex);
nrf_security_mutex_unlock(cracen_prng_pool_mutex);
return status;
}
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ struct sx_pk_acq_req sx_pk_acquire_req(const struct sx_pk_cmd_def *cmd)
{
struct sx_pk_acq_req req = {NULL, SX_OK};

nrf_security_mutex_lock(&cracen_mutex_asymmetric);
nrf_security_mutex_lock(cracen_mutex_asymmetric);
req.req = &silex_pk_engine.instance;
req.req->cmd = cmd;
req.req->cnx = &silex_pk_engine;
Expand Down Expand Up @@ -220,7 +220,7 @@ void sx_pk_release_req(sx_pk_req *req)
cracen_release();
req->cmd = NULL;
req->userctxt = NULL;
nrf_security_mutex_unlock(&cracen_mutex_asymmetric);
nrf_security_mutex_unlock(cracen_mutex_asymmetric);
}

struct sx_regs *sx_pk_get_regs(void)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ NRF_SECURITY_MUTEX_DEFINE(cracen_mutex_symmetric);
void sx_hw_reserve(struct sx_dmactl *dma)
{
cracen_acquire();
nrf_security_mutex_lock(&cracen_mutex_symmetric);
nrf_security_mutex_lock(cracen_mutex_symmetric);

if (dma) {
dma->hw_acquired = true;
Expand All @@ -48,7 +48,7 @@ void sx_cmdma_release_hw(struct sx_dmactl *dma)
{
if (dma == NULL || dma->hw_acquired) {
cracen_release();
nrf_security_mutex_unlock(&cracen_mutex_symmetric);
nrf_security_mutex_unlock(cracen_mutex_symmetric);
if (dma) {
dma->hw_acquired = false;
}
Expand Down
9 changes: 5 additions & 4 deletions subsys/nrf_security/src/threading/include/threading_alt.h
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
/*
* Copyright (c) 2023, Arm Limited and Contributors. All rights reserved.
* Copyright (c) 2024 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: BSD-3-Clause
* SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
*/

#ifndef MBEDTLS_THREADING_ALT_H
#define MBEDTLS_THREADING_ALT_H

#include "mbedtls/build_info.h"
#include "nrf_security_mutexes.h"
#include <zephyr/kernel.h>

typedef struct k_mutex mbedtls_threading_mutex_t;

#endif /* MBEDTLS_THREADING_ALT_H */
57 changes: 13 additions & 44 deletions subsys/nrf_security/src/threading/threading_alt.c
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@

/*
* Copyright (c) 2023, Arm Limited and Contributors. All rights reserved.
* Copyright (c) 2024 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: BSD-3-Clause OR Arm’s non-OSI source license
* SPDX-License-Identifier: LicenseRef-Nordic-5-Clause
*/

#include "threading_alt.h"
Expand All @@ -11,62 +10,32 @@
#include <string.h>
#include <zephyr/init.h>
#include <zephyr/kernel.h>
#include <zephyr/sys/__assert.h>

#include "nrf_security_mutexes.h"

#if !defined(MBEDTLS_CONFIG_FILE)
#include "mbedtls/config.h"
#else
#include MBEDTLS_CONFIG_FILE
#endif
K_MUTEX_DEFINE(mbedtls_threading_key_slot_mutex);
K_MUTEX_DEFINE(mbedtls_threading_psa_globaldata_mutex);
K_MUTEX_DEFINE(mbedtls_threading_psa_rngdata_mutex);

NRF_SECURITY_MUTEX_DEFINE(mbedtls_threading_key_slot_mutex);
NRF_SECURITY_MUTEX_DEFINE(mbedtls_threading_psa_globaldata_mutex);
NRF_SECURITY_MUTEX_DEFINE(mbedtls_threading_psa_rngdata_mutex);

static void mbedtls_mutex_init_fn(mbedtls_threading_mutex_t * mutex)
void mbedtls_mutex_init_fn(mbedtls_threading_mutex_t *mutex)
{
if(!k_is_pre_kernel() && !k_is_in_isr()) {
nrf_security_mutex_init(mutex);
}
k_mutex_init(mutex);
}

static void mbedtls_mutex_free_fn(mbedtls_threading_mutex_t * mutex)
void mbedtls_mutex_free_fn(mbedtls_threading_mutex_t *mutex)
{
if(!k_is_pre_kernel() && !k_is_in_isr()) {
nrf_security_mutex_free(mutex);
}
}

static int mbedtls_mutex_lock_fn(mbedtls_threading_mutex_t * mutex)
int mbedtls_mutex_lock_fn(mbedtls_threading_mutex_t *mutex)
{
if(!k_is_pre_kernel() && !k_is_in_isr()) {
return nrf_security_mutex_lock(mutex);
} else {
return 0;
}
return k_mutex_lock(mutex, K_FOREVER);
}

static int mbedtls_mutex_unlock_fn(mbedtls_threading_mutex_t * mutex)
int mbedtls_mutex_unlock_fn(mbedtls_threading_mutex_t *mutex)
{
if(!k_is_pre_kernel() && !k_is_in_isr()) {
return nrf_security_mutex_unlock(mutex);
} else {
return 0;
}
return k_mutex_unlock(mutex);
}

void (*mbedtls_mutex_init)(mbedtls_threading_mutex_t *mutex) = mbedtls_mutex_init_fn;
void (*mbedtls_mutex_free)(mbedtls_threading_mutex_t *mutex) = mbedtls_mutex_free_fn;
int (*mbedtls_mutex_lock)(mbedtls_threading_mutex_t *mutex) = mbedtls_mutex_lock_fn;
int (*mbedtls_mutex_unlock)(mbedtls_threading_mutex_t *mutex) = mbedtls_mutex_unlock_fn;

static int post_kernel_init(void)
{
mbedtls_mutex_init(&mbedtls_threading_key_slot_mutex);
mbedtls_mutex_init(&mbedtls_threading_psa_globaldata_mutex);
mbedtls_mutex_init(&mbedtls_threading_psa_rngdata_mutex);
return 0;
}

SYS_INIT(post_kernel_init, POST_KERNEL, CONFIG_KERNEL_INIT_PRIORITY_DEFAULT);
52 changes: 12 additions & 40 deletions subsys/nrf_security/src/utils/nrf_security_mutexes.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,63 +11,35 @@
#include <stdbool.h>
#include "nrf_security_mutexes.h"

#if !defined(__NRF_TFM__)
#ifdef NRF_SECURITY_MUTEX_IMPLEMENTATION
#include <zephyr/kernel.h>
#endif

#if defined(CONFIG_MULTITHREADING) && !defined(__NRF_TFM__)

void nrf_security_mutex_init(mbedtls_threading_mutex_t * mutex)
#ifdef NRF_SECURITY_MUTEX_IMPLEMENTATION
int nrf_security_mutex_lock(nrf_security_mutex_t mutex)
{
if ((mutex->flags & NRF_SECURITY_MUTEX_FLAGS_INITIALIZED) == 0) {
k_mutex_init(&mutex->mutex);
}

mutex->flags |= NRF_SECURITY_MUTEX_FLAGS_INITIALIZED;
}
int ret = k_mutex_lock(mutex, K_FOREVER);

void nrf_security_mutex_free(mbedtls_threading_mutex_t * mutex)
{
(void)mutex;
__ASSERT_NO_MSG(ret == 0);
return ret;
}

int nrf_security_mutex_lock(mbedtls_threading_mutex_t * mutex)
int nrf_security_mutex_unlock(nrf_security_mutex_t mutex)
{
if ((mutex->flags & NRF_SECURITY_MUTEX_FLAGS_INITIALIZED) != 0) {
return k_mutex_lock(&mutex->mutex, K_FOREVER);
} else {
return -EINVAL;
}
}
int ret = k_mutex_unlock(mutex);

int nrf_security_mutex_unlock(mbedtls_threading_mutex_t * mutex)
{
if ((mutex->flags & NRF_SECURITY_MUTEX_FLAGS_INITIALIZED) != 0) {
return k_mutex_unlock(&mutex->mutex);
} else {
return -EINVAL;
}
__ASSERT_NO_MSG(ret == 0);
return ret;
}

#else

void nrf_security_mutex_init(mbedtls_threading_mutex_t * mutex)
{
(void)mutex;
}

void nrf_security_mutex_free(mbedtls_threading_mutex_t * mutex)
{
(void)mutex;
}

int nrf_security_mutex_lock(mbedtls_threading_mutex_t * mutex)
int nrf_security_mutex_lock(nrf_security_mutex_t mutex)
{
(void)mutex;
return 0;
}

int nrf_security_mutex_unlock(mbedtls_threading_mutex_t * mutex)
int nrf_security_mutex_unlock(nrf_security_mutex_t mutex)
{
(void)mutex;
return 0;
Expand Down
Loading

0 comments on commit 16654ce

Please sign in to comment.