Now that the EDK2 tree uses GetMemoryProtectionsLib to query
the platform memory protection settings, we can add additional
profiles to SetMemoryProtectionsLib to give plaforms more options
for setting memory protections.
Signed-off-by: Taylor Beebe <taylor.d.beebe@gmail.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
---
MdeModulePkg/Library/SetMemoryProtectionsLib/SetMemoryProtectionsLib.c | 417 +++++++++++++++++++-
MdeModulePkg/Include/Library/SetMemoryProtectionsLib.h | 7 +
2 files changed, 422 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Library/SetMemoryProtectionsLib/SetMemoryProtectionsLib.c b/MdeModulePkg/Library/SetMemoryProtectionsLib/SetMemoryProtectionsLib.c
index 13032ec80fbf..5f054504b75e 100644
--- a/MdeModulePkg/Library/SetMemoryProtectionsLib/SetMemoryProtectionsLib.c
+++ b/MdeModulePkg/Library/SetMemoryProtectionsLib/SetMemoryProtectionsLib.c
@@ -28,6 +28,227 @@ typedef struct {
// DXE PROFILE DEFINITIONS //
/////////////////////////////
+//
+// A memory profile with strict settings ideal for development scenarios.
+//
+#define DXE_MEMORY_PROTECTION_SETTINGS_DEBUG \
+{ \
+ DXE_MEMORY_PROTECTION_SIGNATURE, \
+ DXE_MEMORY_PROTECTION_SETTINGS_CURRENT_VERSION, \
+ TRUE, /* Stack Guard */ \
+ TRUE, /* Stack Execution Protection */ \
+ { /* NULL Pointer Detection */ \
+ .Enabled = TRUE, \
+ .DisableEndOfDxe = FALSE, \
+ .NonstopModeEnabled = TRUE \
+ }, \
+ { /* Image Protection */ \
+ .ProtectImageFromUnknown = TRUE, \
+ .ProtectImageFromFv = TRUE \
+ }, \
+ { /* Execution Protection */ \
+ .EnabledForType = { \
+ [EfiReservedMemoryType] = TRUE, \
+ [EfiLoaderCode] = FALSE, \
+ [EfiLoaderData] = TRUE, \
+ [EfiBootServicesCode] = FALSE, \
+ [EfiBootServicesData] = TRUE, \
+ [EfiRuntimeServicesCode] = FALSE, \
+ [EfiRuntimeServicesData] = TRUE, \
+ [EfiConventionalMemory] = TRUE, \
+ [EfiUnusableMemory] = TRUE, \
+ [EfiACPIReclaimMemory] = TRUE, \
+ [EfiACPIMemoryNVS] = TRUE, \
+ [EfiMemoryMappedIO] = TRUE, \
+ [EfiMemoryMappedIOPortSpace] = TRUE, \
+ [EfiPalCode] = TRUE, \
+ [EfiPersistentMemory] = FALSE, \
+ [EfiUnacceptedMemoryType] = TRUE, \
+ [OEM_RESERVED_MPS_MEMORY_TYPE] = TRUE, \
+ [OS_RESERVED_MPS_MEMORY_TYPE] = TRUE \
+ } \
+ }, \
+ { /* Heap Guard */ \
+ .PageGuardEnabled = TRUE, \
+ .PoolGuardEnabled = TRUE, \
+ .FreedMemoryGuardEnabled = FALSE, \
+ .NonstopModeEnabled = TRUE, \
+ .GuardAlignedToTail = TRUE \
+ }, \
+ { /* Pool Guard */ \
+ .EnabledForType = { \
+ [EfiReservedMemoryType] = TRUE, \
+ [EfiLoaderCode] = TRUE, \
+ [EfiLoaderData] = TRUE, \
+ [EfiBootServicesCode] = TRUE, \
+ [EfiBootServicesData] = TRUE, \
+ [EfiRuntimeServicesCode] = TRUE, \
+ [EfiRuntimeServicesData] = TRUE, \
+ [EfiConventionalMemory] = FALSE, \
+ [EfiUnusableMemory] = TRUE, \
+ [EfiACPIReclaimMemory] = TRUE, \
+ [EfiACPIMemoryNVS] = TRUE, \
+ [EfiMemoryMappedIO] = TRUE, \
+ [EfiMemoryMappedIOPortSpace] = TRUE, \
+ [EfiPalCode] = TRUE, \
+ [EfiPersistentMemory] = FALSE, \
+ [EfiUnacceptedMemoryType] = TRUE, \
+ [OEM_RESERVED_MPS_MEMORY_TYPE] = TRUE, \
+ [OS_RESERVED_MPS_MEMORY_TYPE] = TRUE \
+ } \
+ }, \
+ { /* Page Guard */ \
+ .EnabledForType = { \
+ [EfiReservedMemoryType] = TRUE, \
+ [EfiLoaderCode] = TRUE, \
+ [EfiLoaderData] = TRUE, \
+ [EfiBootServicesCode] = TRUE, \
+ [EfiBootServicesData] = TRUE, \
+ [EfiRuntimeServicesCode] = TRUE, \
+ [EfiRuntimeServicesData] = TRUE, \
+ [EfiConventionalMemory] = FALSE, \
+ [EfiUnusableMemory] = TRUE, \
+ [EfiACPIReclaimMemory] = TRUE, \
+ [EfiACPIMemoryNVS] = TRUE, \
+ [EfiMemoryMappedIO] = TRUE, \
+ [EfiMemoryMappedIOPortSpace] = TRUE, \
+ [EfiPalCode] = TRUE, \
+ [EfiPersistentMemory] = FALSE, \
+ [EfiUnacceptedMemoryType] = TRUE, \
+ [OEM_RESERVED_MPS_MEMORY_TYPE] = TRUE, \
+ [OS_RESERVED_MPS_MEMORY_TYPE] = TRUE \
+ } \
+ } \
+}
+
+//
+// A memory profile recommended for production. Compared to the debug
+// settings, this profile removes the pool guards and uses page guards
+// for fewer memory types.
+//
+#define DXE_MEMORY_PROTECTION_SETTINGS_PROD_MODE \
+{ \
+ DXE_MEMORY_PROTECTION_SIGNATURE, \
+ DXE_MEMORY_PROTECTION_SETTINGS_CURRENT_VERSION, \
+ TRUE, /* Stack Guard */ \
+ TRUE, /* Stack Execution Protection */ \
+ { /* NULL Pointer Detection */ \
+ .Enabled = TRUE, \
+ .DisableEndOfDxe = FALSE, \
+ .NonstopModeEnabled = FALSE \
+ }, \
+ { /* Image Protection */ \
+ .ProtectImageFromUnknown = FALSE, \
+ .ProtectImageFromFv = TRUE \
+ }, \
+ { /* Execution Protection */ \
+ .EnabledForType = { \
+ [EfiReservedMemoryType] = TRUE, \
+ [EfiLoaderCode] = FALSE, \
+ [EfiLoaderData] = TRUE, \
+ [EfiBootServicesCode] = FALSE, \
+ [EfiBootServicesData] = TRUE, \
+ [EfiRuntimeServicesCode] = FALSE, \
+ [EfiRuntimeServicesData] = TRUE, \
+ [EfiConventionalMemory] = TRUE, \
+ [EfiUnusableMemory] = TRUE, \
+ [EfiACPIReclaimMemory] = TRUE, \
+ [EfiACPIMemoryNVS] = TRUE, \
+ [EfiMemoryMappedIO] = TRUE, \
+ [EfiMemoryMappedIOPortSpace] = TRUE, \
+ [EfiPalCode] = TRUE, \
+ [EfiPersistentMemory] = FALSE, \
+ [EfiUnacceptedMemoryType] = TRUE, \
+ [OEM_RESERVED_MPS_MEMORY_TYPE] = TRUE, \
+ [OS_RESERVED_MPS_MEMORY_TYPE] = TRUE \
+ } \
+ }, \
+ { /* Heap Guard */ \
+ .PageGuardEnabled = TRUE, \
+ .PoolGuardEnabled = FALSE, \
+ .FreedMemoryGuardEnabled = FALSE, \
+ .NonstopModeEnabled = FALSE, \
+ .GuardAlignedToTail = TRUE \
+ }, \
+ { /* Pool Guard */ \
+ 0 \
+ }, \
+ { /* Page Guard */ \
+ .EnabledForType = { \
+ [EfiReservedMemoryType] = FALSE, \
+ [EfiLoaderCode] = FALSE, \
+ [EfiLoaderData] = FALSE, \
+ [EfiBootServicesCode] = FALSE, \
+ [EfiBootServicesData] = TRUE, \
+ [EfiRuntimeServicesCode] = FALSE, \
+ [EfiRuntimeServicesData] = TRUE, \
+ [EfiConventionalMemory] = FALSE, \
+ [EfiUnusableMemory] = FALSE, \
+ [EfiACPIReclaimMemory] = FALSE, \
+ [EfiACPIMemoryNVS] = FALSE, \
+ [EfiMemoryMappedIO] = FALSE, \
+ [EfiMemoryMappedIOPortSpace] = FALSE, \
+ [EfiPalCode] = FALSE, \
+ [EfiPersistentMemory] = FALSE, \
+ [EfiUnacceptedMemoryType] = FALSE, \
+ [OEM_RESERVED_MPS_MEMORY_TYPE] = FALSE, \
+ [OS_RESERVED_MPS_MEMORY_TYPE] = FALSE \
+ } \
+ } \
+}
+
+//
+// A memory profile which mirrors DXE_MEMORY_PROTECTION_SETTINGS_PROD_MODE
+// but doesn't include page guards.
+//
+#define DXE_MEMORY_PROTECTION_SETTINGS_PROD_MODE_NO_PAGE_GUARDS \
+{ \
+ DXE_MEMORY_PROTECTION_SIGNATURE, \
+ DXE_MEMORY_PROTECTION_SETTINGS_CURRENT_VERSION, \
+ TRUE, /* Stack Guard */ \
+ TRUE, /* Stack Execution Protection */ \
+ { /* NULL Pointer Detection */ \
+ .Enabled = TRUE, \
+ .DisableEndOfDxe = FALSE, \
+ .NonstopModeEnabled = FALSE \
+ }, \
+ { /* Image Protection */ \
+ .ProtectImageFromUnknown = FALSE, \
+ .ProtectImageFromFv = TRUE \
+ }, \
+ { /* Execution Protection */ \
+ .EnabledForType = { \
+ [EfiReservedMemoryType] = TRUE, \
+ [EfiLoaderCode] = FALSE, \
+ [EfiLoaderData] = TRUE, \
+ [EfiBootServicesCode] = FALSE, \
+ [EfiBootServicesData] = TRUE, \
+ [EfiRuntimeServicesCode] = FALSE, \
+ [EfiRuntimeServicesData] = TRUE, \
+ [EfiConventionalMemory] = TRUE, \
+ [EfiUnusableMemory] = TRUE, \
+ [EfiACPIReclaimMemory] = TRUE, \
+ [EfiACPIMemoryNVS] = TRUE, \
+ [EfiMemoryMappedIO] = TRUE, \
+ [EfiMemoryMappedIOPortSpace] = TRUE, \
+ [EfiPalCode] = TRUE, \
+ [EfiPersistentMemory] = FALSE, \
+ [EfiUnacceptedMemoryType] = TRUE, \
+ [OEM_RESERVED_MPS_MEMORY_TYPE] = TRUE, \
+ [OS_RESERVED_MPS_MEMORY_TYPE] = TRUE \
+ } \
+ }, \
+ { /* Heap Guard */ \
+ 0 \
+ }, \
+ { /* Pool Guard */ \
+ 0 \
+ }, \
+ { /* Page Guard */ \
+ 0 \
+ } \
+}
+
//
// A memory profile which uses the fixed at build PCDs defined in MdeModulePkg.dec
//
@@ -121,10 +342,146 @@ typedef struct {
} \
}
+//
+// A memory profile which disables all DXE memory protection settings.
+//
+#define DXE_MEMORY_PROTECTION_SETTINGS_OFF \
+{ \
+ DXE_MEMORY_PROTECTION_SIGNATURE, \
+ DXE_MEMORY_PROTECTION_SETTINGS_CURRENT_VERSION, \
+ FALSE, /* Stack Guard */ \
+ FALSE, /* Stack Execution Protection */ \
+ { /* NULL Pointer Detection */ \
+ 0 \
+ }, \
+ { /* Image Protection */ \
+ 0 \
+ }, \
+ { /* Execution Protection */ \
+ 0 \
+ }, \
+ { /* Heap Guard */ \
+ 0 \
+ }, \
+ { /* Pool Guard */ \
+ 0 \
+ }, \
+ { /* Page Guard */ \
+ 0 \
+ } \
+}
+
////////////////////////////
// MM PROFILE DEFINITIONS //
////////////////////////////
+//
+// A memory profile ideal for development scenarios.
+//
+#define MM_MEMORY_PROTECTION_SETTINGS_DEBUG \
+{ \
+ MM_MEMORY_PROTECTION_SIGNATURE, \
+ MM_MEMORY_PROTECTION_SETTINGS_CURRENT_VERSION, \
+ { /* NULL Pointer Detection */ \
+ .Enabled = TRUE, \
+ .NonstopModeEnabled = TRUE \
+ }, \
+ { /* Heap Guard */ \
+ .PageGuardEnabled = TRUE, \
+ .PoolGuardEnabled = TRUE, \
+ .NonstopModeEnabled = TRUE, \
+ .GuardAlignedToTail = TRUE \
+ }, \
+ { /* Pool Guard */ \
+ .EnabledForType = { \
+ [EfiReservedMemoryType] = FALSE, \
+ [EfiLoaderCode] = FALSE, \
+ [EfiLoaderData] = FALSE, \
+ [EfiBootServicesCode] = FALSE, \
+ [EfiBootServicesData] = TRUE, \
+ [EfiRuntimeServicesCode] = FALSE, \
+ [EfiRuntimeServicesData] = TRUE, \
+ [EfiConventionalMemory] = FALSE, \
+ [EfiUnusableMemory] = FALSE, \
+ [EfiACPIReclaimMemory] = FALSE, \
+ [EfiACPIMemoryNVS] = FALSE, \
+ [EfiMemoryMappedIO] = FALSE, \
+ [EfiMemoryMappedIOPortSpace] = FALSE, \
+ [EfiPalCode] = FALSE, \
+ [EfiPersistentMemory] = FALSE, \
+ [EfiUnacceptedMemoryType] = FALSE, \
+ [OEM_RESERVED_MPS_MEMORY_TYPE] = FALSE, \
+ [OS_RESERVED_MPS_MEMORY_TYPE] = FALSE \
+ } \
+ }, \
+ { /* Page Guard */ \
+ .EnabledForType = { \
+ [EfiReservedMemoryType] = FALSE, \
+ [EfiLoaderCode] = FALSE, \
+ [EfiLoaderData] = FALSE, \
+ [EfiBootServicesCode] = FALSE, \
+ [EfiBootServicesData] = TRUE, \
+ [EfiRuntimeServicesCode] = FALSE, \
+ [EfiRuntimeServicesData] = TRUE, \
+ [EfiConventionalMemory] = FALSE, \
+ [EfiUnusableMemory] = FALSE, \
+ [EfiACPIReclaimMemory] = FALSE, \
+ [EfiACPIMemoryNVS] = FALSE, \
+ [EfiMemoryMappedIO] = FALSE, \
+ [EfiMemoryMappedIOPortSpace] = FALSE, \
+ [EfiPalCode] = FALSE, \
+ [EfiPersistentMemory] = FALSE, \
+ [EfiUnacceptedMemoryType] = FALSE, \
+ [OEM_RESERVED_MPS_MEMORY_TYPE] = FALSE, \
+ [OS_RESERVED_MPS_MEMORY_TYPE] = FALSE \
+ } \
+ } \
+}
+
+//
+// A memory profile ideal for production scenarios.
+//
+#define MM_MEMORY_PROTECTION_SETTINGS_PROD_MODE \
+{ \
+ MM_MEMORY_PROTECTION_SIGNATURE, \
+ MM_MEMORY_PROTECTION_SETTINGS_CURRENT_VERSION, \
+ { /* NULL Pointer Detection */ \
+ .Enabled = TRUE, \
+ .NonstopModeEnabled = FALSE \
+ }, \
+ { /* Heap Guard */ \
+ .PageGuardEnabled = TRUE, \
+ .PoolGuardEnabled = FALSE, \
+ .NonstopModeEnabled = FALSE, \
+ .GuardAlignedToTail = TRUE \
+ }, \
+ { /* Pool Guard */ \
+ 0 \
+ }, \
+ { /* Page Guard */ \
+ .EnabledForType = { \
+ [EfiReservedMemoryType] = FALSE, \
+ [EfiLoaderCode] = FALSE, \
+ [EfiLoaderData] = FALSE, \
+ [EfiBootServicesCode] = FALSE, \
+ [EfiBootServicesData] = TRUE, \
+ [EfiRuntimeServicesCode] = FALSE, \
+ [EfiRuntimeServicesData] = TRUE, \
+ [EfiConventionalMemory] = FALSE, \
+ [EfiUnusableMemory] = FALSE, \
+ [EfiACPIReclaimMemory] = FALSE, \
+ [EfiACPIMemoryNVS] = FALSE, \
+ [EfiMemoryMappedIO] = FALSE, \
+ [EfiMemoryMappedIOPortSpace] = FALSE, \
+ [EfiPalCode] = FALSE, \
+ [EfiPersistentMemory] = FALSE, \
+ [EfiUnacceptedMemoryType] = FALSE, \
+ [OEM_RESERVED_MPS_MEMORY_TYPE] = FALSE, \
+ [OS_RESERVED_MPS_MEMORY_TYPE] = FALSE \
+ } \
+ } \
+}
+
//
// A memory profile which uses the fixed at build PCDs defined in MdeModulePkg.dec
//
@@ -188,24 +545,80 @@ typedef struct {
} \
}
+//
+// A memory profile which disables all MM memory protection settings.
+//
+#define MM_MEMORY_PROTECTION_SETTINGS_OFF \
+{ \
+ MM_MEMORY_PROTECTION_SIGNATURE, \
+ MM_MEMORY_PROTECTION_SETTINGS_CURRENT_VERSION, \
+ { /* NULL Pointer Detection */ \
+ 0 \
+ }, \
+ { /* Heap Guard */ \
+ 0 \
+ }, \
+ { /* Pool Guard */ \
+ 0 \
+ }, \
+ { /* Page Guard */ \
+ 0 \
+ } \
+}
+
////////////////////////////
// PROFILE CONFIGURATIONS //
////////////////////////////
DXE_MEMORY_PROTECTION_PROFILES DxeMemoryProtectionProfiles[DxeMemoryProtectionSettingsMax] = {
- [DxeMemoryProtectionSettingsPcd] = {
+ [DxeMemoryProtectionSettingsDebug] = {
+ .Name = "Debug",
+ .Description = "Development profile ideal for debug scenarios",
+ .Settings = DXE_MEMORY_PROTECTION_SETTINGS_DEBUG
+ },
+ [DxeMemoryProtectionSettingsRelease] = {
+ .Name = "Release",
+ .Description = "Release profile recommended for production scenarios",
+ .Settings = DXE_MEMORY_PROTECTION_SETTINGS_PROD_MODE
+ },
+ [DxeMemoryProtectionSettingsReleaseNoPageGuards] = {
+ .Name = "ReleaseNoPageGuards",
+ .Description = "Release profile without page guards recommended for performance sensitive production scenarios",
+ .Settings = DXE_MEMORY_PROTECTION_SETTINGS_PROD_MODE_NO_PAGE_GUARDS
+ },
+ [DxeMemoryProtectionSettingsPcd] = {
.Name = "Pcd",
.Description = "Memory protection settings from PCDs",
.Settings = DXE_MEMORY_PROTECTION_SETTINGS_PCD
},
+ [DxeMemoryProtectionSettingsOff] = {
+ .Name = "Off",
+ .Description = "Disables all memory protection settings",
+ .Settings = DXE_MEMORY_PROTECTION_SETTINGS_OFF
+ }
};
MM_MEMORY_PROTECTION_PROFILES MmMemoryProtectionProfiles[MmMemoryProtectionSettingsMax] = {
- [MmMemoryProtectionSettingsPcd] = {
+ [MmMemoryProtectionSettingsDebug] = {
+ .Name = "Debug",
+ .Description = "Development profile ideal for debug scenarios",
+ .Settings = MM_MEMORY_PROTECTION_SETTINGS_DEBUG
+ },
+ [MmMemoryProtectionSettingsRelease] = {
+ .Name = "Release",
+ .Description = "Release profile recommended for production scenarios",
+ .Settings = MM_MEMORY_PROTECTION_SETTINGS_PROD_MODE
+ },
+ [MmMemoryProtectionSettingsPcd] = {
.Name = "Pcd",
.Description = "Memory protection settings from PCDs",
.Settings = MM_MEMORY_PROTECTION_SETTINGS_PCD
},
+ [MmMemoryProtectionSettingsOff] = {
+ .Name = "Off",
+ .Description = "Disables all memory protection settings",
+ .Settings = MM_MEMORY_PROTECTION_SETTINGS_OFF
+ }
};
/////////////////////////////////////
diff --git a/MdeModulePkg/Include/Library/SetMemoryProtectionsLib.h b/MdeModulePkg/Include/Library/SetMemoryProtectionsLib.h
index 023c987c3c7e..f4665130b0b3 100644
--- a/MdeModulePkg/Include/Library/SetMemoryProtectionsLib.h
+++ b/MdeModulePkg/Include/Library/SetMemoryProtectionsLib.h
@@ -17,6 +17,10 @@ typedef struct {
} DXE_MEMORY_PROTECTION_PROFILES;
typedef enum {
+ DxeMemoryProtectionSettingsDebug = 0,
+ DxeMemoryProtectionSettingsRelease,
+ DxeMemoryProtectionSettingsReleaseNoPageGuards,
+ DxeMemoryProtectionSettingsOff,
DxeMemoryProtectionSettingsPcd,
DxeMemoryProtectionSettingsMax
} DXE_MEMORY_PROTECTION_PROFILE_INDEX;
@@ -28,6 +32,9 @@ typedef struct {
} MM_MEMORY_PROTECTION_PROFILES;
typedef enum {
+ MmMemoryProtectionSettingsDebug = 0,
+ MmMemoryProtectionSettingsRelease,
+ MmMemoryProtectionSettingsOff,
MmMemoryProtectionSettingsPcd,
MmMemoryProtectionSettingsMax
} MM_MEMORY_PROTECTION_PROFILE_INDEX;
--
2.42.0.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108882): https://edk2.groups.io/g/devel/message/108882
Mute This Topic: https://groups.io/mt/101469960/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-On Tue, Sep 19, 2023 at 05:57:43PM -0700, Taylor Beebe wrote: > Now that the EDK2 tree uses GetMemoryProtectionsLib to query > the platform memory protection settings, we can add additional > profiles to SetMemoryProtectionsLib to give plaforms more options > for setting memory protections. What is the recommended way to add more profiles? Specifically I have a bunch of linux test cases failing when testing this series, which is most likely causes by older + broken grub versions (which are known to use EfiLoaderData for code). So I think I need a "GrubCompat" profile which has ExecutionProtection.EnabledForType[EfiLoaderData] = FALSE but is otherwise identical to the production profile. Should that go into SetMemoryProtectionsLib? Or MemoryProtectionConfigLib? take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109113): https://edk2.groups.io/g/devel/message/109113 Mute This Topic: https://groups.io/mt/101469960/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Sorry for the slow reply :) Additional profiles which fit general use cases can be added to SetMemoryProtectionsLib, but because this is a profile for grub compatibility I'd say it's better suited for platform code making MemoryProtectionConfigLib in OvmfPkg the best spot. I'll add an additional static profile array to MemoryProtectionConfigLib and have logic loop through both to see if a profile matches. I'll add the GrubCompat profile you outlined to this new profile array. I can also update ArmVirtPkg to disable execution protection for EfiLoaderData by default until fw_cfg parsing support is added to ArmVirtPkg. Let me know if you think this is necessary. Thanks for the feedback :) -Taylor On 9/27/23 1:19 AM, Gerd Hoffmann wrote: > On Tue, Sep 19, 2023 at 05:57:43PM -0700, Taylor Beebe wrote: >> Now that the EDK2 tree uses GetMemoryProtectionsLib to query >> the platform memory protection settings, we can add additional >> profiles to SetMemoryProtectionsLib to give plaforms more options >> for setting memory protections. > What is the recommended way to add more profiles? > > Specifically I have a bunch of linux test cases failing when testing > this series, which is most likely causes by older + broken grub versions > (which are known to use EfiLoaderData for code). > > So I think I need a "GrubCompat" profile which has > ExecutionProtection.EnabledForType[EfiLoaderData] = FALSE > but is otherwise identical to the production profile. > > Should that go into SetMemoryProtectionsLib? > Or MemoryProtectionConfigLib? > > take care, > Gerd > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109215): https://edk2.groups.io/g/devel/message/109215 Mute This Topic: https://groups.io/mt/101469960/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Fri, Sep 29, 2023 at 12:52:35PM -0700, Taylor Beebe wrote: > Sorry for the slow reply :) > > > Additional profiles which fit general use cases can be added to > SetMemoryProtectionsLib, but because this is a profile for grub > compatibility I'd say it's better suited for platform code making > MemoryProtectionConfigLib in OvmfPkg the best spot. > I'll add an additional static profile array to MemoryProtectionConfigLib > and have logic loop through both to see if a profile matches. I'll > add the GrubCompat profile you outlined to this new profile array. Sounds good to me. > I can also update ArmVirtPkg to disable execution protection > for EfiLoaderData by default until fw_cfg parsing > support is added to ArmVirtPkg. Let me know if you think > this is necessary. With MemoryProtectionConfigLib adding fw_cfg parsing support to ArmVirtPkg should be easy, so maybe just do that? take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109310): https://edk2.groups.io/g/devel/message/109310 Mute This Topic: https://groups.io/mt/101469960/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 10/4/23 1:46 AM, Gerd Hoffmann wrote: > On Fri, Sep 29, 2023 at 12:52:35PM -0700, Taylor Beebe wrote: >> I can also update ArmVirtPkg to disable execution protection >> for EfiLoaderData by default until fw_cfg parsing >> support is added to ArmVirtPkg. Let me know if you think >> this is necessary. > With MemoryProtectionConfigLib adding fw_cfg parsing support to > ArmVirtPkg should be easy, so maybe just do that? From what I see, the QemuFwCfgLib instance compatible with Arm requires UefiBootServicesTableLib so fw_cfg cannot be parsed early enough to set the memory protection policy on ArmVirt. An Arm compatible PEIM instance of QemuFwCfgLib will need to be created. I'm happy to look into it, but I don't want to hang up this patch series on that addition. Instead, I'll set the protection policy for ArmVirtPkg to the equivalent of the new GrubCompat profile in this series. Please let me know if I'm missing an obvious route to PEI fw_cfg parsing on Arm. -Taylor -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109328): https://edk2.groups.io/g/devel/message/109328 Mute This Topic: https://groups.io/mt/101469960/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 10/4/23 18:31, Taylor Beebe wrote: > > On 10/4/23 1:46 AM, Gerd Hoffmann wrote: >> On Fri, Sep 29, 2023 at 12:52:35PM -0700, Taylor Beebe wrote: >>> I can also update ArmVirtPkg to disable execution protection >>> for EfiLoaderData by default until fw_cfg parsing >>> support is added to ArmVirtPkg. Let me know if you think >>> this is necessary. >> With MemoryProtectionConfigLib adding fw_cfg parsing support to >> ArmVirtPkg should be easy, so maybe just do that? > > From what I see, the QemuFwCfgLib instance compatible with Arm requires > UefiBootServicesTableLib so fw_cfg cannot be parsed early enough to set > the memory protection policy on ArmVirt. QemuFwCfgLibMmio is DXE_DRIVER and UEFI_DRIVER only; it depends on (a) the FDT client protocol, (b) MMIO accesses (that is, page tables). On x86, PEI can use IO ports (no page tables), but that's not available on ARM. I don't recall off-hand where / when exactly page tables are set up during ArmVirtQemu boot. You'd probably have to do something similar to "ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c" and "ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c" -- parse the FDT on every fw_cfg access for the MMIO registers, then use those registers to fetch the profile name, and do all that early enough to configure the page protections -- so possibly *before*, or as a part of, creating the page tables in the first place. > An Arm compatible PEIM instance of QemuFwCfgLib will need to be created. > I'm happy to look into it, but I don't want to hang up this patch series on > that addition. Instead, I'll set the protection policy for ArmVirtPkg to > the equivalent of the new GrubCompat profile in this series. Can you base the default policy (i.e., the one that takes effect in the absence of fw_cfg) on a PCD? Such a PCD could be fixed-at-build, but I think it might even be DynamicHII (compare commit 7e5f1b673870, "ArmVirtPkg/PlatformHasAcpiDtDxe: allow guest level ACPI disable override", 2017-03-31). That would have the benefit that people could set a default at build time, with the --pcd build option. With DynamicHII, the default could be stored in a non-volatile UEFI variable, and ArmVirtQemu does have PEI-time (read-only) variable access. Well, one argument against DynamicHII is that you'd want to prevent later phases of the firmware from overwriting that variable, so you'd have to get into variable policy / locking whatnot. So I'd suggest picking the default profile from a fixed-at-build PCD (impossible to overwrite "from the inside", and still enables the build-time --pcd switch, for setting the platform default), *plus* fw-cfg for dynamic configuration. (Sorry I don't know anything about your series; it's possible you already set the platform default via PCDs.) I think platform builders should have the choice of picking a default profile at build time; neither the Release (?) nor the GrubCompat profile will fit everyone. I agree the *DEC* default should be the strictest, but --pcd should be able to override that at build time, even if -fw_cfg will allow for totally dynamic configuration too. Laszlo > Please let me know if I'm missing an obvious route to PEI fw_cfg > parsing on Arm. > > > -Taylor > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109344): https://edk2.groups.io/g/devel/message/109344 Mute This Topic: https://groups.io/mt/101469960/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi, > On x86, PEI can use IO ports (no page tables), but that's not available > on ARM. I don't recall off-hand where / when exactly page tables are set > up during ArmVirtQemu boot. It also changed roughly one year ago, Ard updated the arm platform setup to turn on paging much earlier (for w^x and IIRC also fix some caching problems). I think PEI runs with paging already enabled these days. > > An Arm compatible PEIM instance of QemuFwCfgLib will need to be created. > > I'm happy to look into it, but I don't want to hang up this patch series on > > that addition. Instead, I'll set the protection policy for ArmVirtPkg to > > the equivalent of the new GrubCompat profile in this series. > > Can you base the default policy (i.e., the one that takes effect in the > absence of fw_cfg) on a PCD? That would be nice indeed. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109346): https://edk2.groups.io/g/devel/message/109346 Mute This Topic: https://groups.io/mt/101469960/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi, > > > An Arm compatible PEIM instance of QemuFwCfgLib will need to be created. > > > I'm happy to look into it, but I don't want to hang up this patch series on > > > that addition. Instead, I'll set the protection policy for ArmVirtPkg to > > > the equivalent of the new GrubCompat profile in this series. > > > > Can you base the default policy (i.e., the one that takes effect in the > > absence of fw_cfg) on a PCD? > > That would be nice indeed. While being at it: Does it make sense to have *two* defaults, one for secureboot=on (strict) and one for secureboot=off (compat) ? take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109349): https://edk2.groups.io/g/devel/message/109349 Mute This Topic: https://groups.io/mt/101469960/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 10/5/23 12:23, Gerd Hoffmann wrote: > Hi, > >>>> An Arm compatible PEIM instance of QemuFwCfgLib will need to be created. >>>> I'm happy to look into it, but I don't want to hang up this patch series on >>>> that addition. Instead, I'll set the protection policy for ArmVirtPkg to >>>> the equivalent of the new GrubCompat profile in this series. >>> >>> Can you base the default policy (i.e., the one that takes effect in the >>> absence of fw_cfg) on a PCD? >> >> That would be nice indeed. > > While being at it: Does it make sense to have *two* defaults, one for > secureboot=on (strict) and one for secureboot=off (compat) ? I'm not sure, for now we can't enforce truly secure secure boot anyway. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109354): https://edk2.groups.io/g/devel/message/109354 Mute This Topic: https://groups.io/mt/101469960/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
I appreciate the suggestions on how to add PEI fw_cfg parsing support -- it should speed up the investigation/implementation. The focus of this series is a more-or-less lateral update from the PCDs to the new interface, and even then this transitional series has grown quite long and still has zero reviewed-bys 3 months in. There are many more functional and test updates to come which can be done in parallel once this series is complete. The currently planned work can be found on the Tianocore Memory Protections project (https://github.com/orgs/tianocore/projects/3). I'll add an item there to add fw_cfg parsing support for PEI ArmVirt and assign myself. Any more task suggestions are welcome :) In this series, I'll add a FixedAtBuild PCD to ArmVirtPkg which will dictate the protection profile used for the boot. The default will be the release profile. I'll also make the other updates mentioned in this thread. -Taylor On 10/5/2023 5:57 AM, Laszlo Ersek wrote: > On 10/5/23 12:23, Gerd Hoffmann wrote: >> Hi, >> >>>>> An Arm compatible PEIM instance of QemuFwCfgLib will need to be created. >>>>> I'm happy to look into it, but I don't want to hang up this patch series on >>>>> that addition. Instead, I'll set the protection policy for ArmVirtPkg to >>>>> the equivalent of the new GrubCompat profile in this series. >>>> Can you base the default policy (i.e., the one that takes effect in the >>>> absence of fw_cfg) on a PCD? >>> That would be nice indeed. >> While being at it: Does it make sense to have *two* defaults, one for >> secureboot=on (strict) and one for secureboot=off (compat) ? > I'm not sure, for now we can't enforce truly secure secure boot anyway. > > Laszlo > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109403): https://edk2.groups.io/g/devel/message/109403 Mute This Topic: https://groups.io/mt/101469960/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.