From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 4946DAC1946 for ; Sat, 9 Mar 2024 19:06:07 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=KyFfMSBUJReu76BWOqaMiCzY1QRZ13Jgx4NiqfjzL4k=; c=relaxed/simple; d=groups.io; h=DKIM-Filter:From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding; s=20240206; t=1710011165; v=1; b=WkHcfZ1SWIEeHAtkLH132Zkkzj+ckaPXUXZWp8rCyZ4uXrlOJBBwtp4b17lKok1ccVCxcRVt Hpke0R4llTc+A2MK14T/zPl2P2Nv+lo+WC+Tecc5NzdyUzS9ffGe8Nc30BSW6FbLtOXjBmHP66K nece3ySzsqp2lEflM3ulm4Z8xYqI7ze9LaD+rv3pPx7/5pDy1fY9tzMCpB87Gf8UTCd6AHVAcb1 vTWpPkS3f5xhOQpq/RWoXLada94cHXCeN1iUMS7BM44vxwaZEqMRvEFmFLBc0VpdDf+wZz7Cocf Ew/sEARIYGXQ2niqxefR+AQtJi4QHRswdgUPLT2NAvGQw== X-Received: by 127.0.0.2 with SMTP id dtvzYY7687511xq7WTqgcwtd; Sat, 09 Mar 2024 11:06:05 -0800 X-Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web10.18868.1710011163808855254 for ; Sat, 09 Mar 2024 11:06:03 -0800 X-Received: from OSD-Desktop.redmond.corp.microsoft.com (unknown [131.107.160.171]) by linux.microsoft.com (Postfix) with ESMTPSA id 2E7D920B74C3; Sat, 9 Mar 2024 11:06:03 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 2E7D920B74C3 From: "Oliver Smith-Denny" To: devel@edk2.groups.io Cc: Leif Lindholm , Ard Biesheuvel , Sami Mujawar , Liming Gao Subject: [edk2-devel][PATCH v3 3/3] MdeModulePkg: DxeCore: Do Not Apply Guards to Unsupported Types Date: Sat, 09 Mar 2024 11:06:03 -0800 Message-Id: <20240309190559.28677-4-osde@linux.microsoft.com> In-Reply-To: <20240309190559.28677-1-osde@linux.microsoft.com> References: <20240309190559.28677-1-osde@linux.microsoft.com> MIME-Version: 1.0 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,osde@linux.microsoft.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: ov7uc69igD03XfNdzidAaLYnx7686176AA= Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=WkHcfZ1S; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=linux.microsoft.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io Currently, there are multiple issues when page or pool guards are allocated for runtime memory regions that are aligned to non-EFI_PAGE_SIZE alignments. Multiple other issues have been fixed for these same systems (notably ARM64 which has a 64k runtime page allocation granularity) recently. The heap guard system is only built to support 4k guard pages and 4k alignment. Today, the address returned to a caller of AllocatePages will not be aligned correctly to the runtime page allocation granularity, because the heap guard system does not take non-4k alignment requirements into consideration. However, even with this bug fixed, the Memory Allocation Table cannot be produced and an OS with a larger than 4k page granularity will not have aligned memory regions because the guard pages are reported as part of the same memory allocation. So what would have been, on an ARM64 system, a 64k runtime memory allocation is actually a 72k memory allocation as tracked by the Page.c code because the guard pages are tracked as part of the same allocation. This is a core function of the current heap guard architecture. This could also be fixed with rearchitecting the heap guard system to respect alignment requirements and shift the guard pages inside of the outer rounded allocation or by having guard pages be the runtime granularity. Both of these approaches have issues. In the former case, we break UEFI spec 2.10 section 2.3.6 for AARCH64, which states that each 64k page for runtime memory regions may not have mixed memory attributes, which pushing the guard pages inside would create. In the latter case, an immense amount of memory is wasted to support such large guard pages, and with pool guard many systems could not support an additional 128k allocation for all runtime memory. The simpler and safer solution is to disallow page and pool guards for runtime memory allocations for systems that have a runtime granularity greater than the EFI_PAGE_SIZE (4k). The usefulness of such guards is limited, as OSes do not map guard pages today, so there is only boot time protection of these ranges. This also prevents other bugs from being exposed by using guards for regions that have a non-4k alignment requirement, as again, multiple have cropped up because the heap guard system was not built to support it. This patch adds both a static assert to ensure that either the runtime granularity is the EFI_PAGE_SIZE or that the PCD bits are not set to enable heap guard for runtime memory regions. It also adds a check in the page and pool allocation system to ensure that at runtime we are not allocating a runtime region and attempt to guard it (the PCDs are close to being removed in favor of dynamic heap guard configurations). BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4674 Github PR: https://github.com/tianocore/edk2/pull/5382 Cc: Leif Lindholm Cc: Ard Biesheuvel Cc: Sami Mujawar Cc: Liming Gao Signed-off-by: Oliver Smith-Denny --- MdeModulePkg/MdeModulePkg.dec | 10 ++++++++++ MdeModulePkg/Core/Dxe/Mem/HeapGuard.h | 14 ++++++++++++++ MdeModulePkg/Core/Dxe/Mem/Page.c | 11 +++++++++++ MdeModulePkg/Core/Dxe/Mem/Pool.c | 11 +++++++++++ 4 files changed, 46 insertions(+) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.de= c index a2cd83345f5b..a82dedc070df 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -1027,6 +1027,11 @@ [PcdsFixedAtBuild] # free pages for all of them. The page allocation for the type related= to # cleared bits keeps the same as ususal. # + # The heap guard system only supports guarding EfiRuntimeServicesCode,= EfiRuntimeServicesData, + # EfiReservedMemoryType, and EfiACPIMemoryNVS memory types for systems= that have + # RUNTIME_PAGE_ALLOCATION_GRANULARITY =3D=3D EFI_PAGE_SIZE. This is to= preserve alignment requirements + # without extending the page guard size to very large granularities. + # # This PCD is only valid if BIT0 and/or BIT2 are set in PcdHeapGuardPr= opertyMask. # # Below is bit mask for this PCD: (Order is same as UEFI spec)
@@ -1058,6 +1063,11 @@ [PcdsFixedAtBuild] # if there's enough free memory for all of them. The pool allocation f= or the # type related to cleared bits keeps the same as ususal. # + # The heap guard system only supports guarding EfiRuntimeServicesCode,= EfiRuntimeServicesData, + # EfiReservedMemoryType, and EfiACPIMemoryNVS memory types for systems= that have + # RUNTIME_PAGE_ALLOCATION_GRANULARITY =3D=3D EFI_PAGE_SIZE. This is to= preserve alignment requirements + # without extending the page guard size to very large granularities. + # # This PCD is only valid if BIT1 and/or BIT3 are set in PcdHeapGuardPr= opertyMask. # # Below is bit mask for this PCD: (Order is same as UEFI spec)
diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.h b/MdeModulePkg/Core/Dx= e/Mem/HeapGuard.h index 24b4206c0e02..578e85746585 100644 --- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.h +++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.h @@ -469,4 +469,18 @@ PromoteGuardedFreePages ( =20 extern BOOLEAN mOnGuarding; =20 +// +// The heap guard system does not support non-EFI_PAGE_SIZE alignments. +// Architectures that require larger RUNTIME_PAGE_ALLOCATION_GRANULARITY +// cannot have EfiRuntimeServicesCode, EfiRuntimeServicesData, EfiReserv= edMemoryType, +// and EfiACPIMemoryNVS guarded. OSes do not map guard pages anyway, so = this is a +// minimal loss. Not guarding prevents alignment mismatches +// +STATIC_ASSERT ( + RUNTIME_PAGE_ALLOCATION_GRANULARITY =3D=3D EFI_PAGE_SIZE || + (((FixedPcdGet64 (PcdHeapGuardPageType) & 0x461) =3D=3D 0) && + ((FixedPcdGet64 (PcdHeapGuardPoolType) & 0x461) =3D=3D 0)), + "Unsupported Heap Guard configuration on system with greater than EFI_= PAGE_SIZE RUNTIME_PAGE_ALLOCATION_GRANULARITY" + ); + #endif diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem= /Page.c index cd201d36a389..26584648c236 100644 --- a/MdeModulePkg/Core/Dxe/Mem/Page.c +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c @@ -1411,6 +1411,17 @@ CoreInternalAllocatePages ( Alignment =3D RUNTIME_PAGE_ALLOCATION_GRANULARITY; } =20 + // + // The heap guard system does not support non-EFI_PAGE_SIZE alignments= . + // Architectures that require larger RUNTIME_PAGE_ALLOCATION_GRANULARI= TY + // will have the runtime memory regions unguarded. OSes do not + // map guard pages anyway, so this is a minimal loss. Not guarding pre= vents + // alignment mismatches + // + if (Alignment !=3D EFI_PAGE_SIZE) { + NeedGuard =3D FALSE; + } + if (Type =3D=3D AllocateAddress) { if ((*Memory & (Alignment - 1)) !=3D 0) { return EFI_NOT_FOUND; diff --git a/MdeModulePkg/Core/Dxe/Mem/Pool.c b/MdeModulePkg/Core/Dxe/Mem= /Pool.c index ccfce8c5f959..72293e6dfe40 100644 --- a/MdeModulePkg/Core/Dxe/Mem/Pool.c +++ b/MdeModulePkg/Core/Dxe/Mem/Pool.c @@ -380,6 +380,17 @@ CoreAllocatePoolI ( Granularity =3D DEFAULT_PAGE_ALLOCATION_GRANULARITY; } =20 + // + // The heap guard system does not support non-EFI_PAGE_SIZE alignments= . + // Architectures that require larger RUNTIME_PAGE_ALLOCATION_GRANULARI= TY + // will have the runtime memory regions unguarded. OSes do not + // map guard pages anyway, so this is a minimal loss. Not guarding pre= vents + // alignment mismatches + // + if (Granularity !=3D EFI_PAGE_SIZE) { + NeedGuard =3D FALSE; + } + // // Adjust the size by the pool header & tail overhead // --=20 2.40.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#116561): https://edk2.groups.io/g/devel/message/116561 Mute This Topic: https://groups.io/mt/104832607/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-