diff --git a/ArmPkg/Include/Library/ArmStandaloneMmCoreEntryPoint.h b/ArmPkg/Include/Library/ArmStandaloneMmCoreEntryPoint.h index d08db8c1d9..2f99fa1b6e 100644 --- a/ArmPkg/Include/Library/ArmStandaloneMmCoreEntryPoint.h +++ b/ArmPkg/Include/Library/ArmStandaloneMmCoreEntryPoint.h @@ -89,7 +89,6 @@ typedef RETURN_STATUS (*REGION_PERMISSION_UPDATE_FUNC) ( @param [in] ImageBase Base of image in memory @param [in] SectionHeaderOffset Offset of PE/COFF image section header @param [in] NumberOfSections Number of Sections - @param [in] TextUpdater Function to change code permissions @param [in] ReadOnlyUpdater Function to change RO permissions @param [in] ReadWriteUpdater Function to change RW permissions @@ -101,7 +100,6 @@ UpdateMmFoundationPeCoffPermissions ( IN EFI_PHYSICAL_ADDRESS ImageBase, IN UINT32 SectionHeaderOffset, IN CONST UINT16 NumberOfSections, - IN REGION_PERMISSION_UPDATE_FUNC TextUpdater, IN REGION_PERMISSION_UPDATE_FUNC ReadOnlyUpdater, IN REGION_PERMISSION_UPDATE_FUNC ReadWriteUpdater ); diff --git a/ArmPkg/Include/Library/StandaloneMmMmuLib.h b/ArmPkg/Include/Library/StandaloneMmMmuLib.h index 76f9e443a1..4c4f155ed8 100644 --- a/ArmPkg/Include/Library/StandaloneMmMmuLib.h +++ b/ArmPkg/Include/Library/StandaloneMmMmuLib.h @@ -57,7 +57,7 @@ ArmClearMemoryRegionNoExec ( ); /** - Set the memory to read-only while preserving execute permission. + Change memory permission as RO ignoring former permission. @param [in] BaseAddress Base address for the memory region. @param [in] Length Length of the memory region. @@ -75,13 +75,13 @@ ArmClearMemoryRegionNoExec ( **/ EFI_STATUS -ArmSetMemoryRegionReadOnly ( +ArmSetMemoryRegionReadOnlyPerm ( IN EFI_PHYSICAL_ADDRESS BaseAddress, IN UINT64 Length ); /** - Set the memory to read-write while preserving execute permission. + Change memory permission as RW ignoring former permission. @param [in] BaseAddress Base address for the memory region. @param [in] Length Length of the memory region. @@ -99,7 +99,31 @@ ArmSetMemoryRegionReadOnly ( **/ EFI_STATUS -ArmClearMemoryRegionReadOnly ( +ArmSetMemoryRegionReadWritePerm ( + IN EFI_PHYSICAL_ADDRESS BaseAddress, + IN UINT64 Length + ); + +/** + Change memory permission as ROX ignoring former permission. + + @param [in] BaseAddress Base address for the memory region. + @param [in] Length Length of the memory region. + + @retval EFI_SUCCESS Request successfull. + @retval EFI_INVALID_PARAMETER A parameter is invalid. + @retval EFI_NOT_READY Callee is busy or not in a state to handle + this request. + @retval EFI_UNSUPPORTED This function is not implemented by the + callee. + @retval EFI_ABORTED Message target ran into an unexpected error + and has aborted. + @retval EFI_ACCESS_DENIED Access denied. + @retval EFI_OUT_OF_RESOURCES Out of memory to perform operation. + +**/ +EFI_STATUS +ArmSetMemoryRegionReadOnlyExecPerm ( IN EFI_PHYSICAL_ADDRESS BaseAddress, IN UINT64 Length ); diff --git a/ArmPkg/Library/ArmStandaloneMmCoreEntryPoint/ArmStandaloneMmCoreEntryPoint.c b/ArmPkg/Library/ArmStandaloneMmCoreEntryPoint/ArmStandaloneMmCoreEntryPoint.c index d5c786ba88..344cfe3904 100644 --- a/ArmPkg/Library/ArmStandaloneMmCoreEntryPoint/ArmStandaloneMmCoreEntryPoint.c +++ b/ArmPkg/Library/ArmStandaloneMmCoreEntryPoint/ArmStandaloneMmCoreEntryPoint.c @@ -1198,9 +1198,8 @@ CEntryPoint ( ImageBase, SectionHeaderOffset, NumberOfSections, - ArmSetMemoryRegionNoExec, - ArmSetMemoryRegionReadOnly, - ArmClearMemoryRegionReadOnly + ArmSetMemoryRegionReadOnlyPerm, + ArmSetMemoryRegionReadWritePerm ); if (EFI_ERROR (Status)) { goto finish; @@ -1208,8 +1207,7 @@ CEntryPoint ( if (ImageContext.ImageAddress != (UINTN)TeData) { ImageContext.ImageAddress = (UINTN)TeData; - ArmSetMemoryRegionNoExec (ImageBase, SIZE_4KB); - ArmClearMemoryRegionReadOnly (ImageBase, SIZE_4KB); + ArmSetMemoryRegionReadWritePerm (ImageBase, SIZE_4KB); Status = PeCoffLoaderRelocateImage (&ImageContext); ASSERT_EFI_ERROR (Status); diff --git a/ArmPkg/Library/ArmStandaloneMmCoreEntryPoint/SetPermissions.c b/ArmPkg/Library/ArmStandaloneMmCoreEntryPoint/SetPermissions.c index 8fa748488b..56bf1c11f4 100644 --- a/ArmPkg/Library/ArmStandaloneMmCoreEntryPoint/SetPermissions.c +++ b/ArmPkg/Library/ArmStandaloneMmCoreEntryPoint/SetPermissions.c @@ -34,7 +34,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent @param [in] ImageBase Base of image in memory @param [in] SectionHeaderOffset Offset of PE/COFF image section header @param [in] NumberOfSections Number of Sections - @param [in] TextUpdater Function to change code permissions @param [in] ReadOnlyUpdater Function to change RO permissions @param [in] ReadWriteUpdater Function to change RW permissions @@ -46,7 +45,6 @@ UpdateMmFoundationPeCoffPermissions ( IN EFI_PHYSICAL_ADDRESS ImageBase, IN UINT32 SectionHeaderOffset, IN CONST UINT16 NumberOfSections, - IN REGION_PERMISSION_UPDATE_FUNC TextUpdater, IN REGION_PERMISSION_UPDATE_FUNC ReadOnlyUpdater, IN REGION_PERMISSION_UPDATE_FUNC ReadWriteUpdater ) @@ -128,8 +126,6 @@ UpdateMmFoundationPeCoffPermissions ( if ((SectionHeader.Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) == 0) { Base = ImageBase + SectionHeader.VirtualAddress; - TextUpdater (Base, ALIGN_VALUE (SectionHeader.Misc.VirtualSize, SectionAlignment)); - if ((SectionHeader.Characteristics & EFI_IMAGE_SCN_MEM_WRITE) != 0) { ReadWriteUpdater (Base, ALIGN_VALUE (SectionHeader.Misc.VirtualSize, SectionAlignment)); DEBUG (( @@ -140,6 +136,7 @@ UpdateMmFoundationPeCoffPermissions ( ImageContext->ImageAddress )); } else { + ReadOnlyUpdater (Base, ALIGN_VALUE (SectionHeader.Misc.VirtualSize, SectionAlignment)); DEBUG (( DEBUG_INFO, "%a: Mapping section %d of image at 0x%lx with RO-XN permissions\n", diff --git a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c index 8199110aa2..eefed6ddb1 100644 --- a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c +++ b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c @@ -309,6 +309,14 @@ ArmSetMemoryRegionNoExec ( UINTN Size; UINT32 PageCount; + /* + * The .reloc section and others may be empty. + * In that case, return success silently. + */ + if (Length == 0) { + return EFI_SUCCESS; + } + UseFfaAbis = IsFfaMemoryAbiSupported (&Version); while (Length > 0) { @@ -391,6 +399,14 @@ ArmClearMemoryRegionNoExec ( UINTN Size; UINT32 PageCount; + /* + * The .reloc section and others may be empty. + * In that case, return success silently. + */ + if (Length == 0) { + return EFI_SUCCESS; + } + UseFfaAbis = IsFfaMemoryAbiSupported (&Version); while (Length > 0) { @@ -442,7 +458,7 @@ ArmClearMemoryRegionNoExec ( } /** - Set the memory to read-only while preserving execute permission. + Change memory permission as RO ignoring former permission. @param [in] BaseAddress Base address for the memory region. @param [in] Length Length of the memory region. @@ -460,71 +476,47 @@ ArmClearMemoryRegionNoExec ( **/ EFI_STATUS -ArmSetMemoryRegionReadOnly ( +ArmSetMemoryRegionReadOnlyPerm ( IN EFI_PHYSICAL_ADDRESS BaseAddress, IN UINT64 Length ) { - EFI_STATUS Status; - UINT32 MemoryAttributes; - UINT32 PermissionRequest; - BOOLEAN UseFfaAbis; - UINT32 Version; - UINTN Size; - UINT32 PageCount; + UINT32 PermissionRequest; + BOOLEAN UseFfaAbis; - UseFfaAbis = IsFfaMemoryAbiSupported (&Version); + /* + * The .reloc section and others may be empty. + * In that case, return success silently. + */ + if (Length == 0) { + return EFI_SUCCESS; + } - while (Length > 0) { - Status = GetMemoryPermissions ( - UseFfaAbis, - Version, - BaseAddress, - Length, - &MemoryAttributes, - &PageCount - ); - if (EFI_ERROR (Status)) { - break; - } + UseFfaAbis = IsFfaMemoryAbiSupported (NULL); + Length = ALIGN_VALUE (Length, EFI_PAGE_SIZE); - Size = EFI_PAGES_TO_SIZE (PageCount); + if (UseFfaAbis) { + PermissionRequest = ARM_FFA_SET_MEM_ATTR_MAKE_PERM_REQUEST ( + ARM_FFA_SET_MEM_ATTR_DATA_PERM_RO, + ARM_FFA_SET_MEM_ATTR_CODE_PERM_XN + ); + } else { + PermissionRequest = ARM_SPM_MM_SET_MEM_ATTR_MAKE_PERM_REQUEST ( + ARM_SPM_MM_SET_MEM_ATTR_DATA_PERM_RO, + ARM_SPM_MM_SET_MEM_ATTR_CODE_PERM_XN + ); + } - if (UseFfaAbis) { - PermissionRequest = ARM_FFA_SET_MEM_ATTR_MAKE_PERM_REQUEST ( - ARM_FFA_SET_MEM_ATTR_DATA_PERM_RO, - (MemoryAttributes >> ARM_FFA_SET_MEM_ATTR_CODE_PERM_SHIFT) - ); - } else { - PermissionRequest = ARM_SPM_MM_SET_MEM_ATTR_MAKE_PERM_REQUEST ( - ARM_SPM_MM_SET_MEM_ATTR_DATA_PERM_RO, - (MemoryAttributes >> ARM_SPM_MM_SET_MEM_ATTR_CODE_PERM_SHIFT) - ); - } - - if (Length < Size) { - Length = Size; - } - - Status = RequestMemoryPermissionChange ( - UseFfaAbis, - BaseAddress, - Size, - PermissionRequest - ); - if (EFI_ERROR (Status)) { - return Status; - } - - Length -= Size; - BaseAddress += Size; - } // while - - return Status; + return RequestMemoryPermissionChange ( + UseFfaAbis, + BaseAddress, + Length, + PermissionRequest + ); } /** - Set the memory to read-write while preserving execute permission. + Change memory permission as RW ignoring former permission. @param [in] BaseAddress Base address for the memory region. @param [in] Length Length of the memory region. @@ -542,65 +534,99 @@ ArmSetMemoryRegionReadOnly ( **/ EFI_STATUS -ArmClearMemoryRegionReadOnly ( +ArmSetMemoryRegionReadWritePerm ( IN EFI_PHYSICAL_ADDRESS BaseAddress, IN UINT64 Length ) { - EFI_STATUS Status; - UINT32 MemoryAttributes; - UINT32 PermissionRequest; - BOOLEAN UseFfaAbis; - UINT32 Version; - UINTN Size; - UINT32 PageCount; + UINT32 PermissionRequest; + BOOLEAN UseFfaAbis; - UseFfaAbis = IsFfaMemoryAbiSupported (&Version); + /* + * The .reloc section and others may be empty. + * In that case, return success silently. + */ + if (Length == 0) { + return EFI_SUCCESS; + } - while (Length > 0) { - Status = GetMemoryPermissions ( - UseFfaAbis, - Version, - BaseAddress, - Length, - &MemoryAttributes, - &PageCount - ); - if (EFI_ERROR (Status)) { - break; - } + UseFfaAbis = IsFfaMemoryAbiSupported (NULL); + Length = ALIGN_VALUE (Length, EFI_PAGE_SIZE); - Size = EFI_PAGES_TO_SIZE (PageCount); + if (UseFfaAbis) { + PermissionRequest = ARM_FFA_SET_MEM_ATTR_MAKE_PERM_REQUEST ( + ARM_FFA_SET_MEM_ATTR_DATA_PERM_RW, + ARM_FFA_SET_MEM_ATTR_CODE_PERM_XN + ); + } else { + PermissionRequest = ARM_SPM_MM_SET_MEM_ATTR_MAKE_PERM_REQUEST ( + ARM_SPM_MM_SET_MEM_ATTR_DATA_PERM_RW, + ARM_SPM_MM_SET_MEM_ATTR_CODE_PERM_XN + ); + } - if (UseFfaAbis) { - PermissionRequest = ARM_FFA_SET_MEM_ATTR_MAKE_PERM_REQUEST ( - ARM_FFA_SET_MEM_ATTR_DATA_PERM_RW, - (MemoryAttributes >> ARM_FFA_SET_MEM_ATTR_CODE_PERM_SHIFT) - ); - } else { - PermissionRequest = ARM_SPM_MM_SET_MEM_ATTR_MAKE_PERM_REQUEST ( - ARM_SPM_MM_SET_MEM_ATTR_DATA_PERM_RW, - (MemoryAttributes >> ARM_SPM_MM_SET_MEM_ATTR_CODE_PERM_SHIFT) - ); - } - - if (Length < Size) { - Length = Size; - } - - Status = RequestMemoryPermissionChange ( - UseFfaAbis, - BaseAddress, - Size, - PermissionRequest - ); - if (EFI_ERROR (Status)) { - return Status; - } - - Length -= Size; - BaseAddress += Size; - } // while - - return Status; + return RequestMemoryPermissionChange ( + UseFfaAbis, + BaseAddress, + Length, + PermissionRequest + ); +} + +/** + Change memory permission as ROX ignoring former permission. + + @param [in] BaseAddress Base address for the memory region. + @param [in] Length Length of the memory region. + + @retval EFI_SUCCESS Request successfull. + @retval EFI_INVALID_PARAMETER A parameter is invalid. + @retval EFI_NOT_READY Callee is busy or not in a state to handle + this request. + @retval EFI_UNSUPPORTED This function is not implemented by the + callee. + @retval EFI_ABORTED Message target ran into an unexpected error + and has aborted. + @retval EFI_ACCESS_DENIED Access denied. + @retval EFI_OUT_OF_RESOURCES Out of memory to perform operation. + +**/ +EFI_STATUS +ArmSetMemoryRegionReadOnlyExecPerm ( + IN EFI_PHYSICAL_ADDRESS BaseAddress, + IN UINT64 Length + ) +{ + UINT32 PermissionRequest; + BOOLEAN UseFfaAbis; + + /* + * The .reloc section and others may be empty. + * In that case, return success silently. + */ + if (Length == 0) { + return EFI_SUCCESS; + } + + UseFfaAbis = IsFfaMemoryAbiSupported (NULL); + Length = ALIGN_VALUE (Length, EFI_PAGE_SIZE); + + if (UseFfaAbis) { + PermissionRequest = ARM_FFA_SET_MEM_ATTR_MAKE_PERM_REQUEST ( + ARM_FFA_SET_MEM_ATTR_DATA_PERM_RO, + ARM_FFA_SET_MEM_ATTR_CODE_PERM_X + ); + } else { + PermissionRequest = ARM_SPM_MM_SET_MEM_ATTR_MAKE_PERM_REQUEST ( + ARM_SPM_MM_SET_MEM_ATTR_DATA_PERM_RO, + ARM_SPM_MM_SET_MEM_ATTR_CODE_PERM_X + ); + } + + return RequestMemoryPermissionChange ( + UseFfaAbis, + BaseAddress, + Length, + PermissionRequest + ); } diff --git a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/AArch64/StandaloneMmPeCoffExtraActionLib.c b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/AArch64/StandaloneMmPeCoffExtraActionLib.c index 70ba26f78f..efe4621248 100644 --- a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/AArch64/StandaloneMmPeCoffExtraActionLib.c +++ b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/AArch64/StandaloneMmPeCoffExtraActionLib.c @@ -28,7 +28,8 @@ RETURN_STATUS UpdatePeCoffPermissions ( IN CONST PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext, IN REGION_PERMISSION_UPDATE_FUNC NoExecUpdater, - IN REGION_PERMISSION_UPDATE_FUNC ReadOnlyUpdater + IN REGION_PERMISSION_UPDATE_FUNC ReadOnlyUpdater, + IN REGION_PERMISSION_UPDATE_FUNC ReadOnlyExecUpdater ) { RETURN_STATUS Status; @@ -205,8 +206,7 @@ UpdatePeCoffPermissions ( Base, SectionHeader.Misc.VirtualSize )); - ReadOnlyUpdater (Base, ALIGN_VALUE (SectionHeader.Misc.VirtualSize, SectionAlignment)); - NoExecUpdater (Base, ALIGN_VALUE (SectionHeader.Misc.VirtualSize, SectionAlignment)); + ReadOnlyExecUpdater (Base, ALIGN_VALUE (SectionHeader.Misc.VirtualSize, SectionAlignment)); } SectionHeaderOffset += sizeof (EFI_IMAGE_SECTION_HEADER); @@ -233,7 +233,8 @@ PeCoffLoaderRelocateImageExtraAction ( UpdatePeCoffPermissions ( ImageContext, ArmClearMemoryRegionNoExec, - ArmSetMemoryRegionReadOnly + ArmSetMemoryRegionReadOnlyPerm, + ArmSetMemoryRegionReadOnlyExecPerm ); } @@ -256,6 +257,7 @@ PeCoffLoaderUnloadImageExtraAction ( UpdatePeCoffPermissions ( ImageContext, ArmSetMemoryRegionNoExec, - ArmClearMemoryRegionReadOnly + ArmSetMemoryRegionReadWritePerm, + ArmSetMemoryRegionReadWritePerm ); }