From ca94a49eff97b9c222c7d2d5897ab0da6d7aae2b Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Mon, 4 Jun 2018 10:29:39 +0300 Subject: [PATCH 1/8] Add reference counter for platform context 1. Move the `mbedtls_platform_context` to be platform code, in `features/mbedtls/platfrom/`. 2. Add static refernce counter, to setup and teardown the platform code only once. 3. Adjust Cryptocell porting accordingly. --- .../FEATURE_CRYPTOCELL310/Readme.md | 4 +- .../cc_platform_nrf52840.c | 33 ----------- .../crypto_platform.c} | 31 ++++------ .../{cc_platform.h => crypto_platform.h} | 8 ++- .../platform/inc}/platform_alt.h | 30 +++++----- features/mbedtls/platform/src/platform_alt.c | 59 +++++++++++++++++++ 6 files changed, 92 insertions(+), 73 deletions(-) delete mode 100644 features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/cc_platform_nrf52840.c rename features/cryptocell/FEATURE_CRYPTOCELL310/{platform_alt.c => TARGET_MCU_NRF52840/crypto_platform.c} (67%) rename features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/{cc_platform.h => crypto_platform.h} (81%) rename features/{cryptocell/FEATURE_CRYPTOCELL310 => mbedtls/platform/inc}/platform_alt.h (61%) create mode 100644 features/mbedtls/platform/src/platform_alt.c diff --git a/features/cryptocell/FEATURE_CRYPTOCELL310/Readme.md b/features/cryptocell/FEATURE_CRYPTOCELL310/Readme.md index 629ef238ffe..941954e45ac 100644 --- a/features/cryptocell/FEATURE_CRYPTOCELL310/Readme.md +++ b/features/cryptocell/FEATURE_CRYPTOCELL310/Readme.md @@ -16,6 +16,6 @@ To port your CC 310 driver to Mbed OS on your specific target, do the following: 1. In `objects.h`, include `objects_cryptocell.h`. You can use the `FEATURE_CRYPTOCELL310` precompilation check as defined above. 1. In `features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_`, add your platform-specific libraries for all toolchains in `TOOLCHAIN_ARM`, `TOOLCHAIN_GCC_ARM` and `TOOLCHAIN_IAR` respectively. 1. Add your CC setup code: - * Implement `cc_platform_setup()` and `cc_platform_terminate()` to enable CC on your platform, in case you have board-specific setup functionality, required for CC setup. These functions can be empty. - * Define `cc_platform_ctx` in `cc_platform.h` in a way that suits your implementation. + * Implement `crypto_platform_setup()` and `crypto_platform_terminate()` to enable CC on your platform, in case you have board-specific setup functionality, required for CC setup. You MUST call 'SaSi_LibInit()` and 'SaSi_LibFini()' respectively in these functions. + * Define `crypto_platform_ctx` in `crypto_platform.h` in a way that suits your implementation. diff --git a/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/cc_platform_nrf52840.c b/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/cc_platform_nrf52840.c deleted file mode 100644 index 1ff1462e9a0..00000000000 --- a/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/cc_platform_nrf52840.c +++ /dev/null @@ -1,33 +0,0 @@ - /* - * cc_platform_nrf52840.c - * - * Copyright (C) 2018, Arm Limited, All Rights Reserved - * SPDX-License-Identifier: Apache-2.0 - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -#include "platform_alt.h" -#include "nrf52840.h" - -int cc_platform_setup( cc_platform_ctx *ctx ) -{ - NRF_CRYPTOCELL->ENABLE = 1; - return ( 0 ); -} - -void cc_platform_terminate( cc_platform_ctx *ctx ) -{ - NRF_CRYPTOCELL->ENABLE = 0; -} diff --git a/features/cryptocell/FEATURE_CRYPTOCELL310/platform_alt.c b/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.c similarity index 67% rename from features/cryptocell/FEATURE_CRYPTOCELL310/platform_alt.c rename to features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.c index ed579901ada..6d2e4ee7fe6 100644 --- a/features/cryptocell/FEATURE_CRYPTOCELL310/platform_alt.c +++ b/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.c @@ -1,5 +1,5 @@ /* - * platform_alt.c + * crypto_platform.c * * Copyright (C) 2018, Arm Limited, All Rights Reserved * SPDX-License-Identifier: Apache-2.0 @@ -18,10 +18,10 @@ * */ -#include "mbedtls/platform.h" -#if defined(MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT) +#include "platform_alt.h" +#include "nrf52840.h" #include "sns_silib.h" - +#if defined(MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT) /* once https://github.com/ARMmbed/mbedtls/issues/1200 will be supported, * rndState should be part of mbedtls_platform_context * Until then, we should keep it global and extern */ @@ -29,29 +29,20 @@ CRYS_RND_State_t rndState = { { 0 } } ; CRYS_RND_WorkBuff_t rndWorkBuff = { { 0 } } ; - -int mbedtls_platform_setup( mbedtls_platform_context *ctx ) +int crypto_platform_setup( crypto_platform_ctx *ctx ) { - int ret = 0; - if( ctx == NULL ) - return ( -1 ); - - /* call platform specific code to setup CC driver*/ - if( ( ret = cc_platform_setup( &ctx->platform_impl_ctx ) ) != 0 ) - return ( ret ); + NRF_CRYPTOCELL->ENABLE = 1; if( SaSi_LibInit( &rndState, &rndWorkBuff ) != 0 ) - return ( -1 ); + return ( -1 ); + return ( 0 ); } -void mbedtls_platform_teardown( mbedtls_platform_context *ctx ) +void crypto_platform_terminate( crypto_platform_ctx *ctx ) { - if( ctx == NULL ) - return; - SaSi_LibFini( &rndState ); - cc_platform_terminate( &ctx->platform_impl_ctx ); + NRF_CRYPTOCELL->ENABLE = 0; } -#endif /* MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT*/ +#endif /* MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT */ diff --git a/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/cc_platform.h b/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.h similarity index 81% rename from features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/cc_platform.h rename to features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.h index 87433c55774..f0dc1cade79 100644 --- a/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/cc_platform.h +++ b/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.h @@ -19,6 +19,7 @@ */ #ifndef __CC_PLATFORM_H_ #define __CC_PLATFORM_H_ +#if defined(MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT) /** * \brief The CC platform context structure. * @@ -27,7 +28,10 @@ */ typedef struct { char dummy; /**< Placeholder member, as empty structs are not portable. */ + /* + * Add CRYS_RND_State_t rndState; when https://github.com/ARMmbed/mbedtls/issues/1200 is supported + */ } -cc_platform_ctx; - +crypto_platform_ctx; +#endif /* MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT */ #endif /* __CC_PLATFORM_H_ */ diff --git a/features/cryptocell/FEATURE_CRYPTOCELL310/platform_alt.h b/features/mbedtls/platform/inc/platform_alt.h similarity index 61% rename from features/cryptocell/FEATURE_CRYPTOCELL310/platform_alt.h rename to features/mbedtls/platform/inc/platform_alt.h index 8ed39ae15d2..27ec530868d 100644 --- a/features/cryptocell/FEATURE_CRYPTOCELL310/platform_alt.h +++ b/features/mbedtls/platform/inc/platform_alt.h @@ -20,8 +20,9 @@ #ifndef __PLATFORM_ALT__ #define __PLATFORM_ALT__ -#include "cc_platform.h" -#include "crys_rnd.h" +#include "platform_mbed.h" +#if defined(MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT) +#include "crypto_platform.h" /** * \brief The platform context structure. @@ -30,40 +31,37 @@ * setup or teardown operations. */ typedef struct { - cc_platform_ctx platform_impl_ctx; /** A context holding all the partner's platform specific context */ - /* - * Add CRYS_RND_State_t rndState; when https://github.com/ARMmbed/mbedtls/issues/1200 is supported - * */ + crypto_platform_ctx platform_impl_ctx; /* A context holding all the platform specific context for cryptography. Should be defined in crypto_platform.h */ } mbedtls_platform_context; +void mbedtls_platform_init( mbedtls_platform_context* ctx); /** - * \brief This function performs any partner platform initialization operations, - * needed top enable CryptoCell. + * \brief This function performs any platform initialization operations, + * needed for setting up cryptographic modules. * * \param ctx The platform specific context. * * \return \c 0 on success. * - * \note This function is intended to allow platform-specific initialization for CryptoCell, - * and is called before initializing the CC library(SaSi_LibInit). Its + * \note This function is intended to allow platform-specific initialization for Mbed TLS, + * and is called before initializing the Mbed TLS functions. Its * implementation is platform-specific, and its implementation MUST be provided. * */ -int cc_platform_setup( cc_platform_ctx *ctx ); +int crypto_platform_setup( crypto_platform_ctx *ctx ); /** - * \brief This function performs any partner platform teardown operations, to disable CryptoCell. + * \brief This function performs any platform teardown operations, to disable cryptographic operations. * * \param ctx The platform specific context. * - * \note This function is called after terminating CC library(SaSi_LibFini) - * and intended to free any resource used for CryptoCell by the platform. + * \note This function is intended to free any resource used Mbed TLS by the platform. * Its implementation is platform-specific,and its implementation MUST be provided. * */ -void cc_platform_terminate( cc_platform_ctx *ctx ); - +void crypto_platform_terminate( crypto_platform_ctx *ctx ); +#endif #endif /* __PLATFORM_ALT__ */ diff --git a/features/mbedtls/platform/src/platform_alt.c b/features/mbedtls/platform/src/platform_alt.c new file mode 100644 index 00000000000..bb3b264123f --- /dev/null +++ b/features/mbedtls/platform/src/platform_alt.c @@ -0,0 +1,59 @@ + /* + * platform_alt.c + * + * Copyright (C) 2018, Arm Limited, All Rights Reserved + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#include "mbedtls/platform.h" +#if defined(MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT) + +static int reference_count = 0; + +int mbedtls_platform_setup( mbedtls_platform_context *ctx ) +{ + int ret = 0; + if( ctx == NULL ) + return ( -1 ); + + reference_count++; + + if( reference_count == 1 ) + { + /* call platform specific code to setup crypto driver*/ + ret = crypto_platform_setup( &ctx->platform_impl_ctx ); + } + return ( ret ); +} + +void mbedtls_platform_teardown( mbedtls_platform_context *ctx ) +{ + if( ctx == NULL ) + return; + + if( reference_count == 0 ) + return; + + reference_count--; + + if( reference_count == 0 ) + { + /* call platform specific code to terminate crypto driver*/ + crypto_platform_terminate( &ctx->platform_impl_ctx ); + } +} + +#endif /* MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT*/ From c3b31bc500024de2c34852ae6116030f05b8b05f Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Mon, 4 Jun 2018 14:01:59 +0300 Subject: [PATCH 2/8] Add Mbed TLS Platform module errors 1. Add error codes for platform setup \ teardown. 2. Reassign `reference_count` to 0 after terminating platform, and remove condition for 0 --- .../TARGET_MCU_NRF52840/crypto_platform.c | 2 +- features/mbedtls/platform/inc/platform_mbed.h | 3 +++ features/mbedtls/platform/src/platform_alt.c | 8 +++----- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.c b/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.c index 6d2e4ee7fe6..06441bfcad8 100644 --- a/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.c +++ b/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.c @@ -34,7 +34,7 @@ int crypto_platform_setup( crypto_platform_ctx *ctx ) NRF_CRYPTOCELL->ENABLE = 1; if( SaSi_LibInit( &rndState, &rndWorkBuff ) != 0 ) - return ( -1 ); + return ( MBEDTLS_PLATFORM_HW_FAILED ); return ( 0 ); } diff --git a/features/mbedtls/platform/inc/platform_mbed.h b/features/mbedtls/platform/inc/platform_mbed.h index 8632ec03283..5412b213836 100644 --- a/features/mbedtls/platform/inc/platform_mbed.h +++ b/features/mbedtls/platform/inc/platform_mbed.h @@ -24,3 +24,6 @@ #if defined(MBEDTLS_CONFIG_HW_SUPPORT) #include "mbedtls_device.h" #endif + +#define MBEDTLS_PLATFORM_INVALID_DATA -0x0080 +#define MBEDTLS_PLATFORM_HW_FAILED -0x0082 diff --git a/features/mbedtls/platform/src/platform_alt.c b/features/mbedtls/platform/src/platform_alt.c index bb3b264123f..41ceb9c31ba 100644 --- a/features/mbedtls/platform/src/platform_alt.c +++ b/features/mbedtls/platform/src/platform_alt.c @@ -27,7 +27,7 @@ int mbedtls_platform_setup( mbedtls_platform_context *ctx ) { int ret = 0; if( ctx == NULL ) - return ( -1 ); + return ( MBEDTLS_PLATFORM_INVALID_DATA ); reference_count++; @@ -44,15 +44,13 @@ void mbedtls_platform_teardown( mbedtls_platform_context *ctx ) if( ctx == NULL ) return; - if( reference_count == 0 ) - return; - reference_count--; - if( reference_count == 0 ) + if( reference_count <= 0 ) { /* call platform specific code to terminate crypto driver*/ crypto_platform_terminate( &ctx->platform_impl_ctx ); + reference_count = 0; } } From 127b68fbbca6fd7de4daa862f1e7420a86a6e9e0 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Tue, 28 Aug 2018 20:29:26 +0300 Subject: [PATCH 3/8] Make the platform context a global variable Make the platform context a global variable, adding the refernce counter to it. --- .../TARGET_MCU_NRF52840/crypto_platform.c | 10 +++----- .../TARGET_MCU_NRF52840/crypto_platform.h | 14 +++++------ .../cryptocell/FEATURE_CRYPTOCELL310/trng.c | 9 ++++--- features/mbedtls/platform/inc/platform_alt.h | 2 +- features/mbedtls/platform/src/platform_alt.c | 25 ++++++++----------- 5 files changed, 25 insertions(+), 35 deletions(-) diff --git a/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.c b/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.c index 06441bfcad8..01eda67fd6f 100644 --- a/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.c +++ b/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.c @@ -22,18 +22,14 @@ #include "nrf52840.h" #include "sns_silib.h" #if defined(MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT) -/* once https://github.com/ARMmbed/mbedtls/issues/1200 will be supported, - * rndState should be part of mbedtls_platform_context - * Until then, we should keep it global and extern */ -CRYS_RND_State_t rndState = { { 0 } } ; -CRYS_RND_WorkBuff_t rndWorkBuff = { { 0 } } ; +static CRYS_RND_WorkBuff_t rndWorkBuff = { { 0 } } ; int crypto_platform_setup( crypto_platform_ctx *ctx ) { NRF_CRYPTOCELL->ENABLE = 1; - if( SaSi_LibInit( &rndState, &rndWorkBuff ) != 0 ) + if( SaSi_LibInit( &ctx->rndState, &rndWorkBuff ) != 0 ) return ( MBEDTLS_PLATFORM_HW_FAILED ); return ( 0 ); @@ -41,7 +37,7 @@ int crypto_platform_setup( crypto_platform_ctx *ctx ) void crypto_platform_terminate( crypto_platform_ctx *ctx ) { - SaSi_LibFini( &rndState ); + SaSi_LibFini( &ctx->rndState ); NRF_CRYPTOCELL->ENABLE = 0; } diff --git a/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.h b/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.h index f0dc1cade79..9b611b249d5 100644 --- a/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.h +++ b/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.h @@ -1,5 +1,5 @@ /* - * cc_platform.h + * crypto_platform.h * * Copyright (C) 2018, Arm Limited, All Rights Reserved * SPDX-License-Identifier: Apache-2.0 @@ -17,9 +17,10 @@ * limitations under the License. * */ -#ifndef __CC_PLATFORM_H_ -#define __CC_PLATFORM_H_ +#ifndef __CRYPTO_PLATFORM_H_ +#define __CRYPTO_PLATFORM_H_ #if defined(MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT) +#include "crys_rnd.h" /** * \brief The CC platform context structure. * @@ -27,11 +28,8 @@ * setup or teardown operations. */ typedef struct { - char dummy; /**< Placeholder member, as empty structs are not portable. */ - /* - * Add CRYS_RND_State_t rndState; when https://github.com/ARMmbed/mbedtls/issues/1200 is supported - */ + CRYS_RND_State_t rndState; } crypto_platform_ctx; #endif /* MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT */ -#endif /* __CC_PLATFORM_H_ */ +#endif /* __CRYPTO_PLATFORM_H_ */ diff --git a/features/cryptocell/FEATURE_CRYPTOCELL310/trng.c b/features/cryptocell/FEATURE_CRYPTOCELL310/trng.c index c0d238e71e3..5af3e3df2e6 100644 --- a/features/cryptocell/FEATURE_CRYPTOCELL310/trng.c +++ b/features/cryptocell/FEATURE_CRYPTOCELL310/trng.c @@ -22,9 +22,10 @@ #include #include "trng_api.h" +#include "mbedtls/platform.h" -extern CRYS_RND_State_t rndState; -extern CRYS_RND_WorkBuff_t rndWorkBuff; +extern mbedtls_platform_context ctx; +static CRYS_RND_WorkBuff_t rndWorkBuff; /* Implementation that should never be optimized out by the compiler */ static void mbedtls_zeroize( void *v, size_t n ) { @@ -48,7 +49,7 @@ CRYSError_t LLF_RND_GetTrngSource( void trng_init(trng_t *obj) { - RNG_PLAT_SetUserRngParameters(&rndState, obj); + RNG_PLAT_SetUserRngParameters(&ctx.platform_impl_ctx.rndState, obj); } void trng_free(trng_t *obj) @@ -66,7 +67,7 @@ int trng_get_bytes(trng_t *obj, uint8_t *output, size_t length, size_t *outputLe uint32_t actualLength; ret = LLF_RND_GetTrngSource( - &rndState , /*in/out*/ + &ctx.platform_impl_ctx.rndState , /*in/out*/ obj, /*in/out*/ 0, /*in*/ &entropySizeBits, /*in/out*/ diff --git a/features/mbedtls/platform/inc/platform_alt.h b/features/mbedtls/platform/inc/platform_alt.h index 27ec530868d..1b556954cfd 100644 --- a/features/mbedtls/platform/inc/platform_alt.h +++ b/features/mbedtls/platform/inc/platform_alt.h @@ -23,7 +23,6 @@ #include "platform_mbed.h" #if defined(MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT) #include "crypto_platform.h" - /** * \brief The platform context structure. * @@ -32,6 +31,7 @@ */ typedef struct { crypto_platform_ctx platform_impl_ctx; /* A context holding all the platform specific context for cryptography. Should be defined in crypto_platform.h */ + int reference_count; } mbedtls_platform_context; diff --git a/features/mbedtls/platform/src/platform_alt.c b/features/mbedtls/platform/src/platform_alt.c index 41ceb9c31ba..e2acf320365 100644 --- a/features/mbedtls/platform/src/platform_alt.c +++ b/features/mbedtls/platform/src/platform_alt.c @@ -20,37 +20,32 @@ #include "mbedtls/platform.h" #if defined(MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT) +mbedtls_platform_context ctx = {0}; -static int reference_count = 0; - -int mbedtls_platform_setup( mbedtls_platform_context *ctx ) +int mbedtls_platform_setup( mbedtls_platform_context *obsolete_ctx ) { int ret = 0; - if( ctx == NULL ) - return ( MBEDTLS_PLATFORM_INVALID_DATA ); - reference_count++; + ctx.reference_count++; - if( reference_count == 1 ) + if( ctx.reference_count == 1 ) { /* call platform specific code to setup crypto driver*/ - ret = crypto_platform_setup( &ctx->platform_impl_ctx ); + ret = crypto_platform_setup( &ctx.platform_impl_ctx ); } return ( ret ); } -void mbedtls_platform_teardown( mbedtls_platform_context *ctx ) +void mbedtls_platform_teardown( mbedtls_platform_context *obsolete_ctx ) { - if( ctx == NULL ) - return; - reference_count--; + ctx.reference_count--; - if( reference_count <= 0 ) + if( ctx.reference_count <= 0 ) { /* call platform specific code to terminate crypto driver*/ - crypto_platform_terminate( &ctx->platform_impl_ctx ); - reference_count = 0; + crypto_platform_terminate( &ctx.platform_impl_ctx ); + ctx.reference_count = 0; } } From 479438953f17d5b9949a91cf81c24ab4380a950a Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Wed, 29 Aug 2018 19:02:57 +0300 Subject: [PATCH 4/8] Rename error codes 1. Rename error codes to fit Mbed TLS error code names. 2. Remove the Invalid input error code, as it's not used anymore. --- .../TARGET_MCU_NRF52840/crypto_platform.c | 2 +- features/mbedtls/platform/inc/platform_mbed.h | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.c b/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.c index 01eda67fd6f..7a5781fc43e 100644 --- a/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.c +++ b/features/cryptocell/FEATURE_CRYPTOCELL310/TARGET_MCU_NRF52840/crypto_platform.c @@ -30,7 +30,7 @@ int crypto_platform_setup( crypto_platform_ctx *ctx ) NRF_CRYPTOCELL->ENABLE = 1; if( SaSi_LibInit( &ctx->rndState, &rndWorkBuff ) != 0 ) - return ( MBEDTLS_PLATFORM_HW_FAILED ); + return ( MBEDTLS_ERR_PLATFORM_HW_FAILED ); return ( 0 ); } diff --git a/features/mbedtls/platform/inc/platform_mbed.h b/features/mbedtls/platform/inc/platform_mbed.h index 5412b213836..efb5b6dc11b 100644 --- a/features/mbedtls/platform/inc/platform_mbed.h +++ b/features/mbedtls/platform/inc/platform_mbed.h @@ -25,5 +25,4 @@ #include "mbedtls_device.h" #endif -#define MBEDTLS_PLATFORM_INVALID_DATA -0x0080 -#define MBEDTLS_PLATFORM_HW_FAILED -0x0082 +#define MBEDTLS_ERR_PLATFORM_HW_FAILED -0x0080 From 1f5cee967d0c28c57162646ad465e562386e8cec Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Thu, 30 Aug 2018 11:18:23 +0300 Subject: [PATCH 5/8] Address concurrency and style issues 1. Use atomic operations to increase and decrease counter. 2. Style fixes. Remove unused function declaration. --- features/mbedtls/platform/inc/platform_alt.h | 1 - features/mbedtls/platform/src/platform_alt.c | 13 +++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/features/mbedtls/platform/inc/platform_alt.h b/features/mbedtls/platform/inc/platform_alt.h index 1b556954cfd..7246c3bb77c 100644 --- a/features/mbedtls/platform/inc/platform_alt.h +++ b/features/mbedtls/platform/inc/platform_alt.h @@ -36,7 +36,6 @@ typedef struct { mbedtls_platform_context; -void mbedtls_platform_init( mbedtls_platform_context* ctx); /** * \brief This function performs any platform initialization operations, * needed for setting up cryptographic modules. diff --git a/features/mbedtls/platform/src/platform_alt.c b/features/mbedtls/platform/src/platform_alt.c index e2acf320365..bf10192c1c9 100644 --- a/features/mbedtls/platform/src/platform_alt.c +++ b/features/mbedtls/platform/src/platform_alt.c @@ -20,17 +20,19 @@ #include "mbedtls/platform.h" #if defined(MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT) -mbedtls_platform_context ctx = {0}; +#include "mbed_critical.h" + +mbedtls_platform_context ctx = { }; int mbedtls_platform_setup( mbedtls_platform_context *obsolete_ctx ) { int ret = 0; - ctx.reference_count++; + core_util_atomic_incr_u32( ( volatile uint32_t * )&ctx.reference_count, 1 ); if( ctx.reference_count == 1 ) { - /* call platform specific code to setup crypto driver*/ + /* call platform specific code to setup crypto driver */ ret = crypto_platform_setup( &ctx.platform_impl_ctx ); } return ( ret ); @@ -39,11 +41,10 @@ int mbedtls_platform_setup( mbedtls_platform_context *obsolete_ctx ) void mbedtls_platform_teardown( mbedtls_platform_context *obsolete_ctx ) { - ctx.reference_count--; - + core_util_atomic_decr_u32( ( volatile uint32_t * )&ctx.reference_count, 1 ); if( ctx.reference_count <= 0 ) { - /* call platform specific code to terminate crypto driver*/ + /* call platform specific code to terminate crypto driver */ crypto_platform_terminate( &ctx.platform_impl_ctx ); ctx.reference_count = 0; } From 666ebe392ac9635a8ec065ead073f85e9e1ac6f7 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Thu, 30 Aug 2018 13:51:58 +0300 Subject: [PATCH 6/8] Change the terminate limit check Check for counter to be `< 1` instead of `<= 0` before terminating. --- features/mbedtls/platform/src/platform_alt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/mbedtls/platform/src/platform_alt.c b/features/mbedtls/platform/src/platform_alt.c index bf10192c1c9..c76c593036e 100644 --- a/features/mbedtls/platform/src/platform_alt.c +++ b/features/mbedtls/platform/src/platform_alt.c @@ -42,7 +42,7 @@ void mbedtls_platform_teardown( mbedtls_platform_context *obsolete_ctx ) { core_util_atomic_decr_u32( ( volatile uint32_t * )&ctx.reference_count, 1 ); - if( ctx.reference_count <= 0 ) + if( ctx.reference_count < 1 ) { /* call platform specific code to terminate crypto driver */ crypto_platform_terminate( &ctx.platform_impl_ctx ); From c1b6fdc5af8b620493c331a01dbbf9e81f1b0ea0 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Fri, 31 Aug 2018 13:53:29 +0300 Subject: [PATCH 7/8] Rename parameter name Rename `obsolete_ctx` to `unused_ctx` as it is simply unused. --- features/mbedtls/platform/inc/platform_alt.h | 4 ++-- features/mbedtls/platform/src/platform_alt.c | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/features/mbedtls/platform/inc/platform_alt.h b/features/mbedtls/platform/inc/platform_alt.h index 7246c3bb77c..687be6d642d 100644 --- a/features/mbedtls/platform/inc/platform_alt.h +++ b/features/mbedtls/platform/inc/platform_alt.h @@ -49,7 +49,7 @@ mbedtls_platform_context; * implementation is platform-specific, and its implementation MUST be provided. * */ -int crypto_platform_setup( crypto_platform_ctx *ctx ); +int crypto_platform_setup( crypto_platform_ctx *unused_ctx ); /** * \brief This function performs any platform teardown operations, to disable cryptographic operations. @@ -60,7 +60,7 @@ int crypto_platform_setup( crypto_platform_ctx *ctx ); * Its implementation is platform-specific,and its implementation MUST be provided. * */ -void crypto_platform_terminate( crypto_platform_ctx *ctx ); +void crypto_platform_terminate( crypto_platform_ctx *unused_ctx ); #endif #endif /* __PLATFORM_ALT__ */ diff --git a/features/mbedtls/platform/src/platform_alt.c b/features/mbedtls/platform/src/platform_alt.c index c76c593036e..285261032ce 100644 --- a/features/mbedtls/platform/src/platform_alt.c +++ b/features/mbedtls/platform/src/platform_alt.c @@ -24,7 +24,7 @@ mbedtls_platform_context ctx = { }; -int mbedtls_platform_setup( mbedtls_platform_context *obsolete_ctx ) +int mbedtls_platform_setup( mbedtls_platform_context *unused_ctx ) { int ret = 0; @@ -38,9 +38,8 @@ int mbedtls_platform_setup( mbedtls_platform_context *obsolete_ctx ) return ( ret ); } -void mbedtls_platform_teardown( mbedtls_platform_context *obsolete_ctx ) +void mbedtls_platform_teardown( mbedtls_platform_context *unused_ctx ) { - core_util_atomic_decr_u32( ( volatile uint32_t * )&ctx.reference_count, 1 ); if( ctx.reference_count < 1 ) { From a2531b567466ade0e8a1061a0cd680dd0ce0fe63 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Sun, 2 Sep 2018 10:48:31 +0300 Subject: [PATCH 8/8] Fix build error on IAR IAR fails to build when a variable is initialized with empty curly braces. Added `{ { 0 } }` to fix that. --- features/cryptocell/FEATURE_CRYPTOCELL310/trng.c | 2 +- features/mbedtls/platform/src/platform_alt.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/features/cryptocell/FEATURE_CRYPTOCELL310/trng.c b/features/cryptocell/FEATURE_CRYPTOCELL310/trng.c index 5af3e3df2e6..5c3431d2e3f 100644 --- a/features/cryptocell/FEATURE_CRYPTOCELL310/trng.c +++ b/features/cryptocell/FEATURE_CRYPTOCELL310/trng.c @@ -25,7 +25,7 @@ #include "mbedtls/platform.h" extern mbedtls_platform_context ctx; -static CRYS_RND_WorkBuff_t rndWorkBuff; +static CRYS_RND_WorkBuff_t rndWorkBuff = { { 0 } }; /* Implementation that should never be optimized out by the compiler */ static void mbedtls_zeroize( void *v, size_t n ) { diff --git a/features/mbedtls/platform/src/platform_alt.c b/features/mbedtls/platform/src/platform_alt.c index 285261032ce..d3250d11053 100644 --- a/features/mbedtls/platform/src/platform_alt.c +++ b/features/mbedtls/platform/src/platform_alt.c @@ -22,7 +22,7 @@ #if defined(MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT) #include "mbed_critical.h" -mbedtls_platform_context ctx = { }; +mbedtls_platform_context ctx = { { 0 } }; int mbedtls_platform_setup( mbedtls_platform_context *unused_ctx ) {