Skip to content

Commit

Permalink
Simplify message queues.
Browse files Browse the repository at this point in the history 
My original implementation was a bit CHERI-happy and put a lot of
capabilities in each endpoint to enforce properties with a
not-very-well-thought-through thread model.

The new code is simpler in a few ways:

 - The queue metadata and ring buffer are a single allocation.
 - The producer / consumer counters are indexed directly, not via
   indirection.
 - We don't do permission checks and claims for defensive programming in
   the send and receive functions, we use the unwind error handling so
   don't need to enumerate possible badness.
 - The library returns a pointer to the allocation directly, rather than
   a (large) value type and a pointer that is used for freeing.
 - Restricted endpoints are just a simple indirection layer and are only
   in the sealed version where they might make sense with a threat
   model.
 - The FreeRTOS compatibility APIs no longer need a heap allocation to
   store a pointer to another heap allocation.

This is an API break, but the new APIs are simpler so it's probably
worth it, and good to do before 1.0.  I believe the only users are using
the FreeRTOS-Compat wrappers, which are not changed (though can now
easily be extended to support message queues between compartments in the
compat layer).

Fixes #309
  • Loading branch information
@davidchisnall
davidchisnall committed Oct 26, 2024
1 parent ce5ad17 commit c3c0fb1
Show file tree
Hide file tree
Showing 10 changed files with 456 additions and 484 deletions.
9 changes: 4 additions & 5 deletions examples/06.producer-consumer/producer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,16 @@ using Debug = ConditionalDebug<true, "Producer">;
void __cheri_compartment("producer") run()
{
// Allocate the queue
SObj sendHandle;
SObj receiveHandle;
SObj queue;
non_blocking<queue_create_sealed>(
MALLOC_CAPABILITY, &sendHandle, &receiveHandle, sizeof(int), 16);
MALLOC_CAPABILITY, &queue, sizeof(int), 16);
// Pass the queue handle to the consumer.
set_queue(receiveHandle);
set_queue(queue);
Debug::log("Starting producer loop");
// Loop, sending some numbers to the other thread.
for (int i = 1; i < 200; i++)
{
int ret = blocking_forever<queue_send_sealed>(sendHandle, &i);
int ret = blocking_forever<queue_send_sealed>(queue, &i);
// Abort if the queue send errors.
Debug::Invariant(ret == 0, "Queue send failed {}", ret);
}
Expand Down
36 changes: 7 additions & 29 deletions sdk/include/FreeRTOS-Compat/queue.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,7 @@
/**
* Queue handle. This is used to reference queues in the API functions.
*/
typedef struct
{
/**
* The real handle, holds pointers to the relevant elements of the queue.
*/
struct QueueHandle handle;
/**
* The pointer used to free the queue.
*/
void *freePointer;
} * QueueHandle_t;
typedef struct MessageQueue *QueueHandle_t;

/**
* Receive a message on a queue. The message is received into `buffer`, which
Expand All @@ -34,7 +24,7 @@ static inline BaseType_t
xQueueReceive(QueueHandle_t queueHandle, void *buffer, TickType_t waitTicks)
{
struct Timeout timeout = {0, waitTicks};
int rv = queue_receive(&timeout, &queueHandle->handle, buffer);
int rv = queue_receive(&timeout, queueHandle, buffer);

if (rv == 0)
return pdPASS;
Expand All @@ -54,7 +44,7 @@ static inline BaseType_t xQueueSendToBack(QueueHandle_t queueHandle,
TickType_t waitTicks)
{
struct Timeout timeout = {0, waitTicks};
int rv = queue_send(&timeout, &queueHandle->handle, buffer);
int rv = queue_send(&timeout, queueHandle, buffer);

if (rv == 0)
return pdPASS;
Expand Down Expand Up @@ -86,24 +76,13 @@ xQueueSendToBackFromISR(QueueHandle_t queueHandle,
static inline QueueHandle_t xQueueCreate(UBaseType_t uxQueueLength,
UBaseType_t uxItemSize)
{
QueueHandle_t ret;
QueueHandle_t ret = NULL;
struct Timeout timeout = {0, UnlimitedTimeout};
ret = (QueueHandle_t)malloc(sizeof(*ret));
if (!ret)
{
return NULL;
}
int rc = queue_create(&timeout,
MALLOC_CAPABILITY,
&ret->handle,
&ret->freePointer,
&ret,
uxItemSize,
uxQueueLength);
if (rc)
{
free(ret);
return NULL;
}
return ret;
}
#endif
Expand All @@ -113,8 +92,7 @@ static inline QueueHandle_t xQueueCreate(UBaseType_t uxQueueLength,
*/
static inline void vQueueDelete(QueueHandle_t xQueue)
{
free(xQueue->freePointer);
free(xQueue);
queue_destroy(MALLOC_CAPABILITY, xQueue);
}

/**
Expand All @@ -126,7 +104,7 @@ static inline void vQueueDelete(QueueHandle_t xQueue)
static inline UBaseType_t uxQueueMessagesWaiting(const QueueHandle_t xQueue)
{
size_t ret;
int rv = queue_items_remaining(&xQueue->handle, &ret);
int rv = queue_items_remaining(xQueue, &ret);

assert(rv == 0);

Expand Down
149 changes: 89 additions & 60 deletions sdk/include/queue.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,50 +31,71 @@
#include <timeout.h>

/**
* A handle to a queue endpoint.
* Structure representing a queue. This structure represents the queue
* metadata, the buffer is stored at the end.
*
* Dropping permissions can make this a receive-only or a send-only handle.
* A queue is a ring buffer of fixed-sized elements with a producer and consumer
* counter.
*/
struct QueueHandle
struct MessageQueue
{
/**
* The size of one element in this queue.
* The size of one element in this queue. This should not be modified after
* construction.
*/
size_t elementSize;
/**
* The size of the queue.
* The size of the queue. This should not be modified after construction.
*/
size_t queueSize;
/**
* The buffer used for storing queue elements.
*/
void *buffer;
/**
* The producer counter.
*/
_Atomic(uint32_t) *producer;
_Atomic(uint32_t) producer;
/**
* The consumer counter.
*/
_Atomic(uint32_t) *consumer;
_Atomic(uint32_t) consumer;
#ifdef __cplusplus
MessageQueue(size_t elementSize, size_t queueSize)
: elementSize(elementSize), queueSize(queueSize)
{
}
#endif
};

_Static_assert(sizeof(struct MessageQueue) % sizeof(void *) == 0,
"MessageQueue structure must end correctly aligned for storing "
"capabilities.");

__BEGIN_DECLS

/**
* Returns the allocation size needed for a queue with the specified number and
* size of elements. This can be used to statically allocate queues.
*
* Returns the allocation size on success, or `-EINVAL` if the arguments would
* cause an overflow.
*/
ssize_t __cheri_libcall queue_allocation_size(size_t elementSize,
size_t elementCount);

/**
* Allocates space for a queue using `heapCapability` and stores a handle to it
* via `outQueue`. The underlying allocation (which is necessary to free the
* queue) is returned via `outAllocation`.
* via `outQueue`.
*
* The queue is has space for `elementCount` entries. Each entry is a fixed
* size, `elementSize` bytes.
*
* Returns 0 on success, `-ENOMEM` on allocation failure, and `-EINVAL` if the
* arguments are invalid (for example, if the requested number of elements
* multiplied by the element size would overflow).
*/
int __cheri_libcall queue_create(Timeout *timeout,
struct SObjStruct *heapCapability,
struct QueueHandle *outQueue,
void **outAllocation,
size_t elementSize,
size_t elementCount);
int __cheri_libcall queue_create(Timeout *timeout,
struct SObjStruct *heapCapability,
struct MessageQueue **outQueue,
size_t elementSize,
size_t elementCount);

/**
* Destroys a queue. This wakes up all threads waiting to produce or consume,
Expand All @@ -87,29 +108,8 @@ int __cheri_libcall queue_create(Timeout *timeout,
* Returns 0 on success. On failure, returns `-EPERM` if the queue handle is
* restricted (see comment above).
*/
int __cheri_libcall queue_destroy(struct SObjStruct *heapCapability,
struct QueueHandle *handle);

/**
* Convert a queue handle returned from `queue_create` into one that can be
* used *only* for receiving.
*
* Note: This is primarily defence in depth. A malicious holder of this queue
* handle can still set the consumer counter to invalid values.
*/
struct QueueHandle __cheri_libcall
queue_make_receive_handle(struct QueueHandle handle);

/**
* Convert a queue handle returned from `queue_create` into one that can be
* used *only* for sending.
*
* Note: This is primarily defence in depth. A malicious holder of this queue
* handle can still set the producer counter to invalid values and overwrite
* arbitrary queue locations.
*/
struct QueueHandle __cheri_libcall
queue_make_send_handle(struct QueueHandle handle);
int __cheri_libcall queue_destroy(struct SObjStruct *heapCapability,
struct MessageQueue *handle);

/**
* Send a message to the queue specified by `handle`. This expects to be able
Expand All @@ -122,9 +122,9 @@ queue_make_send_handle(struct QueueHandle handle);
* This expected to be called with a valid queue handle. It does not validate
* that this is correct. It uses `safe_memcpy` and so will check the buffer.
*/
int __cheri_libcall queue_send(Timeout *timeout,
struct QueueHandle *handle,
const void *src);
int __cheri_libcall queue_send(Timeout *timeout,
struct MessageQueue *handle,
const void *src);

/**
* Receive a message over a queue specified by `handle`. This expects to be
Expand All @@ -135,9 +135,9 @@ int __cheri_libcall queue_send(Timeout *timeout,
* Returns 0 on success, `-ETIMEOUT` if the timeout was exhausted, `-EINVAL` on
* invalid arguments.
*/
int __cheri_libcall queue_receive(Timeout *timeout,
struct QueueHandle *handle,
void *dst);
int __cheri_libcall queue_receive(Timeout *timeout,
struct MessageQueue *handle,
void *dst);

/**
* Returns the number of items in the queue specified by `handle` via `items`.
Expand All @@ -150,27 +150,28 @@ int __cheri_libcall queue_receive(Timeout *timeout,
* may change in between the return of this function and the caller acting on
* the result.
*/
int __cheri_libcall queue_items_remaining(struct QueueHandle *handle,
size_t *items);
int __cheri_libcall queue_items_remaining(struct MessageQueue *handle,
size_t *items);

/**
* Allocate a new message queue that is managed by the message queue
* compartment. This is returned as two sealed pointers to send and receive
* ends of the queue.
* compartment. The resulting queue handle (returned in `outQueue`) is a
* sealed capability to a queue that can be used for both sending and
* receiving.
*/
int __cheri_compartment("message_queue")
queue_create_sealed(Timeout *timeout,
struct SObjStruct *heapCapability,
struct SObjStruct **outQueueSend,
struct SObjStruct **outQueReceive,
struct SObjStruct **outQueue,
size_t elementSize,
size_t elementCount);

/**
* Destroy a queue using a sealed queue endpoint handle. The queue is not
* actually freed until *both* endpoints are destroyed, which means that you
* can safely call this from the sending end without the receiving end losing
* access to messages held in the queue.
* Destroy a queue handle. If this is called on a restricted endpoint
* (returned from `queue_receive_handle_create_sealed` or
* `queue_send_handle_create_sealed`), this frees only the handle. If called
* with the queue handle returned from `queue_create_sealed`, this will destroy
* the queue.
*/
int __cheri_compartment("message_queue")
queue_destroy_sealed(Timeout *timeout,
Expand Down Expand Up @@ -215,7 +216,7 @@ int __cheri_compartment("message_queue")
*/
void __cheri_libcall
multiwaiter_queue_receive_init(struct EventWaiterSource *source,
struct QueueHandle *handle);
struct MessageQueue *handle);

/**
* Initialise an event waiter source so that it will wait for the queue to be
Expand All @@ -224,7 +225,7 @@ multiwaiter_queue_receive_init(struct EventWaiterSource *source,
*/
void __cheri_libcall
multiwaiter_queue_send_init(struct EventWaiterSource *source,
struct QueueHandle *handle);
struct MessageQueue *handle);

/**
* Initialise an event waiter source as in `multiwaiter_queue_receive_init`,
Expand All @@ -250,4 +251,32 @@ int __cheri_compartment("message_queue")
multiwaiter_queue_send_init_sealed(struct EventWaiterSource *source,
struct SObjStruct *handle);

/**
* Convert a queue handle returned from `queue_create_sealed` into one that can
* be used *only* for receiving.
*
* Returns 0 on success and writes the resulting restricted handle via
* `outHandle`. Returns `-ENOMEM` on allocation failure or `-EINVAL` if the
* handle is not valid.
*/
int __cheri_compartment("message_queue")
queue_receive_handle_create_sealed(struct Timeout *timeout,
struct SObjStruct *heapCapability,
struct SObjStruct *handle,
struct SObjStruct **outHandle);

/**
* Convert a queue handle returned from `queue_create_sealed` into one that can
* be used *only* for sending.
*
* Returns 0 on success and writes the resulting restricted handle via
* `outHandle`. Returns `-ENOMEM` on allocation failure or `-EINVAL` if the
* handle is not valid.
*/
int __cheri_compartment("message_queue")
queue_send_handle_create_sealed(struct Timeout *timeout,
struct SObjStruct *heapCapability,
struct SObjStruct *handle,
struct SObjStruct **outHandle);

__END_DECLS
12 changes: 12 additions & 0 deletions sdk/lib/queue/README.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
Message queues
==============

Message queues, as described in [`queue.h`](../../include/queue.h).

This directory provides two targets.

- The message queue library (`message_queue_library`) provides APIs for message queues that can be shared between two threads in the same compartment.
- The message queue compartment (`message_queue`) wraps these in APIs that can be used from different compartments.

The library uses the `setjmp`-based error handler (see: [`unwind.h`](../../include/unwind.h)) to recover from invalid bounds or permissions.
If you are using the library and want to be robust in the presence of CHERI exceptions, you should either add `unwind_error_handler` as a dependency of your compartment or provide an error handler that calls `cleanup_unwind`.
Loading

0 comments on commit c3c0fb1

Please sign in to comment.