Remove reset attack surface #54
Open
Conversation
This file contains hidden or 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
If a thread somehow corrupts the socket list, we will lose the ability to retrieve references to socket locks, ultimately preventing us from unblocking threads blocked on them. To handle that situation, ensure that threads block on the socket lock in steps, checking the network stack reset state at every step through `LockGuard` "conditions". If a network stack reset is detected in this way, threads will bail out. Note that the "condition" lambda is necessary here, since socket locks are allocated on a caller capability which we cannot heap-free-all. (see recent additions to the `LockGuard` class). Signed-off-by: Hugo Lefeuvre <[email protected]>
This commit addresses a number of issues in the reset when socket lists are corrupted. Together with the recent support of steps/conditions when waiting on socket locks and event queues, this removes the socket list from the set of reset-critical variables. Signed-off-by: Hugo Lefeuvre <[email protected]>
Although `currentSocketEpoch` and `userThreadCount` are both reset critical, they should be impossible to corrupt by construction, unless control-flow or spatial memory safety is compromised. Document that. Signed-off-by: Hugo Lefeuvre <[email protected]>
By moving the reset of `threadEntryGuard` at a different place in the execution flow, we can remove it from the set of reset variables. The idea here is to only reset `threadEntryGuard` in the case of a crash, and only if the crash was triggered by a user thread, since resetting is not needed in the case of a network thread crash (due to deterministic execution flow, see comment in the code). Signed-off-by: Hugo Lefeuvre <[email protected]>
This is not used anymore, we stopped using it with the stack-overflow-resilient handler. Signed-off-by: Hugo Lefeuvre <[email protected]>
Currently the example ignores the failure and tries to re-open the listening socket, running into an infinite loop. This is not meaningful, if we cannot close the listening socket we better stop since we will never be able to re-bind onto the server port anymore. Signed-off-by: Hugo Lefeuvre <[email protected]>
|
(The CI fails because the RTOS core PR has not yet been merged). |
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.
One current limitation of the network stack reset is that a certain set of "reset-critical" variables persists across resets, or are used as part of resets, and will prevent resets from being effective if compromised. This is a relevant attack surface, as compromising these variables will fully DoS the network stack, only repairable through a full reboot. This limitations was tracked as part of #31.
In this PR, I intend to entirely eliminate this attack surface.
I go through the list of reset-critical variables and identify those which are not a problem (by construction they cannot be attacked with our threat model). Thanks to @davidchisnall for discussing this.
This leaves us with two important pieces of data: the socket list, and the
threadEntryGuard. I address these through refactoring:threadEntryGuardat a different locations in the code to make it non-vulnerableInitially we intended to address this by adding an internal compartment to protect this state (similarly to microreboots), however we realized that this PR's approach is a much better option.
I tested these changes assuming a fully compromised socket list. The network stack reset still works as a charm, albeit visibly more slowly. In practice this should just be an edge case.