Skip to content

Nuvoton: Fix mbedtls crypto ECC AC management failed #8985

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

Closed
wants to merge 4 commits into from
Closed

Nuvoton: Fix mbedtls crypto ECC AC management failed #8985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e75c34c
[Nuvoton] Fix possible hang in crypto_submodule_release()
ccli8 Dec 5, 2018
bed46e3
[M487] Fix return error code when ECC H/W acceleratioin is failed
ccli8 Dec 5, 2018
550e762
[M487] Fix crypto ECC accelerator management failed
ccli8 Dec 5, 2018
406d480
[M487] Change return error code to MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAIL…
ccli8 Dec 7, 2018
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
122 changes: 96 additions & 26 deletions features/mbedtls/targets/TARGET_NUVOTON/TARGET_M480/ecp/ecp_internal_alt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
* would be defined in mbedtls/ecp.h from ecp.c for our inclusion */
#define ECP_SHORTWEIERSTRASS

#include "mbedtls/ssl.h"
#include "mbedtls/ecp_internal.h"
#include "mbed_toolchain.h"
#include "mbed_assert.h"
Expand Down Expand Up @@ -118,6 +119,15 @@
} \
} while(0)

/* Open ECC accelerator
*
* internal_open_ecc_ac()/internal_close_ecc_ac() must be paired.
*/
NU_STATIC void internal_open_ecc_ac(void);

/* Close ECC accelerator */
NU_STATIC void internal_close_ecc_ac(void);

/**
* \brief Configure ECCOP operation, start it, and wait for its completion
*
Expand Down Expand Up @@ -222,26 +232,20 @@ unsigned char mbedtls_internal_ecp_grp_capable( const mbedtls_ecp_group *grp )

int mbedtls_internal_ecp_init( const mbedtls_ecp_group *grp )
{
/* TODO: Change busy-wait with other means to release CPU */
/* Acquire ownership of ECC accelerator */
while (! crypto_ecc_acquire());

/* Init crypto module */
crypto_init();
ECC_ENABLE_INT();

/* Address mbedtls_internal_ecp_init()/mbedtls_internal_ecp_free() are not paired
*
* Mbed TLS doesn't guarantee mbedtls_internal_ecp_init()/mbedtls_internal_ecp_free() are
* paired. To avoid multiple operations to the same ECC accelerator simultaneously, we
* narrow open period to just real ECC accelerator operation in internal_run_eccop()/
* internal_run_modop().
*/
return 0;
}

void mbedtls_internal_ecp_free( const mbedtls_ecp_group *grp )
{
/* Disable ECC interrupt */
ECC_DISABLE_INT();
/* Uninit crypto module */
crypto_uninit();

/* Release ownership of ECC accelerator */
crypto_ecc_release();
/* See comment in mbedtls_internal_ecp_init() */
}

#if defined(ECP_SHORTWEIERSTRASS)
Expand Down Expand Up @@ -464,6 +468,41 @@ int mbedtls_internal_ecp_normalize_jac_many(const mbedtls_ecp_group *grp,
}
#endif

/* Open ECC accelerator
*
* To avoid race condition, we must acquire ownership of ECC accelerator first and then do
* ECC accelerator related initialization.
*/
NU_STATIC void internal_open_ecc_ac(void)
{
/* TODO: Change busy-wait with other means to release CPU */
/* Acquire ownership of ECC accelerator */
while (! crypto_ecc_acquire());

/* Initialize crypto module */
crypto_init();

/* Enable ECC interrupt */
ECC_ENABLE_INT();
}

/* Close ECC accelerator
*
* To avoid race condition, we must do ECC accelerator related un-initialization first
* and then release ownership of ECC accelerator.
*/
NU_STATIC void internal_close_ecc_ac(void)
{
/* Disable ECC interrupt */
ECC_DISABLE_INT();

/* Uninit crypto module */
crypto_uninit();

/* Release ownership of ECC accelerator */
crypto_ecc_release();
}

NU_STATIC int internal_run_eccop(const mbedtls_ecp_group *grp,
mbedtls_ecp_point *R,
const mbedtls_mpi *m,
Expand All @@ -489,6 +528,7 @@ NU_STATIC int internal_run_eccop(const mbedtls_ecp_group *grp,

int ret;
bool ecc_done;
bool ecc_ac_open = false;

mbedtls_mpi N_;
const mbedtls_mpi *Np;
Expand Down Expand Up @@ -589,7 +629,18 @@ NU_STATIC int internal_run_eccop(const mbedtls_ecp_group *grp,
ret = MBEDTLS_ERR_ECP_BAD_INPUT_DATA;
goto cleanup;
}


/* Open ECC accelerator
*
* To guarantee ECC accelerator operation is atomic, ECC accelerator open
* period must include:
* 1. Configure big-num parameters through internal_mpi_write_eccreg()
* 2. Trigger and wait
* 3. Read back result through internal_mpi_read_eccreg()
*/
internal_open_ecc_ac();
ecc_ac_open = true;

/* Configure ECC curve coefficients A/B */
/* Special case for A = -3 */
if (grp->A.p == NULL) {
Expand Down Expand Up @@ -632,10 +683,9 @@ NU_STATIC int internal_run_eccop(const mbedtls_ecp_group *grp,
crypto_ecc_prestart();
CRPT->ECC_CTL = (grp->pbits << CRPT_ECC_CTL_CURVEM_Pos) | eccop | CRPT_ECC_CTL_FSEL_Msk | CRPT_ECC_CTL_START_Msk;
ecc_done = crypto_ecc_wait();

/* FIXME: Better error code for ECC accelerator error */
MBEDTLS_MPI_CHK(ecc_done ? 0 : -1);


MBEDTLS_MPI_CHK(ecc_done ? 0 : MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED);

/* (X1, Y1) hold the normalized result. */
MBEDTLS_MPI_CHK(internal_mpi_read_eccreg(&R->X, (uint32_t *) CRPT->ECC_X1, NU_ECC_BIGNUM_MAXWORD));
MBEDTLS_MPI_CHK(internal_mpi_read_eccreg(&R->Y, (uint32_t *) CRPT->ECC_Y1, NU_ECC_BIGNUM_MAXWORD));
Expand All @@ -644,7 +694,17 @@ NU_STATIC int internal_run_eccop(const mbedtls_ecp_group *grp,
cleanup:

mbedtls_mpi_free(&N_);


/* Close ECC accelerator
*
* We must follow the rule that internal_open_ecc_ac()/internal_close_ecc_ac() must
* be paired and so don't allow superfluous internal_close_ecc_ac().
*/
if (ecc_ac_open) {
internal_close_ecc_ac();
ecc_ac_open = false;
}

return ret;
}

Expand Down Expand Up @@ -693,12 +753,17 @@ NU_STATIC int internal_run_modop(mbedtls_mpi *r,

int ret;
bool ecc_done;
bool ecc_ac_open = false;

mbedtls_mpi N_;
const mbedtls_mpi *Np;

mbedtls_mpi_init(&N_);


/* Open ECC accelerator (same as internal_run_eccop()) */
internal_open_ecc_ac();
ecc_ac_open = true;

/* Use INTERNAL_MPI_NORM(Np, N1, N_, P) to get normalized MPI
*
* N_: Holds normalized MPI if the passed-in MPI N1 is not
Expand Down Expand Up @@ -726,17 +791,22 @@ NU_STATIC int internal_run_modop(mbedtls_mpi *r,
crypto_ecc_prestart();
CRPT->ECC_CTL = (pbits << CRPT_ECC_CTL_CURVEM_Pos) | (ECCOP_MODULE | modop) | CRPT_ECC_CTL_FSEL_Msk | CRPT_ECC_CTL_START_Msk;
ecc_done = crypto_ecc_wait();

/* FIXME: Better error code for ECC accelerator error */
MBEDTLS_MPI_CHK(ecc_done ? 0 : -1);


MBEDTLS_MPI_CHK(ecc_done ? 0 : MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED);

/* X1 holds the result. */
MBEDTLS_MPI_CHK(internal_mpi_read_eccreg(r, (uint32_t *) CRPT->ECC_X1, NU_ECC_BIGNUM_MAXWORD));

cleanup:

mbedtls_mpi_free(&N_);

/* Close ECC accelerator (same as internal_run_eccop()) */
if (ecc_ac_open) {
internal_close_ecc_ac();
ecc_ac_open = false;
}

return ret;
}

Expand Down
6 changes: 5 additions & 1 deletion targets/TARGET_NUVOTON/TARGET_M480/crypto/crypto-misc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,11 @@ static bool crypto_submodule_acquire(uint16_t *submodule_avail)
static void crypto_submodule_release(uint16_t *submodule_avail)
{
uint16_t expectedCurrentValue = 0;
while (! core_util_atomic_cas_u16(submodule_avail, &expectedCurrentValue, 1));
while (! core_util_atomic_cas_u16(submodule_avail, &expectedCurrentValue, 1)) {
/* On failure, 'expectedCurrentValue' would be set to 'submodule_avail', so we
* need to re-initialize it. */
expectedCurrentValue = 0;
}
}

static void crypto_submodule_prestart(volatile uint16_t *submodule_done)
Expand Down
6 changes: 5 additions & 1 deletion targets/TARGET_NUVOTON/TARGET_NUC472/crypto/crypto-misc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,11 @@ static bool crypto_submodule_acquire(uint16_t *submodule_avail)
static void crypto_submodule_release(uint16_t *submodule_avail)
{
uint16_t expectedCurrentValue = 0;
while (! core_util_atomic_cas_u16(submodule_avail, &expectedCurrentValue, 1));
while (! core_util_atomic_cas_u16(submodule_avail, &expectedCurrentValue, 1)) {
/* On failure, 'expectedCurrentValue' would be set to 'submodule_avail', so we
* need to re-initialize it. */
expectedCurrentValue = 0;
}
}

static void crypto_submodule_prestart(volatile uint16_t *submodule_done)
Expand Down