-
Notifications
You must be signed in to change notification settings - Fork 643
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
RISC-V write-only page mappings #1119
Comments
Yes, I don't think |
In that case I think we should make all the page mapping invocations consistent with ARM and report an error if the user tries to make a write-only mapping. |
The RISC-V privileged spec explicitly says "Writable pages must also be marked readable; the contrary combinations are reserved for future use.". And in the table "Encoding of PTE R/W/X fields":
|
That does make perfect sense and would not be a big change to verify (should affect the relevant decode functions only). Do we have a volunteer for implementing it? |
It's already consistent with Arm. The behavior on arm if you request write only mapping is that it degrades to VMKernelOnly without returning an error. Systems may rely on silent downgrading of a request to read+write to read only if the underlying cap doesn't have write privileges. I guess what happens when requesting write only was unspecified as it's not a valid final request. But it's possible to have caps with write permissions and so should the kernel also return an error if someone requests a mapping with a cap that only has write rights? |
When I made this issue I mistakenly thought that the kernel was returning an error on ARM for write-only mappings due to the kernel printing an error. seL4/src/arch/arm/64/kernel/vspace.c Line 155 in 76eee24
I guess I find the print confusing - is it trying to hint that user-space has potentially done something wrong or is it supposed to return an actual error? |
This generates just a warning, but I agree that it is a bit weird and should be consistent across architectures if we want the warning. Actually making it an error would require an RFC, because it's a breaking API change. That doesn't mean we can't do it, but the question is if this is really worth a breaking change. Maybe the only thing that is needed is to document this properly in the manual. |
I guess we can start with having consistent error messages making it clear in the manual. Are there any other cases in the kernel where a cap needs to have certain permissions for an invocation to be successful? |
Yes that is a normal kind of check. Whether it's wanted or not depends on each case. |
With the behaviour specified in the manual, I'd be happy to close this. So far there does not seem to be any need for a policy change. |
The creation of write-only page mappings on RISC-V does not seem correct to me.
I believe if I create a mapping with only write permissions, the value of
vmRights
becomes 0x1 based on putting debug logging in the kernel.A
vmRights
of 0x1 means that is theVMKernelOnly
entry of thevm_rights
enum. This means that inmakeUserPte
, the value ofRISCVGetWriteFromVMRights(vm_rights)
is 0, when it should actually be one. Hence, a mapping with write access does not get created.What's more confusing is that the kernel code also has this comment:
/* Write-only frame cap rights not currently supported. */
. However it does not seem to be preventing the user from creating write-only mappings, unlike on ARM.I don't know if I'm reading the code right, so I decided to make an sel4test test to try confirm my suspicions. You can find the patch here: Ivan-Velickovic/sel4test@169f157.
I ran on the HiFive Unleashed and QEMU RISC-V virt and observed a memory fault on both. The following log is from the HiFive Unleashed run:
If we look at the objdump we find the following instructions:
The program counter that caused the fault is at
0x590e8
, which is indeed doing a write.The text was updated successfully, but these errors were encountered: