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

CAndPerms: permit clearing GL on sealed caps #83

Merged
merged 3 commits into from
Nov 28, 2024
Merged

Conversation

nwf
Copy link
Member

@nwf nwf commented Nov 8, 2024

Address #70. This is not ready to merge, in that it hasn't yet decided between the two options:

  1. Require that the mask to CAndPerms on a sealed operand must be all-ones or all-ones-but-GL
  2. Require only that the difference between the initial and masked permissions be at most the GL bit.

Shuffle the comment delimiters around to pick between the two. I believe either is fine from a security perspective; option 1 imposes stronger requirements on the program(mer) while the later tolerates some... incidental correctness. If one is clearly better for microarchitecture, tho', that could be the deciding factor.

Pinging @kliuMsft.

@kliuMsft
Copy link
Contributor

kliuMsft commented Nov 9, 2024

@nwf either p[topm is fine from microarchitecture perspective. Option 1 is a little easier to implement & verify but the difference is not huge.

@rmn30
Copy link
Collaborator

rmn30 commented Nov 11, 2024

If either works for uarch I think sw preference is for option 2 so lets go with that!

@davidchisnall
Copy link
Collaborator

The third option, which is easiest for software, is to treat whatever you have and whatever you have minus G as the only representable things and make CAndPerm do the same thing is does currently when the exact intersection is not representable.

@rmn30
Copy link
Collaborator

rmn30 commented Nov 19, 2024

@davidchisnall what you're suggesting would be the same as forcing all bits of the mask except GL to one if the capability is sealed. Isn't it likely to be a programmer error to try to clear non-GL permissions on a sealed capability? If so that proposal would allow the error to go undetected. On the other hand if the programmer wants to clear permissions on unsealed capabilities and leave sealed ones alone (except GL) that would be useful.

@nwf nwf marked this pull request as ready for review November 22, 2024 18:23
src/cheri_insts.sail Outdated Show resolved Hide resolved
Comment on lines 426 to 428
* Sealed caps clear the tag unless the mask is all ones (~mask == zeros())
* or has the global permission clear. This relies on the prop_null_noperms
* property to ensure that perm_global is a one-hot vector.
Copy link
Collaborator

@rmn30 rmn30 Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Sealed caps clear the tag unless the mask is all ones (~mask == zeros())
* or has the global permission clear. This relies on the prop_null_noperms
* property to ensure that perm_global is a one-hot vector.
* CAndPerms on a sealed cap clears the tag unless the mask is all ones
* or has *only* the global permission clear. Here `perm_global` is a
* vector of zeros except for the global permission bit. To construct this
* we rely on the fact that `null_cap` has all zero permissions.

Hopefully this is clearer. I removed the mention of prop_null_noperms as I thought it might just be confusing to people reading the arch doc. Good to have, though!

Copy link
Collaborator

@rmn30 rmn30 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good modulo suggested changes. Please also add a changelog entry and check if the archdoc text needs updating anywhere. We should mention the rationale for this change too.

@nwf nwf force-pushed the 202411-candperm-seal-gl branch 2 times, most recently from 393fdfd to 65b5499 Compare November 26, 2024 14:21
But require that all other bits in the mask provided to CAndPerms be 1
in order for a tagged sealed input to result in a tagged sealed output.

Co-authored-by: Robert Norton <[email protected]>
@nwf nwf requested a review from rmn30 November 26, 2024 19:00
@nwf nwf merged commit bc66dbb into main Nov 28, 2024
2 checks passed
@nwf nwf deleted the 202411-candperm-seal-gl branch November 28, 2024 15:34
nwf added a commit that referenced this pull request Dec 2, 2024
Sigh, sorry.  This was originally "~(mask | perm_global) == zeros()" and
had one too many inversions applied.
rmn30 pushed a commit that referenced this pull request Dec 2, 2024
Sigh, sorry.  This was originally "~(mask | perm_global) == zeros()" and
had one too many inversions applied.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done (Ibex)
Development

Successfully merging this pull request may close these issues.

4 participants