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

capability mode enables and register access controls for Zcheri_legacy #81

Merged
merged 3 commits into from
Feb 20, 2024

Conversation

sorear
Copy link
Contributor

@sorear sorear commented Jan 31, 2024

A CHERI mode enable for M-mode allows Zcheri_legacy to run full legacy software stacks, including firmware soon after reset.

This also removes the behavioral difference between Zcheri_legacy and Zcheri_mode in terms of the instruction set in effect after reset, making Zcheri_mode a true extension of Zcheri_legacy.

CHERI register access disables for S-mode and U-mode allow Zcheri_legacy to prevent cross-domain interference and covert channels within a legacy environment.

These two are the strictly additive part of #39.

A CHERI mode enable for M-mode allows Zcheri_legacy to run full legacy
software stacks, including firmware soon after reset.
This is a necessary security feature to prevent cross-domain
interference and covert channels through CHERI registers when using
Legacy S-mode.
@sorear sorear changed the title add mseccfg.CME capability mode enables and register access controls for Zcheri_legacy Feb 1, 2024
@tariqkurd-repo
Copy link
Collaborator

tariqkurd-repo commented Feb 14, 2024

What exactly is the difference between Xenvcfg.CME and Xenvcfg.CRE?
Is your intention that for the current privilege level/ring:

  • if CME = 0 then it's legacy mode so memory checking is authorised by DDC, but capabilities can be manipulated, DDC can be accessed etc.
  • if CRE = 0 then no CHERI instructions can be executed, so capabilities cannot be manipulated, DDC can't be updated etc.

so if CME=CRE=0 it really is a pure RISC-V machine - for that privilege level

do you intend CME to override CRE, so if CME=1 then CRE is don't care?

We've been back and forth many times over whether we need these extra enable bits (I was generally in favour of them, others less so), and where they should live (state-emable or envcfg). I'm not so sure that the location is very important, as all these things will be adjusted by the ARC in due course.

Given that we already say that CME=CRE=0 is what you get if the current XLEN < XLENMAX then adding CRE is not describing a new state in the machine, it's making an existing state more useful.

Something else to add is that we can always allow some of these enables to be hardwired in an implementation, e.g. in mseccfg so that it isn't required to implement the full set of different modes.

what's your opinion @andresag01 ?

@andresag01
Copy link
Collaborator

@sorear : Thanks for the proposal. I generally agree with the idea in this PR, but there are a couple of things that are unclear to me:

  • Should M-mode have its own CME and CRE bits?
  • Does the CHERI Execution mode control what the new CHERI instructions and the re-mapped instructions in CHERI do?

IMHO, it should be like this:

  • CME:
    • Makes the re-mapped instructions whatever they were in standard RISC-V (e.g. like the compressed FP) when in Legacy
    • Change all the relevant instruction operands to integer values when in Legacy mode
  • The CRE bit:
    • Forces CHERI mode to be Legacy
    • Disables access to CHERI CSRs
    • Disables access to CHERI instructions

What do you think about this?

@sorear
Copy link
Contributor Author

sorear commented Feb 16, 2024

@tariqkurd-repo

do you intend CME to override CRE, so if CME=1 then CRE is don't care?

I intended CRE=0 to override CME, on the theory that CRE is an access control but CME just affects instruction decoding.

Given that we already say that CME=CRE=0 is what you get if the current XLEN < XLENMAX then adding CRE is not describing a new state in the machine, it's making an existing state more useful.

It's a new mode from the point of view of fixed XLEN hardware.

Something else to add is that we can always allow some of these enables to be hardwired in an implementation, e.g. in mseccfg so that it isn't required to implement the full set of different modes.

There are three modes: capability (CME=1 CRE=1), transitional (CME=0 CRE=1), legacy (CRE=0 CME=x). Ignore transitional until I have a chance to post part 2. I don't think it makes sense to hardwire enables in Zcheri_legacy because if you only want capability mode, you should do Zcheri_purecap, and if you only want legacy mode you shouldn't do Zcheri at all.

@andresag01

Should M-mode have its own CME and CRE bits?

M-mode needs its own CME bit in order to allow running legacy M-mode firmware. M-mode's CRE bit is hardwired to 1 because there is no higher privilege mode that needs to control dataflow in and out of M-mode capability registers.

IMHO, it should be like this:

Unless I'm missing or poorly communicated something, that's how this proposal works.

Copy link
Collaborator

@tariqkurd-repo tariqkurd-repo left a comment

Choose a reason for hiding this comment

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

I like this PR in general.

I think that "CHERI Register Enable" should be "CHERI State Enable" because, just like othe rstate enable bits, it's controlling access to registers, CSRs and instructions.

With that renaming I thin it makes it clear that it should use a bit in Xstateen.

I know all this will change when the ARC get their hands on it, but I think the description more easily fits with the RISC-V ethos that way.

Other than that LGTM

@sorear
Copy link
Contributor Author

sorear commented Feb 16, 2024

With xstateen as currently defined, you can set it to ~0 or omit the extension if you don't care about covert channels; CHERI State Enable would be the first xstateen bit that allows U-mode to mount denial of service attacks against S-mode if it is set prematurely of other software support. I'm hesitating for that reason.

Does the rename block this being resolved?

@arichardson
Copy link
Collaborator

With xstateen as currently defined, you can set it to ~0 or omit the extension if you don't care about covert channels; CHERI State Enable would be the first xstateen bit that allows U-mode to mount denial of service attacks against S-mode if it is set prematurely of other software support. I'm hesitating for that reason.

I agree that we have to ensure CHERI is disabled for lower rings on boot due to DDC not being banked. If your kernel turns on all unknown extensions for lower privilege levels (even ones it doesn't know about), then that's just asking for trouble and I don't think that's something we have to prevent in this spec. Being able to hardwire it to all zeroes is more problematic, but we could just require the CHERI bit for lower privilege levels must be zero on reset?

@jrtc27
Copy link
Collaborator

jrtc27 commented Feb 16, 2024

With xstateen as currently defined, you can set it to ~0 or omit the extension if you don't care about covert channels; CHERI State Enable would be the first xstateen bit that allows U-mode to mount denial of service attacks against S-mode if it is set prematurely of other software support. I'm hesitating for that reason.

I agree that we have to ensure CHERI is disabled for lower rings on boot due to DDC not being banked. If your kernel turns on all unknown extensions for lower privilege levels (even ones it doesn't know about), then that's just asking for trouble and I don't think that's something we have to prevent in this spec. Being able to hardwire it to all zeroes is more problematic, but we could just require the CHERI bit for lower privilege levels must be zero on reset?

See riscvarchive/riscv-state-enable#12. This was a stupid choice in the RISC-V spec that wasn't really thought through properly IMO. Arm have the concept of RES0 and RES1 bits in their system registers for this reason, but in RISC-V we're pretending everything can be RES1, which is clearly a bad idea.

@arichardson
Copy link
Collaborator

With xstateen as currently defined, you can set it to ~0 or omit the extension if you don't care about covert channels; CHERI State Enable would be the first xstateen bit that allows U-mode to mount denial of service attacks against S-mode if it is set prematurely of other software support. I'm hesitating for that reason.

I agree that we have to ensure CHERI is disabled for lower rings on boot due to DDC not being banked. If your kernel turns on all unknown extensions for lower privilege levels (even ones it doesn't know about), then that's just asking for trouble and I don't think that's something we have to prevent in this spec. Being able to hardwire it to all zeroes is more problematic, but we could just require the CHERI bit for lower privilege levels must be zero on reset?

See riscv/riscv-state-enable#12. This was a stupid choice in the RISC-V spec that wasn't really thought through properly IMO. Arm have the concept of RES0 and RES1 bits in their system registers for this reason, but in RISC-V we're pretending everything can be RES1, which is clearly a bad idea.

Ah thanks for pointing me at that issue. It sounds like we do need to go for xenvcfg instead of stateenable if that is recommended.

The more invasive alternative would be to have DDC be per-ring (like with Morello) in which case enabling CHERI for a lower privilege level should not have any effect (other than the lower privilege level being able to shoot itself in the foot more effectively). But that change would require more discussion, so I am happy with the bit allocation as specified in this PR.

@tariqkurd-repo
Copy link
Collaborator

Yes - the stateenable description assume that no privilege can highjack the privilege above. So as Alex says without multiple DDCs then this seems like the way to go.

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.

5 participants