Skip to content

STM32: TRNG: remove call to deprecated HAL_RNG_GetRandomNumber #5179

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

Merged
merged 2 commits into from
Oct 23, 2017
Merged

STM32: TRNG: remove call to deprecated HAL_RNG_GetRandomNumber #5179

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14343c4
STM32: TRNG: remove call to deprecated HAL_RNG_GetRandomNumber
LMESTM Sep 21, 2017
849749f
STM32: RNG: Ensure that no more than 1 instance is used
LMESTM Sep 21, 2017
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
46 changes: 29 additions & 17 deletions targets/TARGET_STM/trng_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,20 @@
#include <stdlib.h>
#include "cmsis.h"
#include "trng_api.h"
#include "mbed_error.h"
#include "mbed_critical.h"

/** trng_get_byte
* @brief Get one byte of entropy from the RNG, assuming it is up and running.
* @param obj TRNG obj
* @param pointer to the hardware generated random byte.
*/
static void trng_get_byte(trng_t *obj, unsigned char *byte )
{
*byte = (unsigned char)HAL_RNG_GetRandomNumber(&obj->handle);
}
static uint8_t users = 0;

void trng_init(trng_t *obj)
{
uint32_t dummy;

/* We're only supporting a single user of RNG */
if (core_util_atomic_incr_u8(&users, 1) > 1 ) {
error("Only 1 RNG instance supported\r\n");
}

#if defined(TARGET_STM32L4)
RCC_PeriphCLKInitTypeDef PeriphClkInitStruct;

Expand All @@ -50,11 +51,13 @@ void trng_init(trng_t *obj)

/* Initialize RNG instance */
obj->handle.Instance = RNG;
obj->handle.State = HAL_RNG_STATE_RESET;
obj->handle.Lock = HAL_UNLOCKED;

HAL_RNG_Init(&obj->handle);

/* first random number generated after setting the RNGEN bit should not be used */
HAL_RNG_GetRandomNumber(&obj->handle);

HAL_RNG_GenerateRandomNumber(&obj->handle, &dummy);
}

void trng_free(trng_t *obj)
Expand All @@ -63,23 +66,32 @@ void trng_free(trng_t *obj)
HAL_RNG_DeInit(&obj->handle);
/* RNG Peripheral clock disable - assume we're the only users of RNG */
__HAL_RCC_RNG_CLK_DISABLE();

users = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 36 - uses atomic access, not here when you are setting it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the free case, I don't have to test the value, I just have to set it to unused, so I preferred to keep it simple: as there is only 1 user, once freed, it will always be 0.

An alternative is to throw an error in case of unbalanced, (like in sleep manager):

core_util_critical_section_enter();
if (users == 0) {
    core_util_critical_section_exit();
    error("Unbalanced number of trng free (< 0)");
}
core_util_atomic_decr_u16(&users, 1);
core_util_critical_section_exit();

but that seemed overkill to me ...
I can do the change anyway if people think it's of added value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, can be as it is.

Why init can be called only once - one owner limitation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes - one HW only and only one client in MBED in features/mbedtls/platform/src/mbed_trng.c, so I'll stick to it for now.

}

int trng_get_bytes(trng_t *obj, uint8_t *output, size_t length, size_t *output_length)
{
int ret;
int ret = 0;
volatile uint8_t random[4];
*output_length = 0;

/* Get Random byte */
for( uint32_t i = 0; i < length; i++ ){
trng_get_byte(obj, output + i );
while ((*output_length < length) && (ret ==0)) {
if ( HAL_RNG_GenerateRandomNumber(&obj->handle, (uint32_t *)random ) != HAL_OK) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if we call HAL_RNG_GenerateRandomNumber() without calling trng_init() first?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HAL would return an error in such case.

ret = -1;
} else {
for (uint8_t i =0; (i < 4) && (*output_length < length) ; i++) {
*output++ = random[i];
*output_length += 1;
random[i] = 0;
}
}
}

*output_length = length;
/* Just be extra sure that we didn't do it wrong */
if( ( __HAL_RNG_GET_FLAG(&obj->handle, (RNG_FLAG_CECS | RNG_FLAG_SECS)) ) != 0 ) {
ret = -1;
} else {
ret = 0;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that I am being paranoid, but could we please wipe the random[] buffer before returning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure - will do so and post an update

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I made an update and now random is erased immediately after use.

return( ret );
Expand Down