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

Consider weakening return sentries to support compiler outlining #85

Open
rmn30 opened this issue Nov 13, 2024 · 4 comments
Open

Consider weakening return sentries to support compiler outlining #85

rmn30 opened this issue Nov 13, 2024 · 4 comments

Comments

@rmn30
Copy link
Collaborator

rmn30 commented Nov 13, 2024

As per CHERIoT-Platform/llvm-project#46 the current restrictions on return sentries cause difficulties for compiler outlining because it may use a non-standard link register. Currently cjal / cjalr always produce a return sentry in the link register which can only be used to return via cret (i.e cjalr with cd == $cnull and cs1 == $cra).

For reference here is the relevant table for cjalr from the ISA doc: Screenshot 2024-11-13 111246

We could relax the ISA to permit using a non-standard link register by modifying cjal / cjalr where cd is not $cra such that they produce either 1) an unsealed capability or 2) a forward interrupt-inheriting sentry in the link register. This would permit a return using cjalr with cd == $cnull and cs1 != $cra. Of these 1) is preferable as it does not introduce the ability to create forward sentries, which is not currently possible without a capability with the appropriate sealing permission. It is already possible to create unsealed capabilities derived from PCC.

@davidchisnall
Copy link
Collaborator

davidchisnall commented Nov 13, 2024

If we made a non-cra link register provide an unsealed capability, then the following two sequences are equivalent:

  cjal {target}, ct0
0:
  auipcc ct0, %cheriot_compartment_hi(1f)
  cincoffset ct0, ct0, %cheriot_compartment_lo_i(0b)
  j  {target}
1:

In both cases, you return with cjr ct0, which is already permitted.

I believe this means that there is no security implication and the relaxation on cjal[r] simply allows a code-size reduction by folding three instructions into one.

@rmn30
Copy link
Collaborator Author

rmn30 commented Nov 13, 2024

As David points out this doesn't really weaken the current model at all. It really just fixes an anomaly in the above table where row three is not currently that useful: it can be used for making a function call with a non-standard link register but the called function has to move that register into $cra in order to return to it, which probably defeats the point of using a non-standard link register. The important thing is that calls to interrupt enabling / disabling sentries must use $cra so that the called function can definitely restore the previous interrupt stance.

nwf added a commit that referenced this issue Nov 13, 2024
nwf added a commit that referenced this issue Nov 13, 2024
nwf added a commit that referenced this issue Nov 13, 2024
Attempt at capturing #85

Co-authored-by: Robert Norton <[email protected]>
@vmurali
Copy link
Contributor

vmurali commented Nov 13, 2024

Another thing to note is that, in a call to a library, if a caller compartment wishes to sanitize the arguments and force the library to only use the correct return entry points (i.e. not trust the library), it can always use $cra as the link register in the call.

nwf added a commit that referenced this issue Nov 14, 2024
Attempt at capturing #85

Co-authored-by: Robert Norton <[email protected]>
@resistor
Copy link

I did a measurement with and without outlining disabled. Re-enabling MachineOutliner will save us approximately 4.4% code size (according to scripts/output_code_size.sh) on the CheriotRTOS test suite binary. As such I think it's definitely worthwhile to push ahead on getting this change in.

nwf added a commit that referenced this issue Nov 25, 2024
Attempt at capturing #85

Co-authored-by: Robert Norton <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

No branches or pull requests

4 participants