Skip to content

Commit

Permalink
random qemu: avoid use of spinlocks as they clash with idf internals
Browse files Browse the repository at this point in the history
Use freertos semaphore mutexes instead, see:
espressif/esp-idf#12381 (comment)
  • Loading branch information
greenaddress authored and JamieDriver committed Oct 13, 2023
1 parent 0404302 commit 8bd6c85
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 15 deletions.
23 changes: 13 additions & 10 deletions main/qemu/qemu_tcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ static esp_eth_phy_t* s_phy = NULL;
static void* s_eth_glue = NULL;
static const char* TAG = "jade";

static portMUX_TYPE sockmutex;
static SemaphoreHandle_t sockmutex = NULL;
static int qemu_tcp_sock = 0;
static int qemu_tcp_listen_sock = 0;

Expand All @@ -43,6 +43,7 @@ esp_event_handler_instance_t ctx_got_ip;

static void qemu_tcp_reader(void* ignore)
{
JADE_ASSERT(sockmutex);
struct sockaddr_in dest_addr;
struct sockaddr_in* dest_addr_ip4 = (struct sockaddr_in*)&dest_addr;
dest_addr_ip4->sin_addr.s_addr = htonl(INADDR_ANY);
Expand All @@ -65,18 +66,18 @@ static void qemu_tcp_reader(void* ignore)
TickType_t last_processing_time = 0;

while (1) {
portENTER_CRITICAL(&sockmutex);
JADE_SEMAPHORE_TAKE(sockmutex);
// if we are connected we reuse it
int tmp_qemu_tcp_sock = qemu_tcp_sock;
portEXIT_CRITICAL(&sockmutex);
JADE_SEMAPHORE_GIVE(sockmutex);

if (tmp_qemu_tcp_sock == 0) {
// otherwise we wait for a new connection (note: this server supports only one client at the time)
tmp_qemu_tcp_sock = accept(qemu_tcp_listen_sock, (struct sockaddr*)&source_addr, &addr_len);
JADE_ASSERT(tmp_qemu_tcp_sock > 0);
portENTER_CRITICAL(&sockmutex);
JADE_SEMAPHORE_TAKE(sockmutex);
qemu_tcp_sock = tmp_qemu_tcp_sock;
portEXIT_CRITICAL(&sockmutex);
JADE_SEMAPHORE_GIVE(sockmutex);
}

// Read incoming data max to fill buffer
Expand All @@ -85,9 +86,9 @@ static void qemu_tcp_reader(void* ignore)
if (len <= 0) {
// Close socket, pause and retry... will be reopened by above next loop
JADE_LOGE("Error reading bytes from tcp stream device: %u", len);
portENTER_CRITICAL(&sockmutex);
JADE_SEMAPHORE_TAKE(sockmutex);
qemu_tcp_sock = 0;
portEXIT_CRITICAL(&sockmutex);
JADE_SEMAPHORE_GIVE(sockmutex);
shutdown(tmp_qemu_tcp_sock, 0);
close(tmp_qemu_tcp_sock);
read = 0;
Expand All @@ -108,9 +109,9 @@ static bool write_qemu_tcp(const uint8_t* msg, const size_t length, void* ignore
JADE_ASSERT(msg);
JADE_ASSERT(length);

portENTER_CRITICAL(&sockmutex);
JADE_SEMAPHORE_TAKE(sockmutex);
const int tmp_qemu_tcp_sock = qemu_tcp_sock;
portEXIT_CRITICAL(&sockmutex);
JADE_SEMAPHORE_GIVE(sockmutex);
if (tmp_qemu_tcp_sock == 0) {
return false;
}
Expand Down Expand Up @@ -230,7 +231,9 @@ bool qemu_tcp_init(TaskHandle_t* qemu_tcp_handle)
JADE_ASSERT(!full_qemu_tcp_data_in);
JADE_ASSERT(!qemu_tcp_data_out);

spinlock_initialize(&sockmutex);
JADE_ASSERT(!sockmutex);
sockmutex = xSemaphoreCreateMutex();
JADE_ASSERT(sockmutex);

ESP_ERROR_CHECK(esp_netif_init());

Expand Down
14 changes: 9 additions & 5 deletions main/random.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <esp_timer.h>

#include <freertos/FreeRTOS.h>
#include <freertos/semphr.h>
#include <freertos/task.h>
#include <mbedtls/sha512.h>
#include <string.h>
Expand Down Expand Up @@ -43,8 +44,8 @@
} while (false)

static uint8_t entropy_state[SHA256_LEN];
static uint32_t rnd_counter;
static portMUX_TYPE rndmutex;
static uint32_t rnd_counter = 0;
static SemaphoreHandle_t rnd_mutex = NULL;

static uint16_t esp32_get_temperature(void)
{
Expand All @@ -64,6 +65,7 @@ static uint16_t esp32_get_temperature(void)
// returns up to 32 bytes of randomness (optional), takes optionallly extra entropy
static void get_random_internal(uint8_t* bytes_out, const size_t len, const uint8_t* additional, const size_t addlen)
{
JADE_ASSERT(rnd_mutex);
JADE_ASSERT(len <= SHA256_LEN);
JADE_ASSERT((bytes_out && len) || (!bytes_out && !len));
JADE_ASSERT((additional && addlen) || (!additional && !addlen));
Expand Down Expand Up @@ -91,7 +93,7 @@ static void get_random_internal(uint8_t* bytes_out, const size_t len, const uint
add_bytes_to_hasher(ctx, additional, addlen);
}

portENTER_CRITICAL(&rndmutex);
JADE_SEMAPHORE_TAKE(rnd_mutex);

add_bytes_to_hasher(ctx, entropy_state, sizeof(entropy_state));
add_bytes_to_hasher(ctx, &rnd_counter, sizeof(rnd_counter));
Expand All @@ -117,7 +119,7 @@ static void get_random_internal(uint8_t* bytes_out, const size_t len, const uint
// Store the last 32 bytes of the hash output as new RNG state.
memcpy(entropy_state, buf + SHA256_LEN, SHA256_LEN);

portEXIT_CRITICAL(&rndmutex);
JADE_SEMAPHORE_GIVE(rnd_mutex);
mbedtls_sha512_free(&ctx);

// Since refeeding can be called from any task (including internal rtos tasks),
Expand Down Expand Up @@ -259,7 +261,9 @@ void random_start_collecting(void)
esp_fill_random(entropy_state, sizeof(entropy_state));
bootloader_random_disable();

spinlock_initialize(&rndmutex);
JADE_ASSERT(!rnd_mutex);
rnd_mutex = xSemaphoreCreateMutex();
JADE_ASSERT(rnd_mutex);
}

void random_full_initialization(void)
Expand Down

0 comments on commit 8bd6c85

Please sign in to comment.