CTSRD-CHERI / cheri-specification

CHERI ISA Specification
Other
23 stars 7 forks source link

Address zero-length capability issues. #117

Open PeterRugg opened 1 year ago

PeterRugg commented 1 year ago

Just thought I'd record our thoughts on zero-length capabilities.

I believe in a few cases (revocation and generally on free()) we suffer for allowing zero-length capabilities: we would ideally be able to conclude a capability came from an allocation if it is tagged and its base is within the allocation. This fails if you setBounds to length 0, setting the base equal to the top, in which case your capability appears to originate from the next allocation. This has some odd implications for CTestSubset.

It's definitely possible to workaround this in software, but this seems like it might keep coming up as a gotcha, and was one of the motivations behind me looking at encoding the top as inclusive and banning zero-length capabilities outright.

An alternative suggested by @jonwoodruff is to change CSetBounds to clear the tag if you're trying to setBounds the base to match the top, i.e. if the requested length is 0 and the cursor equals the top bound.

tariqkurd-repo commented 1 year ago

I vote for disallowing zero length caps, so it becomes impossible to have the tag set on one. It seems like a special case we should simply remove. This means we need to forbid them in CSetBounds (to create one with zero length) and CBuildCap (to authenticate one with zero length). Is there anywhere else that this needs to be addressed?

PeterRugg commented 1 year ago

@tariqkurd-repo If we want to ban them completely, it seems like a shame to waste the encoding space. It might be best to do the inclusive top encoding in that case? I suppose leaving the encodings there means there aren't any tricky questions about what happens if software asks for a zero-length thing. I had planned to just "round" them to 1-byte (clearing the tag if CSetBoundsExact).

PeterRugg commented 1 year ago

@jrtc27 noted that CSetBounds to length 0 needs to do something sensible, since it can happen in generated code for a variable length array at the end of a struct. I don't think it matters if the tag is set though: there's no way to dereference a zero-length capability in any case.

jrtc27 commented 1 year ago

If we choose to change behaviour around zero-length caps, that is a significant change that will require extensive validation across our corpus of software. One could even do it at a large scale by modifying the Morello compiler to emulate that behaviour for the intrinsic.

tariqkurd-repo commented 1 year ago

An alternative suggested by @jonwoodruff is to change CSetBounds to clear the tag if you're trying to setBounds the base to match the top, i.e. if the requested length is 0 and the cursor equals the top bound.

Can we do it this way? The encoding doesn't change, the software doesn't change, we just define that zero length caps can't be tagged, and as they can't be dereferenced, there should be no downside. Then we can have CGetLen return 0 for underivable caps, and so we merge the definitions of underivable and zero length. Are there any downsides to doing it this way?

PeterRugg commented 1 year ago

@tariqkurd-repo That's not quite what I meant by that: the idea there wasn't to ban all zero-length caps, but to ban setting a capability's base to its existing top, as a middle ground. I'm not sure it's any better than just banning zero length caps as you suggest though. I suspect @jrtc27's concern is about software getting confused about the tag suddenly going missing on zero-length things: not because they'd be dereferenced, but maybe because it manually checks the tag? Seems like it might be okay, but I agree we'd need to validate it.

tariqkurd-repo commented 1 year ago

I suspect @jrtc27's concern is about software getting confused about the tag suddenly going missing on zero-length things: not because they'd be dereferenced, but maybe because it manually checks the tag? Seems like it might be okay, but I agree we'd need to validate it.

If a non-deferenceable cap has been created, I'd argue that it's better to clear the tag up-front to make it clear that the generation is bad, as opposed to delaying the check until there's a dereference.

I like the concept of CGetLen=0 means that cap is unusable/underivable, for which I mean can't be tagged, not can't be dereferenced. I think that's a cleaner definition.