Patch Set 4:

Patch Set 4:

Patch Set 4:

Patch Set 4: Code-Review-1

(1 comment)

For security reasons, the page tables used in SMM must be located in SMRAM.

Please explain your threat model and why it is a security issue if the page table are not in SMRAM.

I imagine it is because th SMM page tables can then be accessed from outside SMM, which could be exploited to escalate privileges to SMM. Which would not be fun.

As page tables currently reside in ROM, it's as secure as the remaining firmware code. If someone is able to change that, the one already has full control over the system on next reboot, so caring about a firmware substate here is futile.

I'll add that to the documentation as TODO, but it's not worth touching the SMM relocation code for a PoC that only runs on emulated hardware.

Though the ROM based page tables are immutable, the SMM page tables should be specific to the code the runs in SMM. I've pulled and rephrased to coreboot needs a couple of the requirements from the Microsoft "System Guard Secure Launch and SMM protection" (https://docs.microsoft.com/en-us/windows/security/threat-protection/windows-defender-system-guard/system-guard-secure-launch-and-smm-protection) to illustrate:

(1) Must not have execute and write permissions for the same page.
(2) Must allow only that TSEG pages can be marked executable.

The short requirement here is make sure that SMM code cannot execute outside of SMRAM.

Another issue with the ROM based pages tables (on x86 processors) is the processor will try to write to page tables because it wants to set the A and D bits (access and dirty). Early in the XHIM development on STM/PE, I tried simulating ROM by setting the EPT pages to R/O. The result was a EPT violation because the processor wanted to write to the page table.

The method to mitigate this is to preset the A and D bits in the page table so that the processor will not attempt to write to the page tables. (http://vzimmer.blogspot.com/2013/02/32-versus-64-bit-and-measuring-uefi.html)

I checked how the page tables are generated (pgtblgen.c) and the A and D bite are not set and I would suggest the code be modified to set those bits.

View Change

To view, visit change 37392. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I26300492e4be62ddd5d80525022c758a019d63a1
Gerrit-Change-Number: 37392
Gerrit-PatchSet: 4
Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-Reviewer: Eugene Myers <cedarhouse1@comcast.net>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus@gmail.com>
Gerrit-CC: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-CC: Patrick Rudolph
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-CC: ron minnich <rminnich@gmail.com>
Gerrit-Comment-Date: Tue, 23 Jun 2020 19:53:00 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment