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

[CHERI-RISC-V] Update for tag clearing #210

Merged
merged 18 commits into from
Jun 16, 2023

Conversation

arichardson
Copy link
Member

@arichardson arichardson commented Jan 17, 2023

Catches up with CTSRD-CHERI/sail-cheri-riscv#56

Verified by running TestRIG against sail. While fuzzing against sail I also found some additional divergences that I'll upload fixes for in separate PRs.

@arichardson arichardson marked this pull request as draft February 1, 2023 13:19
@arichardson arichardson marked this pull request as ready for review April 27, 2023 16:49
@arichardson
Copy link
Member Author

Should be ready for review now.

@arichardson arichardson force-pushed the 2023-update-for-tag-clearing branch from de6412a to f39ac21 Compare May 11, 2023 18:50
@arichardson
Copy link
Member Author

Includes #219 to ensure the CI can pass.

This is cleaner than using TARGET_AARCH64 and will be required to update
CHERI-RISC-V to tag-clearing semantics.
Changing CHERI_TAG_CLEAR_ON_INVALID to 1 swaps over all functions that
are shared with the Morello model to tag clearing. It does not handled
instructions that are specific to MIPS/RISC-V but that will be added in
the following commits.
@arichardson arichardson force-pushed the 2023-update-for-tag-clearing branch from f39ac21 to f663baa Compare June 2, 2023 16:46
@arichardson
Copy link
Member Author

Rebased.

@arichardson arichardson force-pushed the 2023-update-for-tag-clearing branch from 47aef3c to 23e6e3a Compare June 8, 2023 18:57
Copy link
Contributor

@qwattash qwattash 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 to me. My only question is whether we should propagate the "became-unrepresentable" information in incoffset when the source cap is invalid. I don't have a specific case for which it is wrong though.

@arichardson
Copy link
Member Author

Looks Good to me. My only question is whether we should propagate the "became-unrepresentable" information in incoffset when the source cap is invalid. I don't have a specific case for which it is wrong though.

Ah that is a good point - probably not too useful if it's already untagged.

It is still not quite the same as the sail model when processing sentries,
but the trapping behaviour matches now.
This should also mostly fix CCSeal.
The current behaviour does not match sail yet, but this is the first
step towards spec-compliant behaviour.
It still doesn't quite match sail due to the old requirement to have
CAP_PERM_EXECUTE, but that is not a useful restriction - non-executable
sentries can be extremely useful as an alternative to otypes.
When I originally added sentries, I only considered the jump use case
and added a trap for non-executable inputs to catch errors early.
However, non-executable sentries can be extremely useful if you use
re-derivation for unsealing instead.

These semantics now match the sail model where the restriction was
removed as part of the tag-clearing changes.

For now this keeps the trapping behaviour for MIPS as that allows running
the existing testsuite before and after this change.
The cheri-compressed-cap setters assert that the value is in range, so
we have to mask before updating otype to arbitrary values.
Check that the input capability could have been created using a sequence
of valid subsetting instructions and compare the result to the raw bit
pattern instead of accepting any subset of the authorizing capability.

This matches the sail behaviour.
We have to check the second argument first.
Jenkins nested parallel has some rather surprising behaviours - in this
case it seems like a later successful call to junit set the entire build
status back to stable even if there were test failures. Using the same
logic as the CheriBSD job should hopefully make this branch fail as
expected.
This will allow detecting required CHERI features more easily and is
helpful for the transition period where older implementations still exist.

See also: CTSRD-CHERI/sail-cheri-riscv#74
bsdjhb pushed a commit to CTSRD-CHERI/cheribsd that referenced this pull request Jun 16, 2023
Once CTSRD-CHERI/qemu#210 is merged, QEMU will
tag clear instead of rasing exceptions on invalid sealing. To support
the transition period where CheriBSD can still be booted on older QEMU,
we raise a SIGPROT if the seal/unseal operation tag-clears.
The main branch of CheriBSD has test failures with tag-clearing QEMU as
it expects trapping behaviour and those changes will not be cherry-picked,
so we have to boot dev until main is updated.
@arichardson
Copy link
Member Author

Test failures are timeouts on the ASAN builds. Merging.

@arichardson arichardson merged commit 26e0295 into CTSRD-CHERI:dev Jun 16, 2023
@arichardson arichardson deleted the 2023-update-for-tag-clearing branch June 16, 2023 23:43
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