[edk2-devel] [PATCH 0/5] OvmfPkg: improve SMM comms security with adaptive MemoryTypeInformation -- push#2 #442
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The only two OvmfPkg references to "PcdFlashNvStorageVariableBase" are the spurious ones in the runtime DXE driver and the SMM driver INF files of the QEMU flash driver. Remove these references. The flash driver does not access "PcdOvmfFlashNvStorageEventLogBase" either, so remove that from the INF files too. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=386 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200310222739.26717-2-lersek@redhat.com> Acked-by: Leif Lindholm <leif@nuviainc.com>
…esses Extract the dynamic setting of the - PcdFlashNvStorageVariableBase64 - PcdFlashNvStorageFtwWorkingBase - PcdFlashNvStorageFtwSpareBase addresses to a helper function. For now, the helper function is identical (duplicated) between the SMM flash driver and the runtime DXE flash driver. In subsequent patches, this will change. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=386 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200310222739.26717-3-lersek@redhat.com> Acked-by: Leif Lindholm <leif@nuviainc.com>
The following flash-related base addresses: - PcdFlashNvStorageVariableBase64, - PcdFlashNvStorageFtwWorkingBase, - PcdFlashNvStorageFtwSpareBase, are always set to constant (invariable) values in the "-D SMM_REQUIRE" build of OVMF. (That's because in the SMM build, actual pflash is a hard requirement, and the RAM-based emulation is never available.) Set said PCDs statically, at build. This will allow us to depend on their values in the PEI phase. When SMM_REQUIRE is FALSE, this change has no effect (confirmed by report file comparison). When SMM_REQUIRE is TRUE, the report file shows the following changes: - "PcdOvmfFlashNvStorageFtwSpareBase" and "PcdOvmfFlashNvStorageFtwWorkingBase" are no longer consumed by any module directly, - for "PcdFlashNvStorageFtwSpareBase", "PcdFlashNvStorageFtwWorkingBase" and "PcdFlashNvStorageVariableBase64", the access method changes from DYN to FIXED, - for the latter PCDs, the zero (dynamic default) values are replaced with the desired constants. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=386 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200310222739.26717-4-lersek@redhat.com> Acked-by: Leif Lindholm <leif@nuviainc.com>
…QUIRE FaultTolerantWritePei consumes: - PcdFlashNvStorageFtwWorkingBase, - PcdFlashNvStorageFtwSpareBase. VariablePei consumes: - PcdFlashNvStorageVariableBase64. Due to the previous patches in this series, the above PCDs are available in the PEI phase, in the SMM_REQUIRE build. FaultTolerantWritePei produces a GUID-ed HOB with FAULT_TOLERANT_WRITE_LAST_WRITE_DATA as contents. It also installs a Null PPI that carries the same gEdkiiFaultTolerantWriteGuid as the HOB. VariablePei depends on the Null PPI mentioned above with a DEPEX, consumes the HOB (which is safe due to the DEPEX), and produces EFI_PEI_READ_ONLY_VARIABLE2_PPI. This enables read-only access to non-volatile UEFI variables in the PEI phase, in the SMM_REQUIRE build. For now, the DxeLoadCore() function in "MdeModulePkg/Core/DxeIplPeim/DxeLoad.c" will not access the "MemoryTypeInformation" variable, because OVMF's PlatformPei always produces the MemoryTypeInformation HOB. (Note: when the boot mode is BOOT_ON_S3_RESUME, PlatformPei doesn't build the HOB, but that's in sync with DxeLoadCore() also not looking for either the HOB or the UEFI variable.) Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=386 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200310222739.26717-5-lersek@redhat.com> Acked-by: Leif Lindholm <leif@nuviainc.com>
* In the Intel whitepaper: --v-- A Tour Beyond BIOS -- Secure SMM Communication https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Security-White-Papers https://github.com/tianocore-docs/Docs/raw/master/White_Papers/A_Tour_Beyond_BIOS_Secure_SMM_Communication.pdf --^-- bullet#3 in section "Assumption and Recommendation", and bullet#4 in "Call for action", recommend enabling the (adaptive) Memory Type Information feature. * In the Intel whitepaper: --v-- A Tour Beyond BIOS -- Memory Map and Practices in UEFI BIOS https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-white-papers https://github.com/tianocore-docs/Docs/raw/master/White_Papers/A_Tour_Beyond_BIOS_Memory_Map_And_Practices_in_UEFI_BIOS_V2.pdf --^-- figure#6 describes the Memory Type Information feature in detail; namely as a feedback loop between the Platform PEIM, the DXE IPL PEIM, the DXE Core, and BDS. Implement the missing PlatformPei functionality in OvmfPkg, for fulfilling the Secure SMM Communication recommendation. In the longer term, OVMF should install the WSMT ACPI table, and this patch contributes to that. Notes: - the step in figure#6 where the UEFI variable is copied into the HOB is covered by the DXE IPL PEIM, in the DxeLoadCore() function, - "PcdResetOnMemoryTypeInformationChange" must be reverted to the DEC default TRUE value, because both whitepapers indicate that BDS needs to reset the system if the Memory Type Information changes. Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=386 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200310222739.26717-6-lersek@redhat.com> Acked-by: Leif Lindholm <leif@nuviainc.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://bugzilla.tianocore.org/show_bug.cgi?id=386
https://edk2.groups.io/g/devel/message/55726
http://mid.mail-archive.com/20200310222739.26717-1-lersek@redhat.com