1. Tech-Privileged
  2. Topics

comments on PMP enhancements

John Hauser
 

Hi Allen,

The point I was trying to make about locked PMP entries, but failed to
communicate before, is this:

When a system starts up after reset, PMP is enforced according to a
certain set of rules. For two of the proposals, the initial rules
(MML = 0 or MSL = 0) are the same as currently standardized. For Mr.
Kurd's separate M & L proposal, when MML = 0, entries created with
M = 0 follow the currently standardized rules as they should.

After reset, an early stage of boot software might create a locked
PMP entry under the initial PMP rules, intending to restrict M-mode's
access to a certain region of memory. The early boot stage might be
oblivious to the PMP enhancements or (perhaps more likely) it might
want to be compatible with later boot stages that may or may not be
aware of the PMP enhancements.

If a later stage then raises the level of PMP security by setting
MML = 1 or MSL > 0, existing PMP entries are now enforced according
to a new set of rules. For two of the proposals, the new rule for an
existing locked entry is that S/U mode has no access and M mode has
restricted access. But for Mr. Kurd's separate M & L proposal, the
new rule is that S/U mode gets restricted access and M mode gets _no_
access. This is what I find questionable. The original intention
under the initial rules was almost surely to give M mode restricted
access, which has now been revoked. And the worst part is, although
M mode itself no longer has access, it cannot deny access to S/U mode,
because the entry is locked.

Here's what happens, step by step:

1. Machine starts from reset.

2. Early boot stage creates a locked PMP entry under initial PMP
rules, intending to restrict M mode's access to memory.

3. Subsequent boot stage enables PMP enhancement, by setting MML = 1
(or MSL > 0).

Under the separate M & L proposal, at this point M mode has _no_ access
to the memory covered by the PMP entry created in step 2, but S/U mode
_does_ have access. And this situation is unchangeable because the
entry is locked.

Under the task group's working proposal or my four-security-level
proposal, M mode still has restricted access. It is S/U mode that
loses access. I argue that this is better, and the separate M & L
proposal is "wrong" on this point.

On a different topic, I wrote:
Mr. Kurd's proposal does not similarly lock down all of M mode's
executable regions against modification nor prevent the creation of new
PMP entries for executable regions. That is the shortcoming to which I
was referring.
You:
Well, that would be a pretty simple modification of his proposal, then:
disallow creation of new Mode X regions when MML=1.
If that modification was made, what drawbacks remain?
Preventing the creation of new regions with execute permission is only
one piece of the lockdown of M-mode executable regions. The other is
to lock all existing PMP entries that give M mode execute permission,
so their address ranges can't be modified. The task group's working
proposal does that by locking all M-mode-only PMP entries, something
I replicated in my topmost security level, MSL = 3. Currently, the
separate M & L proposal doesn't do this piece either.

Here's what I wrote before about both shortcomings I identified in the
separate M & L proposal:

I believe both of these flaws can be fixed, but only at the expense of
the simple separation of the L and M bits. In fact, you start to get a
design that looks more like my proposal.
And that's what I want to emphasize: Why patch a proposal with several
tweaks when, as I see it, there is another, cleaner proposal on hand
that already covers all the same needs?

You:
I'd still like to simplify either proposal by removing DMC and replacing it
with "any entry locked".
Once you've locked a region, there is no need to access unmapped regions,
since you can do that from the locked region
That's conceivable, but has the disadvantage of sometimes requiring
one or more additional PMP entries. Thus, your desire to save one
flip-flop and a few gates for DMC could sometimes cost 40 or 50
flip-flops and corresponding logic to have at least one more PMP entry.
My own assessment is that, in any RISC-V core that implements standard
PMP, that one flip-flop and small bit of logic is not worth fretting
over.

Regards,

- John Hauser
Allen Baum
 

It is just one extra bit if its combined with an existing CSR, otherwise its extra decoding, scan and DVT logic, (not to mention extra readmux and write enable, regardless of  whether  its merged into another CSR.
My issue with "just one extra bit" is state explosion, which makes more corner cases and validation problems - just what you want to avoid for security.
Practically speaking, I 'm trying to come up with a where making DMC contingent on any entry locked will add an entry. 
The only one I can think of is that the initial bootrom image is never mapped, so once any entry is locked, it is unreachable.
Even there, the next stage must be validated and (eventually) locked, so any bootrom code required beyond that point could be included in the next stage 
(that initial bootrom should be pretty small), or locking could be delayed until all the entries that need access to it are defined (including the case where bootrom is contiguous with the next stage).
Can you give an example of a boot sequence that would need an extra entry? 
 

On Fri, Feb 21, 2020 at 5:20 PM John Hauser <jh.riscv@...> wrote:
Hi Allen,

The point I was trying to make about locked PMP entries, but failed to
communicate before, is this:

When a system starts up after reset, PMP is enforced according to a
certain set of rules.  For two of the proposals, the initial rules
(MML = 0 or MSL = 0) are the same as currently standardized.  For Mr.
Kurd's separate M & L proposal, when MML = 0, entries created with
M = 0 follow the currently standardized rules as they should.

After reset, an early stage of boot software might create a locked
PMP entry under the initial PMP rules, intending to restrict M-mode's
access to a certain region of memory.  The early boot stage might be
oblivious to the PMP enhancements or (perhaps more likely) it might
want to be compatible with later boot stages that may or may not be
aware of the PMP enhancements.

If a later stage then raises the level of PMP security by setting
MML = 1 or MSL > 0, existing PMP entries are now enforced according
to a new set of rules.  For two of the proposals, the new rule for an
existing locked entry is that S/U mode has no access and M mode has
restricted access.  But for Mr. Kurd's separate M & L proposal, the
new rule is that S/U mode gets restricted access and M mode gets _no_
access.  This is what I find questionable.  The original intention
under the initial rules was almost surely to give M mode restricted
access, which has now been revoked.  And the worst part is, although
M mode itself no longer has access, it cannot deny access to S/U mode,
because the entry is locked.

Here's what happens, step by step:

  1. Machine starts from reset.

  2. Early boot stage creates a locked PMP entry under initial PMP
     rules, intending to restrict M mode's access to memory.

  3. Subsequent boot stage enables PMP enhancement, by setting MML = 1
     (or MSL > 0).

Under the separate M & L proposal, at this point M mode has _no_ access
to the memory covered by the PMP entry created in step 2, but S/U mode
_does_ have access.  And this situation is unchangeable because the
entry is locked.

Under the task group's working proposal or my four-security-level
proposal, M mode still has restricted access.  It is S/U mode that
loses access.  I argue that this is better, and the separate M & L
proposal is "wrong" on this point.

On a different topic, I wrote:
> Mr. Kurd's proposal does not similarly lock down all of M mode's
> executable regions against modification nor prevent the creation of new
> PMP entries for executable regions.  That is the shortcoming to which I
> was referring.

You:
> Well, that would be a pretty simple modification of his proposal, then:
> disallow creation of new Mode X regions when MML=1.
> If that modification was made, what drawbacks remain?

Preventing the creation of new regions with execute permission is only
one piece of the lockdown of M-mode executable regions.  The other is
to lock all existing PMP entries that give M mode execute permission,
so their address ranges can't be modified.  The task group's working
proposal does that by locking all M-mode-only PMP entries, something
I replicated in my topmost security level, MSL = 3.  Currently, the
separate M & L proposal doesn't do this piece either.

Here's what I wrote before about both shortcomings I identified in the
separate M & L proposal:

> I believe both of these flaws can be fixed, but only at the expense of
> the simple separation of the L and M bits.  In fact, you start to get a
> design that looks more like my proposal.

And that's what I want to emphasize:  Why patch a proposal with several
tweaks when, as I see it, there is another, cleaner proposal on hand
that already covers all the same needs?

You:
> I'd still like to simplify either proposal by removing DMC and replacing it
> with "any entry locked".
> Once you've locked a region, there is no need to access unmapped regions,
> since you can do that from the locked region

That's conceivable, but has the disadvantage of sometimes requiring
one or more additional PMP entries.  Thus, your desire to save one
flip-flop and a few gates for DMC could sometimes cost 40 or 50
flip-flops and corresponding logic to have at least one more PMP entry.
My own assessment is that, in any RISC-V core that implements standard
PMP, that one flip-flop and small bit of logic is not worth fretting
over.

Regards,

    - John Hauser

1 - 2 of 2