Open mndstrmr opened 4 months ago
Thanks for finding this @mndstrmr I'll be putting a fix together shortly.
In the scenario you describe any instruction that's in the IF stage when the clear is executed has already has its PMP check done under the old pre-clear state. Ultimately whilst clearly an issue that must be fixed I think the practical impact of this is limited.
First it is only the instruction in the IF stage that immediately follows the clear that 'sees' the old PMP state (before clear) and hence could execute when it shouldn't. It's also notable that only the execute part of the PMP permissions is effected as the read/write is checked when we attempt to being the relevant memory transaction and will always see the latest version of the PMP state.
Second there are limited scenarios under which an csrrc can cause a PMP state change that is immediately relevant. You must alter the configuration of a region that you are about to execute (i.e. a region that covers the instruction after your clear) and any change to the PMP CSRs must be done in M mode so you need to alter M mode relevant state. This means either the region is locked (so cannot be altered unless msseccfg.RLB is set) or is one of the epmp shared data regions (which don't provide execute permissions so don't matter for this issue). This means you have to have msseccfg.RLB set to produce a code sequence that can be effected by this issue. The MMWP and MML bits in msseccfg cannot be cleared once set so cannot be affected by this issue (which only relates to CSR bit clears).
For any software running on a version of Ibex effected by this issue it's recommended the CSR clears are never used to alter PMP state to avoid this scenario entirely.
In this commit the pipeline was made to flush on changes to PMP registers, changes made via
CSR_OP_CLEAR
however are missed.This is likely because prior to this commit only mstatus and mie were listed, and clearing is safe for those.
This meant that instructions being fetched were passing PMP checks which they should not have been.