Add a `queue_destroy` API. This new API destroys a queue, waking up all threads waiting to produce or consume, and making them fail to acquire the lock, before deallocating the underlying allocation. This is particularly useful when destroying a message queue from an error handler. This requires to add a destruction mode to the high bits lock class, which reduces the maximum size of the queue from 512MiB to 256MiB. This also requires to revise our approach to the `buffer` pointer in the queue handle. Since we restrict its bounds in `queue_create`, it cannot be used to free the queue. Revise this by restricting the bounds in `queue_make_receive_handle` and `queue_make_send_handle` instead. From an adversarial perspective, this is fine: anyone who has access to the unrestricted queue can arbitrarily write to `consumer` and `producer` anyways. Unfortunately it is a downgrade in terms of catching accidental overflows of the queue `buffer` when using the unrestricted queue. Signed-off-by: Hugo Lefeuvre <hugo.lefeuvre@scisemi.com>
diff --git a/sdk/include/queue.h b/sdk/include/queue.h index 162f91c..dc16e72 100644 --- a/sdk/include/queue.h +++ b/sdk/include/queue.h
@@ -77,6 +77,20 @@ size_t elementCount); /** + * Destroys a queue. This wakes up all threads waiting to produce or consume, + * and makes them fail to acquire the lock, before deallocating the underlying + * allocation. + * + * This must be called on an unrestricted queue handle (*not* one returned by + * `queue_make_receive_handle` or `queue_make_send_handle`). + * + * 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. *
diff --git a/sdk/lib/queue/queue.cc b/sdk/lib/queue/queue.cc index 1b0b965..5a1caf6 100644 --- a/sdk/lib/queue/queue.cc +++ b/sdk/lib/queue/queue.cc
@@ -236,8 +236,9 @@ /** * The bit to use for the lock. */ - static constexpr uint32_t LockBit = 1U << 31; - static constexpr uint32_t WaitersBit = 1U << 30; + static constexpr uint32_t LockBit = 1U << 31; + static constexpr uint32_t WaitersBit = 1U << 30; + static constexpr uint32_t LockedInDestructModeBit = 1U << 29; // Function required to conform to the Lock concept. void lock() @@ -247,7 +248,7 @@ static constexpr uint32_t reserved_bits() { - return LockBit | WaitersBit; + return LockBit | WaitersBit | LockedInDestructModeBit; } /** @@ -259,6 +260,10 @@ do { value = lockWord.load(); + if ((value & LockedInDestructModeBit) != 0) + { + return false; + } if (value & LockBit) { // If the lock is held, set the flag that indicates that @@ -296,6 +301,20 @@ lockWord.notify_all(); } } + + /** + * Set the lock in destruction mode. This has the same + * semantics as `flaglock_upgrade_for_destruction`. + */ + void upgrade_for_destruction() + { + // Atomically set the destruction bit. + lockWord |= LockedInDestructModeBit; + if (lockWord & WaitersBit) + { + lockWord.notify_all(); + } + } }; uint32_t counter_load(std::atomic<uint32_t> *counter) @@ -338,12 +357,35 @@ heap_claim_fast(&t, nullptr, nullptr); } + void bound_queue_buffer(struct QueueHandle handle) + { + Capability buffer = handle.buffer; + Capability producer = handle.producer; + // Restrict the bounds using the address of the producer, which + // comes immediately after the queue buffer. This should be + // strictly equivalent to calculating the size of the queue + // from the number of elements and the size of elements (we + // assert that below). + buffer.bounds() = producer.address() - buffer.address(); + Debug::Assert( + [&]() -> bool { + size_t bufferSize; + bool overflow = __builtin_mul_overflow( + handle.queueSize, handle.elementSize, &bufferSize); + bufferSize = CHERI::representable_length(bufferSize); + return (!overflow) && (buffer.bounds() == bufferSize); + }, + "Mismatch between the size of the queue as reported by `queueSize` " + "and `elementSize` and its real size."); + } + } // namespace struct QueueHandle queue_make_receive_handle(struct QueueHandle handle) { Capability buffer = handle.buffer; Capability producer = handle.producer; + bound_queue_buffer(handle); buffer.permissions() &= ReadOnlyCapability; producer.permissions() &= ReadOnly; handle.buffer = buffer; @@ -355,6 +397,7 @@ { Capability buffer = handle.buffer; Capability consumer = handle.consumer; + bound_queue_buffer(handle); buffer.permissions() &= WriteOnlyCapability; consumer.permissions() &= ReadOnly; handle.buffer = buffer; @@ -362,6 +405,36 @@ return handle; } +int queue_destroy(struct SObjStruct *heapCapability, struct QueueHandle *handle) +{ + int ret = 0; + // Only upgrade the locks for destruction if we know that we will be + // able to free the queue at the end. This will fail if passed a + // restricted buffer, which will happen if `queue_destroy` is called on + // a restricted queue. + if (ret = heap_can_free(heapCapability, handle->buffer); ret != 0) + { + return ret; + } + + auto *producer = handle->producer; + HighBitFlagLock producerLock{*producer}; + producerLock.upgrade_for_destruction(); + + auto *consumer = handle->consumer; + HighBitFlagLock consumerLock{*consumer}; + consumerLock.upgrade_for_destruction(); + + // This should not fail because of the `heap_can_free` check, unless we + // run out of stack. + if (ret = heap_free(heapCapability, handle->buffer); ret != 0) + { + return ret; + } + + return ret; +} + int queue_create(Timeout *timeout, struct SObjStruct *heapCapability, struct QueueHandle *outQueue, @@ -390,7 +463,7 @@ // We need the counters to be able to run to double the queue size without // hitting the high bits. Error if this is the case. // - // This should never be reached: a queue needs to be at least 512 MiB + // This should never be reached: a queue needs to be at least 256 MiB // (assuming one-byte elements) to hit this limit. if (((elementCount | (elementCount * 2)) & HighBitFlagLock::reserved_bits()) != 0) @@ -416,8 +489,7 @@ producer.bounds() = CounterSize; consumer.bounds() = CounterSize; // The pointer used to free the allocation - *outAllocation = buffer; - buffer.bounds() = bufferSize; + *outAllocation = buffer; Debug::log("Created queue with buffer: {}", buffer); // The handle *outQueue = {elementSize, elementCount, buffer, producer, consumer};
diff --git a/tests/queue-test.cc b/tests/queue-test.cc index f6e3aa6..6d49f28 100644 --- a/tests/queue-test.cc +++ b/tests/queue-test.cc
@@ -96,7 +96,7 @@ checkSpace(1); queue_receive(&timeout, &queue, bytes); checkSpace(0); - rv = heap_free(MALLOC_CAPABILITY, queueMemory); + rv = queue_destroy(MALLOC_CAPABILITY, &queue); TEST(rv == 0, "Queue deletion failed with {}", rv); debug_log("All queue library tests successful"); }