-
Notifications
You must be signed in to change notification settings - Fork 5
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
Fix setting bounds (non-canonical) untagged capabilities #11
Merged
arichardson
merged 5 commits into
CTSRD-CHERI:master
from
arichardson:untagged-setbounds
Feb 1, 2023
Merged
Fix setting bounds (non-canonical) untagged capabilities #11
arichardson
merged 5 commits into
CTSRD-CHERI:master
from
arichardson:untagged-setbounds
Feb 1, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jrtc27
reviewed
Jan 11, 2023
arichardson
force-pushed
the
untagged-setbounds
branch
from
January 12, 2023 11:14
f711d90
to
064a9f1
Compare
jrtc27
reviewed
Jan 12, 2023
jrtc27
reviewed
Jan 12, 2023
arichardson
force-pushed
the
untagged-setbounds
branch
from
January 13, 2023 11:59
064a9f1
to
6716d22
Compare
Rebased and fixed issues. |
…ties When called on untagged quantities these do not necessarily hold.
…ility This was found by the fuzzer since it triggered an assertion, now it matches the sail outputs. When calling decompress_raw we have to use req_base and not the current address to infer the bounds. This did not matter previously since we only called the function on in-bounds caps, so the result was the same, but now that we allow out-of-bounds untagged values, we have to use req_base instead and set the address to req_base.
Due to the way we decode top/bot it is possible for top to decode to 1<<66, but that is then truncated to zero and can therefore be less than the base of the capability. The example test case was found by TestRIG See CTSRD-CHERI/sail-cheri-riscv#36 for an alternative decoding approach that guarantees top >= base.
arichardson
force-pushed
the
untagged-setbounds
branch
from
January 13, 2023 15:25
6716d22
to
cb47aab
Compare
Found another assertion that can be triggered on untagged inputs. Relaxing this also seems fine unless we decide to merge CTSRD-CHERI/sail-cheri-riscv#36. |
Actually request more than 2 addresses worth of input, so we end up with a non-zero req_len input. With this change the assertion relaxed in the last commit is also found by libfuzzer.
If there are no further comments I will merge this tomorrow. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was found by the fuzzer since it triggered an assertion, now it
matches the sail outputs.
When calling decompress_raw we have to use req_base and not the current
address to infer the bounds. This did not matter previously since we only
called the function on in-bounds caps, so the result was the same, but now
that we allow out-of-bounds untagged values, we have to use req_base
instead and set the address to req_base.