Move to a stack-overflow-resilient error handler #48
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
davidchisnall
approved these changes
Oct 30, 2024
The TCP/IP stack cannot currently recover from a crash due to a stack overflow in the TCP/IP compartment. This is due to a limitation of the implementation of the switcher, which cannot trigger the error handler on stack overflow. Further, the current implementation of the error handler runs at the point in the stack where the compartment crashed. This implies that the error handler runs on a potentially very small stack, making it possible that it will itself crash due to a stack overflow. This commit addresses these limitations using David's recently-merged stackless error handler mechanism: CHERIoT-Platform/cheriot-rtos#301 The error handler is now declared at thread entry points (API entry points and driver entry point for user threads, and IP thread start for the IP thread). We can thus eliminate the big `compartment_error_handler`. The new error handler runs at the top of the stack, which eliminates all issues mentioned earlier (provided the stack is large-enough, which should be checked through `STACK_CHECK` - a separate problem). We keep a single error handler for user threads and the IP thread (`reset_network_stack_state`), differentiated through a `isIpThread` argument. Though we could separate this into two distinct error handlers for user threads and for the IP thread, it would result in a lot of duplicated error-prone code. Note that we do not "reinstall context" or "force unwind" anymore in the error handler. This is all handled under the hood by the new error handler, and we simply exit the error handler once done, which happens to be the place where we would leave the compartment in the force unwind case, or the place where we would reinstall the context for the IP thread. Signed-off-by: Hugo Lefeuvre <[email protected]>
If a user thread crashes and the error handler of the user thread triggers a crash in the IP thread, the current code will block forever because the IP thread does not release the `ipThreadLockState`. This problem stems from the fact that we only tested user thread crashes in a setting where the IP thread cleanly returned from `prvIPTask` (without a crash, i.e., without triggering the IP thread error handler). Address this by releasing the `ipThreadLockState` in both cases. Add comments to explain. Note: this is a pre-existing bug to the stack-overflow-resilient error handler, so separate commit. Signed-off-by: Hugo Lefeuvre <[email protected]>
hlef
force-pushed
the
hlefeuvre/stack-overflow-resilient-error-handler
branch
from
c7e8a22 to
6f0296b
Compare
|
Addressed the comments. Planning to merge once I re-tested the PR later today (no access to the FPGA right now). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The TCP/IP stack cannot currently recover from a crash due to a stack overflow in the TCP/IP compartment. This is due to a limitation of the implementation of the switcher, which cannot trigger the error handler on stack overflow. Further, the current implementation of the error handler runs at the point in the stack where the compartment crashed. This implies that the error handler runs on a potentially very small stack, making it possible that it will itself crash due to a stack overflow. This limitation is documented in #31.
This commit addresses these limitations using David's recently-merged stackless error handler mechanism: CHERIoT-Platform/cheriot-rtos#301.
We also address a bug in the reset, which happens when an error handler on a user thread triggers a crash of the IP thread. In this case, previously improperly tested, we fail to release a lock, resulting in a failure of the reset.
More information in each commit.