Re: Huawei review of different PMP enhancement schemes


Nick Kossifidis
 

Some thoughts on the various proposals on the spreadsheet (v0.3):


M&L proposal:

The purpose of M bit is not clear, I get that the idea is to be able to
mark a rule that applies to M mode without having that rule also locked,
but for example the combination L,M = 1,0 when MML = 0 doesn't follow
that principle, it marks a rule as locked and also as enforced on M-mode
even though M = 0. When MML = 1 we get unlocked M-mode-only regions when
L,M = 0,1 but we also get locked S/U-only-regions when L,M = 1,0 which
doesn't make much sense (I think John also brought this up).


4level0.2:

With MSL=0 we get the current PMP behavior and with MSL=3 we get mostly
the same behavior as with MML=1 on the group's proposal, only with one
extra bit being used on pmpcfg and a redundant encoding (PL=2 and PL=3
are the same thing). Also it's possible to have a shared region that's
executable by S/U mode and RW by M mode, which is not possible with the
group's proposal as is.

With MSL=2 we get rid of the restriction of not being able to add new
executable M-mode-only regions, however that can be achieved by using
non-locked M-mode-only regions that are also available on MSL=2 (with
PL=3) since there is no such restriction defined for them. In other
words non-locked M-mode-only regions allow for this restriction to be
bypassed anyway.

With MSL=1 we get rid of the restriction of not allowing M-mode to
execute a region without a matching rule. However both locked and
non-locked M-mode-only regions allow for this restriction to be bypassed
on MSL=2 anyway since M-mode can just add such a rule and execute the
region, it's even worse with non-locked rules since afterwards M-mode
can also remove the rule and no one will ever know it happened. So to me
MSL=1 is redundant, I don't see any use for it. It's also obviously
redundant when DMC=1 but I'll come back to DMC later on.

So basically the extras we get are:
a) It's possible to have a region that's executable by S/U and RW by
M-mode for MSL > 0
b) It's possible to have removable M-mode-only rules when 0 < MSL < 3


4level0.3:

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, which allows
for an attacker to perform the attack described on the group's proposal
and is exactly what we are trying to prevent. This is possible on all
security levels by the way, even with MSL=3. It's also more complicated
since PL=0 on MSL=3 encodes both locked and non-locked rules. Finally
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.


Regarding DMC:

As shown above, restricting M-mode from executing memory regions without
a matching rule, only makes sense if it's not possible to add such a
rule (that allows execution). If it's possible to add a rule that
applies to M-mode then any restrictions regarding regions without a
matching rule, are a few instructions away from being bypassed. Same
applies when restricting r/w/x on M-mode with the DMC bit. In both
proposals DMC can be easily bypassed.

Even if we incorporate DMC on the group's proposal we 'll still be able
to add a rule that gives r/w privileges on M-mode, although this rule
will be a locked one so it'll at least be possible to detect this event.

However DMC to me is orthogonal to the various scenarios we discuss and
given that it's possible to reset the hart with a pre-defined set of PMP
rules, it makes sense to have such a mechanism. That's why my initial
reaction to Tariq's proposal regarding DMC, was to propose to him to
submit a separate proposal for this.


What we discussed on this week's TEE TG call:

a) Incorporate mseccfg.DMC to the group's proposal. It'll be a sticky
bit so when it gets set it can only be unset through hard-reset.

b) Allow for M-mode-only rules to be removable temporarily for debugging
/ flexibility purposes during boot (since this approach weakens PMP it
can't be defined as a security feature), with a big disclaimer/warning
in place, through the proposed DPL bit on mseccfg. This is also going to
be defined as an optional feature.

c) Add another bit for locking DPL, it'll only be possible to lock DPL
to 0 (disabled).

d) Use the remaining 2 encodings L=1,R=0,W=1,X=0 and L=1,R=0,W=1,X=1
when MML=1 to define a locked shared region that's executable by both M
and S/U mode but not writable by anyone (when X is set it's also
readable by M-mode), as Tariq proposed. The use case for this is to
share code between M-mode and S/U-mode, e.g. to support vendor-specific
extensions with custom assembly, without having to go through an ecall
(similar to Linux's VDSO).

e) Get rid of the security exception, use normal access faults instead.
S/U mode can use SBI to request more info from M-mode if needed (since
S/U can't access PMP registers to figure it out).


Regards,
Nick

Join tech-privileged@lists.riscv.org to automatically receive all group messages.