Add stack usage checks for the scheduler. It should now be impossible to trick the core compartments of the RTOS into exhausting the stack. It's probably still possible though, because the test suite almost certainly doesn't have sufficient coverage, but this is a start and now we can get failures in other things if we test them with the right build options.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 736ca36..1a024d9 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml
@@ -17,7 +17,7 @@ - build-type: debug build-flags: --debug-loader=y --debug-scheduler=y --debug-allocator=y -m debug - build-type: release - build-flags: --debug-loader=n --debug-scheduler=n --debug-allocator=n -m release --stack-usage-check-allocator=y + build-flags: --debug-loader=n --debug-scheduler=n --debug-allocator=n -m release --stack-usage-check-allocator=y --stack-usage-check-scheduler=y fail-fast: false runs-on: ubuntu-latest container:
diff --git a/sdk/core/scheduler/common.h b/sdk/core/scheduler/common.h index 503809f..a0744ed 100644 --- a/sdk/core/scheduler/common.h +++ b/sdk/core/scheduler/common.h
@@ -30,6 +30,20 @@ ; using Debug = ConditionalDebug<DebugScheduler, "Scheduler">; + + constexpr StackCheckMode StackMode = +#if CHERIOT_STACK_CHECKS_SCHEDULER + StackCheckMode::Asserting +#else + StackCheckMode::Disabled + // Uncomment if checks failed to find the correct values + // StackCheckMode::Logging +#endif + ; + +#define STACK_CHECK(expected) \ + StackUsageCheck<StackMode, expected, __PRETTY_FUNCTION__> stackCheck + /** * Base class for types that are exported from the scheduler with a common * sealing type. Includes an inline type marker.
diff --git a/sdk/core/scheduler/main.cc b/sdk/core/scheduler/main.cc index c06f19a..88442be 100644 --- a/sdk/core/scheduler/main.cc +++ b/sdk/core/scheduler/main.cc
@@ -418,8 +418,10 @@ return ret; } -int __cheri_compartment("sched") thread_sleep(Timeout *timeout) +__cheriot_minimum_stack(0x80) int __cheri_compartment("sched") + thread_sleep(Timeout *timeout) { + STACK_CHECK(0x80); if (!check_timeout_pointer(timeout)) { return -EINVAL; @@ -428,11 +430,12 @@ return 0; } -int futex_timed_wait(Timeout *timeout, - const uint32_t *address, - uint32_t expected, - FutexWaitFlags flags) +__cheriot_minimum_stack(0xa0) int futex_timed_wait(Timeout *timeout, + const uint32_t *address, + uint32_t expected, + FutexWaitFlags flags) { + STACK_CHECK(0xa0); if (!check_timeout_pointer(timeout) || !check_pointer<PermissionSet{Permission::Load}>(address)) { @@ -512,8 +515,9 @@ return 0; } -int futex_wake(uint32_t *address, uint32_t count) +__cheriot_minimum_stack(0x90) int futex_wake(uint32_t *address, uint32_t count) { + STACK_CHECK(0x90); if (!check_pointer<PermissionSet{Permission::Store}>(address)) { return -EINVAL; @@ -552,11 +556,13 @@ return woke; } -int multiwaiter_create(Timeout *timeout, - struct SObjStruct *heapCapability, - MultiWaiter **ret, - size_t maxItems) +__cheriot_minimum_stack(0x50) int multiwaiter_create( + Timeout *timeout, + struct SObjStruct *heapCapability, + MultiWaiter **ret, + size_t maxItems) { + STACK_CHECK(0x50); int error; // Don't bother checking if timeout is valid, the allocator will check for // us. @@ -570,16 +576,20 @@ return write_result(reinterpret_cast<void **>(ret), mw); } -int multiwaiter_delete(struct SObjStruct *heapCapability, MultiWaiter *mw) +__cheriot_minimum_stack(0x60) int multiwaiter_delete( + struct SObjStruct *heapCapability, + MultiWaiter *mw) { + STACK_CHECK(0x60); return deallocate<MultiWaiterInternal>(heapCapability, mw); } -int multiwaiter_wait(Timeout *timeout, - MultiWaiter *waiter, - EventWaiterSource *events, - size_t newEventsCount) +__cheriot_minimum_stack(0xb0) int multiwaiter_wait(Timeout *timeout, + MultiWaiter *waiter, + EventWaiterSource *events, + size_t newEventsCount) { + STACK_CHECK(0xb0); return typed_op<MultiWaiterInternal>(waiter, [&](MultiWaiterInternal &mw) { if (newEventsCount > mw.capacity()) { @@ -667,9 +677,10 @@ }; } // namespace -[[cheri::interrupt_state(disabled)]] const uint32_t * -interrupt_futex_get(struct SObjStruct *sealed) +[[cheri::interrupt_state(disabled)]] __cheriot_minimum_stack( + 0x20) const uint32_t *interrupt_futex_get(struct SObjStruct *sealed) { + STACK_CHECK(0x20); auto *interruptCapability = Handle::unseal<InterruptCapability>(sealed); uint32_t *result = nullptr; if (interruptCapability && interruptCapability->state.mayWait) @@ -686,9 +697,10 @@ return result; } -[[cheri::interrupt_state(disabled)]] int -interrupt_complete(struct SObjStruct *sealed) +[[cheri::interrupt_state(disabled)]] __cheriot_minimum_stack( + 0x10) int interrupt_complete(struct SObjStruct *sealed) { + STACK_CHECK(0x10); auto *interruptCapability = Handle::unseal<InterruptCapability>(sealed); if (interruptCapability && interruptCapability->state.mayComplete) {
diff --git a/sdk/xmake.lua b/sdk/xmake.lua index 1d763bc..96b03cd 100644 --- a/sdk/xmake.lua +++ b/sdk/xmake.lua
@@ -49,6 +49,7 @@ end stackCheckOption("allocator") +stackCheckOption("scheduler") -- Force -Oz irrespective of build config. At -O0, we blow out our stack and -- require much stronger alignment.