Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switcher fixes from #320 #326

Merged
merged 7 commits into from
Oct 29, 2024
Merged

Switcher fixes from #320 #326

merged 7 commits into from
Oct 29, 2024

Conversation

nwf
Copy link
Member

@nwf nwf commented Oct 29, 2024

#320 is taking a while since it's a sizable amount of prose. Pull the end-user visible changes off the bottom and make them their own PR.

nwf and others added 7 commits October 29, 2024 16:50
Add some static assertions to check the switcher's requirement that
these structures have sufficiently compatible layout.
It's important to check the right register when deciding not to zero the
stack.

Co-authored-by: Robert Norton <[email protected]>
Partially addresses
#321 , but is not
the end of it.

Co-authored-by: Robert Norton <[email protected]>
We were not installing the correct thing into ct0 (which would then get
moved into csp) prior to reaching .Linvoke_error_handler when the
thread was in its initial compartment invocation.

FIXES: #321
Don't forcibly unwind if the invoker has exhausted its stack, just
return an error.  Certain other "unlikely to work well" scenarios (such
as stripping store permission from csp before invoking the switcher) can
also trigger this "just an error" logic.  However, the defenses against
invoking the callee with a bad stack remain and will catch things like
"stripping capability handling permission from csp".

Add a test to ensure that this is what happens.

In terms of implementation, this causes the trap handler
(exception_entry_asm) to `mret` to a `cjalr ra` (`cret`) instruction at
the end of the switcher function while clobbering only caller-ignored
registers (t2 in __Z26compartment_switcher_entryz) and the return
registers (a0, a1) to signal the error code.  Thanks to Murali for
brainstorming and pointing out deficiencies in other approaches.

Co-authored-by: Murali Vijayaraghavan <[email protected]>
Ever since #297
landed, we have code that relies on the stack top being bounded per
invocation.  Rather than make CONFIG_NO_SWITCHER_SAFETY work again, it's
probably better to just remove it.
Don't let the stack go out of bounds before we check whether there is
enough room.
@nwf nwf requested a review from davidchisnall October 29, 2024 17:04
@nwf nwf merged commit 5344b71 into main Oct 29, 2024
6 checks passed
@nwf nwf deleted the 202410-switcher_fixes branch October 29, 2024 17:19
nwf added a commit that referenced this pull request Oct 30, 2024
GitHub makes it easy to erase useful information.  Attempt to partially
compensate for my mistake by recovering the commit history of #326 as
one parent of this merge; both parents have identical associated trees.
@nwf nwf mentioned this pull request Nov 7, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants