libjade: implement the rtos binary semaphore type
What the developer wrote
Authored by Daniel Newton
libjade: implement the rtos binary semaphore type
What changed, and why it matters
This commit replaces a placeholder implementation of a binary semaphore with a real one. Previously, creating a binary semaphore actually created a mutex, which could cause incorrect synchronization behavior in the simulated FreeRTOS environment used for testing. The change makes the test/development environment behave more correctly, but it is not a fix for a remotely exploitable vulnerability in the actual Jade hardware wallet firmware.
No immediate security action required. Treat as a normal correctness/maintenance commit. If this change was made because a test or fuzzer exposed a real bug caused by the stub, review related test results for any remaining synchronization issues in the simulation layer.
Security signals we found
Replaces stubbed synchronization primitive with correct implementation
Removes FIXME comments indicating prior implementation was incorrect
Adds type discriminator to prevent destroying a semaphore as a mutex or vice versa
Evidence from the diff
The patch updates libjade/include/freertos/semphr.h to implement a true POSIX unnamed semaphore (sem_t) for FreeRTOS binary semaphores in the POSIX-hosted simulation layer. It adds a SemaphoreType_t discriminator and a union holding either sem_t or pthread_mutex_t, updates xSemaphoreCreateBinary() to call sem_init(), vSemaphoreDelete() to call sem_destroy(), xSemaphoreTake() to use sem_wait()/sem_timedwait(), and xSemaphoreGive() to use sem_post(). The previous code created a mutex when a binary semaphore was requested, which is functionally wrong because a mutex is ownership-based and recursive/unlock rules differ from a counting/signal semaphore. This is a correctness improvement in the host simulation shim, not a security patch for the embedded firmware.
Changed components
libjade/include/freertos/semphr.hPOSIX-hosted FreeRTOS simulation layerInspect captured patch +101 / −20
diff --git a/libjade/include/freertos/semphr.h b/libjade/include/freertos/semphr.h
index 6d08bd4..d12a80a 100644
--- a/libjade/include/freertos/semphr.h
+++ b/libjade/include/freertos/semphr.h
@@ -1,54 +1,135 @@
#ifndef _LIBJADE_FREERTOS_SEMPHR_H_
#define _LIBJADE_FREERTOS_SEMPHR_H_ 1
+#include <errno.h>
#include <freertos/projdefs.h>
+#include <freertos/timecvt.h>
#include <pthread.h>
+#include <semaphore.h>
#include <time.h>
+typedef enum {
+ SEMAPHORE_TYPE_BINARY,
+ SEMAPHORE_TYPE_MUTEX,
+} SemaphoreType_t;
+
typedef struct Semaphore {
- pthread_mutex_t mutex;
+ SemaphoreType_t type;
+ union {
+ sem_t binary;
+ pthread_mutex_t mutex;
+ };
}* SemaphoreHandle_t;
static inline SemaphoreHandle_t xSemaphoreCreateMutex(void)
{
SemaphoreHandle_t out = malloc(sizeof(struct Semaphore));
- if (!out) {
- abort();
- }
- int ret = pthread_mutex_init(&out->mutex, NULL);
- if (ret) {
- abort();
+ if (out) {
+ out->type = SEMAPHORE_TYPE_MUTEX;
+ const int ret = pthread_mutex_init(&out->mutex, NULL);
+ if (ret) {
+ free(out);
+ out = NULL;
+ }
}
return out;
}
static inline SemaphoreHandle_t xSemaphoreCreateBinary(void)
{
- // FIXME: Create an actual signal-able semaphore, this is just
- // a stub.
- return xSemaphoreCreateMutex();
+ SemaphoreHandle_t out = malloc(sizeof(struct Semaphore));
+ if (out) {
+ out->type = SEMAPHORE_TYPE_BINARY;
+ const int ret = sem_init(&out->binary, 0, 0);
+ if (ret) {
+ free(out);
+ out = NULL;
+ }
+ }
+ return out;
}
static inline void vSemaphoreDelete(SemaphoreHandle_t s)
{
- int ret = pthread_mutex_destroy(&s->mutex);
- if (ret) {
- abort();
+ switch (s->type) {
+ case SEMAPHORE_TYPE_BINARY:
+ if (sem_destroy(&s->binary)) {
+ abort();
+ }
+ break;
+ case SEMAPHORE_TYPE_MUTEX:
+ if (pthread_mutex_destroy(&s->mutex)) {
+ abort();
+ }
+ break;
}
free(s);
}
-int xSemaphoreTake(SemaphoreHandle_t s, int timeout)
+static inline int xSemaphoreTake(SemaphoreHandle_t s, int timeout)
{
- // FIXME: timeout
- return pthread_mutex_lock(&s->mutex) ? pdFALSE : pdTRUE;
+ struct timespec ts;
+ int ret;
+ switch (s->type) {
+ case SEMAPHORE_TYPE_BINARY:
+ if (timeout == portMAX_DELAY) {
+ if (sem_wait(&s->binary)) {
+ JADE_LOGE("Unknown error waiting for semaphore (sem_wait): %d", errno);
+ abort();
+ }
+ return pdTRUE;
+ }
+ ts = absolute_timespec_from_ticktype(timeout);
+ if (sem_timedwait(&s->binary, &ts)) {
+ switch (errno) {
+ case 0:
+ break;
+ case ETIMEDOUT:
+ return pdFALSE;
+ default:
+ JADE_LOGE("Unknown error aquiring mutex (sem_timedwait): %d", errno);
+ abort();
+ }
+ }
+ break;
+ case SEMAPHORE_TYPE_MUTEX:
+ if (timeout == portMAX_DELAY) {
+ ret = pthread_mutex_lock(&s->mutex);
+ if (ret) {
+ JADE_LOGE("Unknown error aquiring mutex (pthread_mutex_lock): %d", ret);
+ abort();
+ }
+ return pdTRUE;
+ }
+ ts = absolute_timespec_from_ticktype(timeout);
+ ret = pthread_mutex_timedlock(&s->mutex, &ts);
+ switch (ret) {
+ case 0:
+ break;
+ case ETIMEDOUT:
+ return pdFALSE;
+ default:
+ JADE_LOGE("Unknown error aquiring mutex (pthread_mutex_timedlock): %d", ret);
+ abort();
+ }
+ break;
+ }
+ return pdTRUE;
}
-void xSemaphoreGive(SemaphoreHandle_t s)
+static inline void xSemaphoreGive(SemaphoreHandle_t s)
{
- int ret = pthread_mutex_unlock(&s->mutex);
- if (ret) {
- abort();
+ switch (s->type) {
+ case SEMAPHORE_TYPE_BINARY:
+ if (sem_post(&s->binary)) {
+ abort();
+ }
+ break;
+ case SEMAPHORE_TYPE_MUTEX:
+ if (pthread_mutex_unlock(&s->mutex)) {
+ abort();
+ }
+ break;
}
}
Why this scored 12/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.