-
Notifications
You must be signed in to change notification settings - Fork 679
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Re-integrate Smepmp #1879
base: main
Are you sure you want to change the base?
Re-integrate Smepmp #1879
Conversation
* Current chapter is the proposal document as-is, clean it up to better fit with the rest of the document. * Add a link to the proposal document for more info. * Add references from/to mseccfg definition, and to the PMP chapter. While there also add a reference to the Smmpm chapter on mseccfg definition. * Rename MMWP to MMAP (Allowlist instead of Whitelist) and update the register field definition and the visual representation fo Smepmp. * Clarify the reset state of mseccfg.MML/MMAP/RLB in the reset chapter. * Add the editors of Smepmp (me and Joe Xie) to the list of contributors. Signed-off-by: Nick Kossifidis <mick@ics.forth.gr>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I was imagining a lighter touch rewording that would be a bit easier to review. Not saying the new version isn't better, it's just a bit hard to tell if everything the old version said is still present.
The definitions of the RLB, MMWP, and MML fields are furnished by the | ||
PMP-enhancement extension, Smepmp. | ||
The definitions of the RLB, MMAP, and MML fields are part of the Smepmp | ||
extension, as defined in <<smepmp>>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just hyperlink "the Smepmp extension".
extension, as defined in <<smepmp>>. | |
The RLB, MMWP, and MML fields are defined in <<smepmp,the PMP-enhancement extension, Smepmp>>. |
|
||
The definition of the PMM field is furnished by the Smmpm extension. | ||
The definition of the PMM field is furnished by the Smmpm extension in <<Zpm>>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The definition of the PMM field is furnished by the Smmpm extension in <<Zpm>>. | |
The PMM field is defined in <<Zpm,the Smmpm extension>>. |
the platform mandates a different reset value for some PMP registers’ A | ||
and L fields. If the hypervisor extension is implemented, the | ||
the reset. Writable PMP registers’ A and L fields, and `mseccfg`.MML/MMAP/RLB | ||
fields are set to 0, unless the platform mandates a pre-defined PMP ruleset. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably needs a changelog entry?
@@ -2,73 +2,36 @@ | |||
== "Smepmp" Extension for PMP Enhancements for memory access and execution prevention in Machine mode, Version 1.0 | |||
=== Introduction | |||
|
|||
Being able to access the memory of a process running at a high privileged execution mode, such as the Supervisor or Machine mode, from a lower privileged mode such as the User mode, introduces an obvious attack vector since it allows for an attacker to perform privilege escalation, and tamper with the code and/or data of that process. A less obvious attack vector exists when the reverse happens, in which case an attacker instead of tampering with code and/or data that belong to a high-privileged process, can tamper with the memory of an unprivileged / less-privileged process and trick the high-privileged process to use or execute it. | |||
The Smepmp extension enhances the xref:pmp[xrefstyle=basic] mechanism to improve Machine mode's security, by providing stronger memory isolation and privilege separation. It enables preventing the execution of code from memory regions not explicitly marked as executable by Machine mode, and restricts Machine mode's access to memory regions designated for Supervisor and User modes. These capabilities help to mitigate control-flow subversion attacks that exploit execution of unauthorized code with Machine mode privileges, and provide security guarantees consistent with the mechanism described in <<sum>> for memory access and execution prevention between Supervisor and User mode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Smepmp extension enhances the xref:pmp[xrefstyle=basic] mechanism to improve Machine mode's security, by providing stronger memory isolation and privilege separation. It enables preventing the execution of code from memory regions not explicitly marked as executable by Machine mode, and restricts Machine mode's access to memory regions designated for Supervisor and User modes. These capabilities help to mitigate control-flow subversion attacks that exploit execution of unauthorized code with Machine mode privileges, and provide security guarantees consistent with the mechanism described in <<sum>> for memory access and execution prevention between Supervisor and User mode. | |
The Smepmp extension enhances the xref:pmp[xrefstyle=basic] mechanism to improve Machine mode's security by providing stronger memory isolation and privilege separation. It allows preventing the execution of code from memory regions not explicitly marked as executable by Machine mode, and restricts Machine mode's access to memory regions designated for Supervisor and User modes. These capabilities help to mitigate control-flow subversion attacks that exploit execution of unauthorized code with Machine mode privileges, and provide security guarantees consistent with the mechanism described in <<sum>> for memory access and execution prevention between Supervisor and User mode. |
Tbh I think the old text was a lot clearer. I would just change the second paragraph to
To prevent this attack vector, two mechanisms known as Supervisor Memory Access Prevention (SMAP) and Supervisor Memory Execution Prevention (SMEP) can be used. The first one prevents the OS from accessing the memory of an unprivileged process unless a specific code path is followed, and the second one prevents the OS from executing the memory of an unprivileged process at all times. RISC-V includes support for SMAP, through the
sstatus.SUM
bit, and for SMEP by always denying execution of virtual memory pages marked with the U bit, with Supervisor mode (OS) privileges, as mandated on the Privilege Spec. This extension adds support for similar mechanisms for Machine mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes more sense to explain what this extension does than why it's required, the why makes more sense in the proposal doc, than in the privilege spec. No information is lost here, We still summarize the attack vector, and reference the smap/smep behavior on S-mode for consistency, and it's more compact than the previous version. Also reading the original text people may assume that there is also a specific code path in this case, when instead we use shared regions for that purpose, so the more we mention what S-mode does, the more we risk misleading people.
|
||
However, there are no such mechanisms available on Machine mode in the current (v1.11) Privileged Spec. It is not possible for a PMP rule to be *enforced* only on non-Machine modes and *denied* on Machine mode, to only allow access to a memory region by less-privileged modes. it is only possible to have a *locked* rule that will be *enforced* on all modes, or a rule that will be *enforced* on non-Machine modes and be *ignored* by Machine mode. So for any physical memory region which is not protected with a Locked rule, Machine mode has unlimited access, including the ability to execute it. | ||
|
||
Without being able to protect less-privileged modes from Machine mode, it is not possible to prevent the mentioned attack vector. This becomes even more important for RISC-V than on other architectures, since implementations are allowed where a hart only has Machine and User modes available, so the whole OS will run on Machine mode instead of the non-existent Supervisor mode. In such implementations the attack surface is greatly increased, and the same kind of attacks performed on Supervisor mode and mitigated through SMAP/SMEP, can be performed on Machine mode without any available mitigations. Even on implementations with Supervisor mode present attacks are still possible against the Firmware and/or the Secure Monitor running on Machine mode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all useful text! I don't think it should be deleted; just reworded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo this is all redundant. it reads like a textbook, not an ISA standard
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why an ISA standard shouldn't have some motivation. There are plenty of notes already in the specification that aren't normative.
It could be condensed though for sure. I think it's at least worth mentioning that this is most useful for M/U machines where supervisor protection mechanisms cannot be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Protecting Machine mode is always relevant, in any implementation scenario. What we said in the original document was that on systems where the OS runs on Machine mode, the attack surface is greatly increased, not because you don't have S-mode protections in-between, but because you run more complex code on M-mode with a higher probability of a vulnerability being present there that would allow for the exploitation of memory accesses from M-mode to less privileged modes ( tricking M-mode to execute code from less privileged modes). So it's not that Smepmp is most useful for M/U machines, we make this clear in the original document, in the last sentence of the part you quoted "Even on implementations with Supervisor mode present attacks are still possible...". With CoVE for example coming up, where we are going to have more complex code running on M-mode, it makes perfect sense to have Smepmp available, OpenSBI (which targets systems with Supervisor mode available) already uses it.
In any case this further explains the "why" not the "what" or "how", IMHO it makes more sense to keep the extended version of the "why" part in the proposal document which is referenced in case anyone wants to dig deeper, than having it as part of the ISA manual. The short version is still there: "to improve Machine mode's security by providing stronger memory isolation and privilege separation". I agree non-normative text still makes sense which is why I added tips e.g. for "how" shared regions may be used, or "why" the last two restrictions were added to MML, I think it's cleaner to have short tips than whole paragraphs.
[[proposal]] | ||
=== Proposal | ||
|
||
. *Machine Security Configuration (mseccfg)* is a new RW Machine mode CSR, used for configuring various security mechanisms present on the hart, and only accessible to Machine mode. It is 64 bits wide, and is at address *0x747 on RV64* and *0x747 (low 32bits), 0x757 (high 32bits) on RV32*. All mseccfg fields defined on this proposal are WARL, and the remaining bits are reserved for future standard use and should always read zero. The reset value of mseccfg is implementation-specific, otherwise if backwards compatibility is a requirement it should reset to zero on hard reset. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All mseccfg fields defined on this proposal are WARL
This seems to have been lost.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch ! We make it clear than all bits are locked either on set or on clear, but we should say they are WARL in any case, I'll update the pull request.
|
||
The ``mseccfg.RLB`` bit provides a mechanism to temporarily modify Locked PMP rules: | ||
|
||
* When mseccfg.RLB=1, Locked PMP rules may be modified and locked PMP entries may be edited. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* When mseccfg.RLB=1, Locked PMP rules may be modified and locked PMP entries may be edited. | |
* When mseccfg.RLB=1 *locked* PMP rules may be modified and *locked* PMP entries may be edited. |
|
||
.. If ``mseccfg.MML`` is not set, the combination of ``pmpcfg.RW=01`` remains reserved for future standard use. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has this been lost?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seemed redundant, we only override this when MML is set, didn't make much sense to say that when MML is not set, those bits remain reserved. It's also clean from the visual representation, but if you think it's worth re-adding it, I'm ok with it.
.. In case ``mseccfg.MMWP`` is not set, M-mode can still access and execute any region not covered by a PMP rule. Since we try to prevent M-mode from executing malicious code and since an attacker may manage to place code on some region not covered by PMP (e.g. a directly-addressable flash memory), we need to ensure that M-mode can only execute the code segments initialized during firmware / OS initialization. | ||
=== Smepmp software discovery | ||
|
||
Since all fields defined on ``mseccfg`` under this proposal are either locked when set (``MMAP``/``MML``) or locked when cleared (``RLB``), software cannot dynamically query them to determine the presence of Smepmp. It is expected that BootROM will set ``mseccfg.MMAP`` and/or ``mseccfg.MML`` during early boot, before transferring control to the firmware. The firmware can then determine the presence of Smepmp by reading ``mseccfg`` and verifying the state of ``mseccfg.MMAP`` and ``mseccfg.MML``. Alternatively a software-defined discovery mechanism may be used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BootROM -> "the firmware"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"BootROM, ...during early boot, before transferring control to the firmware" The idea here is that the BootROM is implementation-specific, while the firmware as a software component may be more implementation-agnostic, such as OpenSBI for example or another third-party firmware (it makes perfect sense to have a reference firmware implementation like OpenSBI, that we can maintain/verify etc, and use that in most situations).
|
||
[[rationale]] | ||
=== Rationale |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like a shame to lose all this information too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the important stuff is still there, some as part of the implementation considerations, some as tips/info, and the most important part regarding rlb is mentioned twice. It seems inconsistent with the rest of the document to keep this, it's a bit too much to e.g. explain why we didn't say anything about mprv, or what was our thought process etc. The document is still there for people to dig deeper, it doesn't have to be included in the privilege spec. The chapter was 7 pages long, it's now 4 pages long and still contains what's needed for someone to implement this.
Tim and Andrew,
You are both right in that the general practice for RISC-V arch specs is to
focus on being just an architecture spec - with limited and concise key
non-normative notes. Larger explanations of motivation, the why behind
arch choices, usage guidelines, etc. should be elsewhere.
The "original" ePMP spec contains large amounts of non-normative exposition
- which is what Andrew is correctly commenting on. And part of the
expected exercise in rewriting the spec - and not just integrating it
largely as-is into the ISA manual - is to move it to the aforementioned
general RISC-V practice for arch specs. The overall work needs to cover
both stages of integration and rewrite, and then a review by security HC
(and old ePMP TG) members should be conducted.
Greg
…On Mon, Mar 3, 2025 at 6:04 AM Tim Hutt ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/smepmp.adoc
<#1879 (comment)>
:
> * *PMP Entry*: A pair of ``pmpcfg[i]`` / ``pmpaddr[i]`` registers.
-* *PMP Rule*: The contents of a pmpcfg register and its associated pmpaddr register(s), that encode a valid protected physical memory region, where ``pmpcfg[i].A != OFF``, and if ``pmpcfg[i].A == TOR``, ``pmpaddr[i-1] < pmpaddr[i]``.
-* *Ignored*: Any permissions set by a matching PMP rule are ignored, and _all_ accesses to the requested address range are allowed.
-* *Enforced*: Only access types configured in the PMP rule matching the requested address range are allowed; failures will cause an access-fault exception.
-* *Denied*: Any permissions set by a matching PMP rule are ignored, and _no_ accesses to the requested address range are allowed.; failures will cause an access-fault exception.
-* *Locked*: A PMP rule/entry where the ``pmpcfg.L`` bit is set.
-* *PMP reset*: A reset process where all PMP settings of the hart, including locked rules/settings, are re-initialized to a set of safe defaults, before releasing the hart (back) to the firmware / OS / application.
-====
-
-==== Threat model
-
-However, there are no such mechanisms available on Machine mode in the current (v1.11) Privileged Spec. It is not possible for a PMP rule to be *enforced* only on non-Machine modes and *denied* on Machine mode, to only allow access to a memory region by less-privileged modes. it is only possible to have a *locked* rule that will be *enforced* on all modes, or a rule that will be *enforced* on non-Machine modes and be *ignored* by Machine mode. So for any physical memory region which is not protected with a Locked rule, Machine mode has unlimited access, including the ability to execute it.
-
-Without being able to protect less-privileged modes from Machine mode, it is not possible to prevent the mentioned attack vector. This becomes even more important for RISC-V than on other architectures, since implementations are allowed where a hart only has Machine and User modes available, so the whole OS will run on Machine mode instead of the non-existent Supervisor mode. In such implementations the attack surface is greatly increased, and the same kind of attacks performed on Supervisor mode and mitigated through SMAP/SMEP, can be performed on Machine mode without any available mitigations. Even on implementations with Supervisor mode present attacks are still possible against the Firmware and/or the Secure Monitor running on Machine mode.
I don't see why an ISA standard shouldn't have some motivation. There are
plenty of notes already in the specification that aren't normative.
It could be condensed though for sure. I think it's at least worth
mentioning that this is most useful for M/U machines where supervisor
protection mechanisms cannot be used.
—
Reply to this email directly, view it on GitHub
<#1879 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALLX6GV4LEUCQZFS3Y3Q7W32SROPRAVCNFSM6AAAAABYGDGWM2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMNJUGIZDANJVGI>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Current chapter is the proposal document as-is, clean it up to better fit with the rest of the document.
Add a link to the proposal document for more info.
Add references from/to mseccfg definition, and to the PMP chapter. While there also add a reference to the Smmpm chapter on mseccfg definition.
Rename MMWP to MMAP (Allowlist instead of Whitelist) and update the register field definition and the visual representation fo Smepmp.
Clarify the reset state of mseccfg.MML/MMAP/RLB in the reset chapter.
Add the editors of Smepmp (me and Joe Xie) to the list of contributors.