From 2700df080d0f0857ec00e192b5eeb146f3de0a6a Mon Sep 17 00:00:00 2001 From: Mikhail Krichanov Date: Mon, 16 Sep 2024 17:56:05 +0300 Subject: [PATCH] ArmPkg: Forbade user access to supervisor sections. --- ArmPkg/Include/Chipset/ArmV7Mmu.h | 51 +++++++++---------- ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 2 +- .../Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c | 31 ++++++++--- ArmPkg/Library/CpuArchLib/Arm/Mmu.c | 32 ++++++++---- 4 files changed, 71 insertions(+), 45 deletions(-) diff --git a/ArmPkg/Include/Chipset/ArmV7Mmu.h b/ArmPkg/Include/Chipset/ArmV7Mmu.h index 89b81e33d0..a0af346582 100644 --- a/ArmPkg/Include/Chipset/ArmV7Mmu.h +++ b/ArmPkg/Include/Chipset/ArmV7Mmu.h @@ -97,6 +97,7 @@ #define TT_DESCRIPTOR_PAGE_AF (1UL << 4) #define TT_DESCRIPTOR_SECTION_XN_MASK (0x1UL << 4) +#define TT_DESCRIPTOR_SECTION_PXN_MASK (0x1UL << 0) #define TT_DESCRIPTOR_PAGE_XN_MASK (0x1UL << 0) #define TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK ((3UL << 12) | (1UL << 3) | (1UL << 2)) @@ -136,7 +137,7 @@ #define TT_DESCRIPTOR_SECTION_ATTRIBUTE_MASK (TT_DESCRIPTOR_SECTION_NS_MASK | TT_DESCRIPTOR_SECTION_NG_MASK | \ TT_DESCRIPTOR_SECTION_S_MASK | TT_DESCRIPTOR_SECTION_AP_MASK | \ - TT_DESCRIPTOR_SECTION_AF | \ + TT_DESCRIPTOR_SECTION_AF | TT_DESCRIPTOR_SECTION_PXN_MASK | \ TT_DESCRIPTOR_SECTION_XN_MASK | TT_DESCRIPTOR_SECTION_CACHE_POLICY_MASK) #define TT_DESCRIPTOR_PAGE_ATTRIBUTE_MASK (TT_DESCRIPTOR_PAGE_NG_MASK | TT_DESCRIPTOR_PAGE_S_MASK | \ @@ -161,7 +162,7 @@ TT_DESCRIPTOR_SECTION_NG_GLOBAL | \ TT_DESCRIPTOR_SECTION_S_SHARED | \ TT_DESCRIPTOR_SECTION_DOMAIN(0) | \ - TT_DESCRIPTOR_SECTION_AP_RW_RW | \ + TT_DESCRIPTOR_SECTION_AP_NO_RW | \ TT_DESCRIPTOR_SECTION_AF) #define TT_DESCRIPTOR_SECTION_WRITE_BACK (TT_DESCRIPTOR_SECTION_DEFAULT | \ @@ -176,31 +177,27 @@ #define TT_DESCRIPTOR_SECTION_UNCACHED (TT_DESCRIPTOR_SECTION_DEFAULT | \ TT_DESCRIPTOR_SECTION_CACHE_POLICY_NON_CACHEABLE) -#define TT_DESCRIPTOR_PAGE_WRITE_BACK (TT_DESCRIPTOR_PAGE_TYPE_PAGE | \ - TT_DESCRIPTOR_PAGE_NG_GLOBAL | \ - TT_DESCRIPTOR_PAGE_S_SHARED | \ - TT_DESCRIPTOR_PAGE_AP_RW_RW | \ - TT_DESCRIPTOR_PAGE_AF | \ - TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_BACK_ALLOC) -#define TT_DESCRIPTOR_PAGE_WRITE_THROUGH (TT_DESCRIPTOR_PAGE_TYPE_PAGE | \ - TT_DESCRIPTOR_PAGE_NG_GLOBAL | \ - TT_DESCRIPTOR_PAGE_S_SHARED | \ - TT_DESCRIPTOR_PAGE_AP_RW_RW | \ - TT_DESCRIPTOR_PAGE_AF | \ - TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_THROUGH_NO_ALLOC) -#define TT_DESCRIPTOR_PAGE_DEVICE (TT_DESCRIPTOR_PAGE_TYPE_PAGE | \ - TT_DESCRIPTOR_PAGE_NG_GLOBAL | \ - TT_DESCRIPTOR_PAGE_S_NOT_SHARED | \ - TT_DESCRIPTOR_PAGE_AP_RW_RW | \ - TT_DESCRIPTOR_PAGE_AF | \ - TT_DESCRIPTOR_PAGE_XN_MASK | \ - TT_DESCRIPTOR_PAGE_CACHE_POLICY_SHAREABLE_DEVICE) -#define TT_DESCRIPTOR_PAGE_UNCACHED (TT_DESCRIPTOR_PAGE_TYPE_PAGE | \ - TT_DESCRIPTOR_PAGE_NG_GLOBAL | \ - TT_DESCRIPTOR_PAGE_S_NOT_SHARED | \ - TT_DESCRIPTOR_PAGE_AP_RW_RW | \ - TT_DESCRIPTOR_PAGE_AF | \ - TT_DESCRIPTOR_PAGE_CACHE_POLICY_NON_CACHEABLE) +#define TT_DESCRIPTOR_PAGE_DEFAULT (TT_DESCRIPTOR_PAGE_TYPE_PAGE | \ + TT_DESCRIPTOR_PAGE_NG_GLOBAL | \ + TT_DESCRIPTOR_PAGE_AP_NO_RW | \ + TT_DESCRIPTOR_PAGE_AF) + +#define TT_DESCRIPTOR_PAGE_WRITE_BACK (TT_DESCRIPTOR_PAGE_DEFAULT | \ + TT_DESCRIPTOR_PAGE_S_SHARED | \ + TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_BACK_ALLOC) + +#define TT_DESCRIPTOR_PAGE_WRITE_THROUGH (TT_DESCRIPTOR_PAGE_DEFAULT | \ + TT_DESCRIPTOR_PAGE_S_SHARED | \ + TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_THROUGH_NO_ALLOC) + +#define TT_DESCRIPTOR_PAGE_DEVICE (TT_DESCRIPTOR_PAGE_DEFAULT | \ + TT_DESCRIPTOR_PAGE_S_NOT_SHARED | \ + TT_DESCRIPTOR_PAGE_XN_MASK | \ + TT_DESCRIPTOR_PAGE_CACHE_POLICY_SHAREABLE_DEVICE) + +#define TT_DESCRIPTOR_PAGE_UNCACHED (TT_DESCRIPTOR_PAGE_DEFAULT | \ + TT_DESCRIPTOR_PAGE_S_NOT_SHARED | \ + TT_DESCRIPTOR_PAGE_CACHE_POLICY_NON_CACHEABLE) // First Level Descriptors typedef UINT32 ARM_FIRST_LEVEL_DESCRIPTOR; diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c index 28e4cd9f1a..55ceeaae27 100644 --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c @@ -252,7 +252,7 @@ FillTranslationTable ( break; case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_XP: Attributes = TT_DESCRIPTOR_SECTION_WRITE_BACK; - Attributes |= TT_DESCRIPTOR_SECTION_XN_MASK; + Attributes |= TT_DESCRIPTOR_SECTION_XN_MASK | TT_DESCRIPTOR_SECTION_PXN_MASK; break; case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH: Attributes = TT_DESCRIPTOR_SECTION_WRITE_THROUGH; diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c index 7050052903..8d08ca71e3 100644 --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c @@ -295,14 +295,30 @@ UpdateSectionEntries ( return EFI_UNSUPPORTED; } - if ((Attributes & EFI_MEMORY_RO) != 0) { - EntryValue |= TT_DESCRIPTOR_SECTION_AP_RO_RO; - } else { - EntryValue |= TT_DESCRIPTOR_SECTION_AP_RW_RW; - } + if ((Attributes & EFI_MEMORY_USER) != 0) { + EntryValue |= TT_DESCRIPTOR_SECTION_PXN_MASK; - if ((Attributes & EFI_MEMORY_XP) != 0) { + if ((Attributes & EFI_MEMORY_RO) != 0) { + EntryValue |= TT_DESCRIPTOR_SECTION_AP_RO_RO; + } else { + EntryValue |= TT_DESCRIPTOR_SECTION_AP_RW_RW; + } + + if ((Attributes & EFI_MEMORY_XP) != 0) { + EntryValue |= TT_DESCRIPTOR_SECTION_XN_MASK; + } + } else { EntryValue |= TT_DESCRIPTOR_SECTION_XN_MASK; + + if ((Attributes & EFI_MEMORY_RO) != 0) { + EntryValue |= TT_DESCRIPTOR_SECTION_AP_NO_RO; + } else { + EntryValue |= TT_DESCRIPTOR_SECTION_AP_NO_RW; + } + + if ((Attributes & EFI_MEMORY_XP) != 0) { + EntryValue |= TT_DESCRIPTOR_SECTION_PXN_MASK; + } } if ((Attributes & EFI_MEMORY_RP) == 0) { @@ -534,7 +550,7 @@ ArmSetMemoryAttributes ( } if ((AttributeMask & EFI_MEMORY_XP) != 0) { - TtEntryMask |= TT_DESCRIPTOR_SECTION_XN_MASK; + TtEntryMask |= TT_DESCRIPTOR_SECTION_XN_MASK | TT_DESCRIPTOR_SECTION_PXN_MASK; } } else { ASSERT (AttributeMask == 0); @@ -544,6 +560,7 @@ ArmSetMemoryAttributes ( TtEntryMask = TT_DESCRIPTOR_SECTION_TYPE_MASK | TT_DESCRIPTOR_SECTION_XN_MASK | + TT_DESCRIPTOR_SECTION_PXN_MASK | TT_DESCRIPTOR_SECTION_AP_MASK | TT_DESCRIPTOR_SECTION_AF; } diff --git a/ArmPkg/Library/CpuArchLib/Arm/Mmu.c b/ArmPkg/Library/CpuArchLib/Arm/Mmu.c index 86c375075a..62c2a4ef3a 100644 --- a/ArmPkg/Library/CpuArchLib/Arm/Mmu.c +++ b/ArmPkg/Library/CpuArchLib/Arm/Mmu.c @@ -78,7 +78,10 @@ SectionToGcdAttributes ( } // now process eXectue Never attribute - if ((SectionAttributes & TT_DESCRIPTOR_SECTION_XN_MASK) != 0) { + if ((((SectionAttributes & TT_DESCRIPTOR_SECTION_XN_MASK) != 0) + && ((*GcdAttributes & EFI_MEMORY_USER) != 0)) + || (((SectionAttributes & TT_DESCRIPTOR_SECTION_PXN_MASK) != 0) + && ((*GcdAttributes & EFI_MEMORY_USER) == 0))) { *GcdAttributes |= EFI_MEMORY_XP; } @@ -159,20 +162,24 @@ PageToGcdAttributes ( // determine protection attributes switch (PageAttributes & TT_DESCRIPTOR_PAGE_AP_MASK) { case TT_DESCRIPTOR_PAGE_AP_NO_RW: + break; case TT_DESCRIPTOR_PAGE_AP_RW_RW: - // normal read/write access, do not add additional attributes + *GcdAttributes |= EFI_MEMORY_USER; break; // read only cases map to write-protect case TT_DESCRIPTOR_PAGE_AP_NO_RO: + *GcdAttributes |= EFI_MEMORY_RO; + break; case TT_DESCRIPTOR_PAGE_AP_RO_RO: - *GcdAttributes |= EFI_MEMORY_RO; + *GcdAttributes |= EFI_MEMORY_RO | EFI_MEMORY_USER; break; } // now process eXectue Never attribute - if ((PageAttributes & TT_DESCRIPTOR_PAGE_XN_MASK) != 0) { - *GcdAttributes |= EFI_MEMORY_XP; + if (((PageAttributes & TT_DESCRIPTOR_PAGE_XN_MASK) != 0) + && ((*GcdAttributes & EFI_MEMORY_USER) != 0)) { + *GcdAttributes |= EFI_MEMORY_XP; } if ((PageAttributes & TT_DESCRIPTOR_PAGE_AF) == 0) { @@ -472,6 +479,11 @@ EfiAttributeToArmAttribute ( // Determine protection attributes if ((EfiAttributes & EFI_MEMORY_USER) != 0) { + // Determine eXecute Never attribute + if ((EfiAttributes & EFI_MEMORY_XP) != 0) { + ArmAttributes |= TT_DESCRIPTOR_SECTION_XN_MASK; + } + // // TODO: Add PXN for Translation table descriptors. // @@ -481,6 +493,11 @@ EfiAttributeToArmAttribute ( ArmAttributes |= TT_DESCRIPTOR_SECTION_AP_RW_RW; } } else { + // Determine eXecute Never attribute + if ((EfiAttributes & EFI_MEMORY_XP) != 0) { + ArmAttributes |= TT_DESCRIPTOR_SECTION_PXN_MASK; + } + if ((EfiAttributes & EFI_MEMORY_RO) != 0) { ArmAttributes |= TT_DESCRIPTOR_SECTION_AP_NO_RO; } else { @@ -488,11 +505,6 @@ EfiAttributeToArmAttribute ( } } - // Determine eXecute Never attribute - if ((EfiAttributes & EFI_MEMORY_XP) != 0) { - ArmAttributes |= TT_DESCRIPTOR_SECTION_XN_MASK; - } - if ((EfiAttributes & EFI_MEMORY_RP) == 0) { ArmAttributes |= TT_DESCRIPTOR_SECTION_AF; }