[RISC-V] [tech-tee] Huawei review of different PMP enhancement schemes


Στις 2020-02-28 21:14, John Hauser έγραψε:
Nick Kossifidis wrote:
This is dangerous ! With this revision it's possible to have a region
that's rw by S/U mode and executable by M mode when PL=0, [...]
I agree that would be dangerous, but I intentionally excluded that
possibility, so I don't understand. What is the exact encoding that
you think allows this, when MSL > 0?
You've excluded the possibility of having a region that's writeable by S/U and executable by M mode at the same time. However it's possible for S/U to put code on a R/W region and then request from M-mode to mark that region as R-X with PL=0, in which case this will also make the region executable by M-mode. The restriction of not being able to add new regions that are executable by M-mode only applies to MSL=3, not on MSL < 3. Depending on the boot flow it's also possible for such a region to be added there when MSL < 3 and also affect the system even when MSL=3, in which case it won't be possible to remove it. Also on MSL=3 it's not clear to me if this restriction applies to rules with PL=0, you say that when a rule with PL=0 is X or R-X it becomes locked, does this apply to existing rules or is it possible to add new ones ? Because it's otherwise possible to add new rules with PL=0 from what I understand. If that's the case isn't it a bit over-complicated to have PL=0 encode both locked and non-locked rules at the same time on MSL=3, and /or allowing some rules with PL=0 to be registered but not others ?

In contrast the group's proposal will make any region marked with L=0 inaccessible to M mode when MML is set so even if such a rule that provides X privileges is there before MML is set, it won't affect M mode after MML is set. Also it's not possible to define a shared region before MML gets set since the combination RW=01 is reserved in the current spec (which is the same as MML=0) and PMP registers are WARL. When MML gets set it's only possible to add non-executable shared regions, the only way to add a shared region that can be executable by both M and S/U mode, using the remaining encodings (still without using any extra bits on pmpcfg) as Tariq suggested, will be through setting DPL bit first, which is going to be an optional feature as we discussed on the group and will come with a proper warning / disclaimer.

when MSL=3 and PL=3 we get removable M-mode-only, non-executable
regions, at the highest security level. In terms of security it's a
regression over revision 0.2, not an improvement.
That detail could easily be changed, if that's the only remaining
complaint about the security.
It's not just the security issues mentioned, the redundant encodings and the fact it's using an extra bit on pmpcfg even though we can get what we want without doing so, leaving more options for future use, are also there. The overall complexity of the 4level proposal was also brought up on our last TEE TG call by others as well, and since we reached a consensus on integrating Tariq's proposal to the group's proposal I'm going for that approach instead.