From 5baaac55ce2b9fb89c1f395e92f67abb5276ce6f Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Wed, 8 Jan 2020 09:42:44 -0600 Subject: [PATCH 1/5] vstr_init_len: Don't crash if (size_t)-1 is passed In this unusual case, (len + 1) is zero, the allocation in vstr_init succeeds (allocating 1 byte), and then the caller is likely to erroneously access outside the allocated region, for instance with a memset(). This could be triggered with os.urandom(-1) after it was converted to use mp_obj_new_bytes_of_zeros. --- py/vstr.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/py/vstr.c b/py/vstr.c index 869b278057..888f7069bb 100644 --- a/py/vstr.c +++ b/py/vstr.c @@ -50,6 +50,8 @@ void vstr_init(vstr_t *vstr, size_t alloc) { // Init the vstr so it allocs exactly enough ram to hold a null-terminated // string of the given length, and set the length. void vstr_init_len(vstr_t *vstr, size_t len) { + if(len == SIZE_MAX) + m_malloc_fail(len); vstr_init(vstr, len + 1); vstr->len = len; } From 6735283d8f6e352626d6ad7c4ee48b472ea65476 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Wed, 8 Jan 2020 09:43:13 -0600 Subject: [PATCH 2/5] os: Don't require an on-stack buffer This allows urandom requests of even 100k bytes to succeed on a fresh VM session on a Metro M4 express. --- shared-bindings/os/__init__.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/shared-bindings/os/__init__.c b/shared-bindings/os/__init__.c index 55ebd6f59f..c2b63bbcee 100644 --- a/shared-bindings/os/__init__.c +++ b/shared-bindings/os/__init__.c @@ -33,6 +33,7 @@ #include "lib/oofatfs/diskio.h" #include "py/mpstate.h" #include "py/obj.h" +#include "py/objstr.h" #include "py/runtime.h" #include "shared-bindings/os/__init__.h" @@ -195,11 +196,11 @@ MP_DEFINE_CONST_FUN_OBJ_0(os_sync_obj, os_sync); //| STATIC mp_obj_t os_urandom(mp_obj_t size_in) { mp_int_t size = mp_obj_get_int(size_in); - uint8_t tmp[size]; - if (!common_hal_os_urandom(tmp, size)) { + mp_obj_str_t *result = MP_OBJ_TO_PTR(mp_obj_new_bytes_of_zeros(size)); + if (!common_hal_os_urandom((uint8_t*) result->data, size)) { mp_raise_NotImplementedError(translate("No hardware random available")); } - return mp_obj_new_bytes(tmp, size); + return result; } MP_DEFINE_CONST_FUN_OBJ_1(os_urandom_obj, os_urandom); From b3fb0243011a2797ef41c3014c7fcdaca4331765 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Wed, 8 Jan 2020 10:06:55 -0600 Subject: [PATCH 3/5] nrf: Call into sd as many times as necessary to fill urandom request Generating 51200 bytes in one go takes 4.966s, so that's a rate of about 10KiB/s. --- ports/nrf/common-hal/os/__init__.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/ports/nrf/common-hal/os/__init__.c b/ports/nrf/common-hal/os/__init__.c index e38226f94e..1d89c73b62 100644 --- a/ports/nrf/common-hal/os/__init__.c +++ b/ports/nrf/common-hal/os/__init__.c @@ -31,6 +31,7 @@ #ifdef BLUETOOTH_SD #include "nrf_sdm.h" +#include "tick.h" #endif #include "nrf_rng.h" @@ -66,8 +67,25 @@ bool common_hal_os_urandom(uint8_t *buffer, uint32_t length) { uint8_t sd_en = 0; (void) sd_softdevice_is_enabled(&sd_en); - if (sd_en) - return NRF_SUCCESS == sd_rand_application_vector_get(buffer, length); + if (sd_en) { + while (length != 0) { + uint8_t available = 0; + sd_rand_application_bytes_available_get(&available); + if (available) { + uint32_t request = MIN(length, available); + uint32_t result = sd_rand_application_vector_get(buffer, request); + if (result != NRF_SUCCESS) { + return false; + } + buffer += request; + length -= request; + } else { + RUN_BACKGROUND_TASKS; + tick_delay(500); + } + } + return true; + } #endif nrf_rng_event_clear(NRF_RNG, NRF_RNG_EVENT_VALRDY); From f1c2dee1c05227d63e3833cb10d6fe99b9bd5092 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Wed, 8 Jan 2020 16:36:43 -0600 Subject: [PATCH 4/5] style --- py/vstr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/py/vstr.c b/py/vstr.c index 888f7069bb..91cd7f584f 100644 --- a/py/vstr.c +++ b/py/vstr.c @@ -50,8 +50,9 @@ void vstr_init(vstr_t *vstr, size_t alloc) { // Init the vstr so it allocs exactly enough ram to hold a null-terminated // string of the given length, and set the length. void vstr_init_len(vstr_t *vstr, size_t len) { - if(len == SIZE_MAX) + if(len == SIZE_MAX) { m_malloc_fail(len); + } vstr_init(vstr, len + 1); vstr->len = len; } From 1c6efb9e662fcfb59afa9cac6c562082248a7560 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Wed, 8 Jan 2020 16:48:17 -0600 Subject: [PATCH 5/5] os.urandom: remove unneeded sleep --- ports/nrf/common-hal/os/__init__.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/ports/nrf/common-hal/os/__init__.c b/ports/nrf/common-hal/os/__init__.c index 1d89c73b62..b2ad00a5ca 100644 --- a/ports/nrf/common-hal/os/__init__.c +++ b/ports/nrf/common-hal/os/__init__.c @@ -31,7 +31,6 @@ #ifdef BLUETOOTH_SD #include "nrf_sdm.h" -#include "tick.h" #endif #include "nrf_rng.h" @@ -81,7 +80,6 @@ bool common_hal_os_urandom(uint8_t *buffer, uint32_t length) { length -= request; } else { RUN_BACKGROUND_TASKS; - tick_delay(500); } } return true;