Fix Morello capability store faults #167
Conversation
|
I don't think this is what you want. Obviously, the true correct behaviour when when !sc && cdbm is to have hardware tracking. Given that we don't to support this, I agree it might be fair to just trap (although maybe it should be up to the OS to just not set cdbm? What does arm do with the regular dbm bit when hardware does not support it?). In which case, you want to ignore cdbm completely. Currently what you have will trap capability stores when !sc, but only on TLB refill. The TLB entry still depends on CDBM and so subsequent accesses will fault differently. |
| *prot |= PAGE_SC_TRAP; | ||
| // Software capability dirty tracking means we fault on !sc | ||
| if (!sc) { | ||
| if (!cdbm) |
There was a problem hiding this comment.
I don't understand why this needs checking with the way Armv8 defines DBM. This function isn't checking the normal DBM, for example. SC is the "can I store capabilities bit" and CDBM is the "atomically update SC to 1 when you would otherwise fault due to SC=0" bit.
There was a problem hiding this comment.
This should probably ignore cdbm completely if you want traps because we don't support hardware modification of PTEs. This patch would cause accesses that cause TLB refill to act differently than those that hit the TLB, which is certainly wrong.
|
For regular dirty tracking, the OS gets a protection fault when the write inhibit bit is set. In the handler, if a write caused the fault and DBM is set then it will clear the bit that inhibits write access. To me the analogous thing would be to raise the capability fault when SC is unset and to set SC in the handler if it's a cap store and CDBM is set. I agree with both of your comments about the check for !cdbm in my patch. Initially I didn't have the check, but without it the OS panics during boot with a tag violation. Not sure why that's the case as I'm unfamiliar with QEMU. Does either of you have any ideas about what might be happening? The OS patch is here: CTSRD-CHERI/cheribsd@994796e. |
Previous logic had us only faulting when CDBM and SC are both unset, but for software manged capability dirty tracking we need to fault any time SC isn't set.