From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.5435.1675879108545179638 for ; Wed, 08 Feb 2023 09:58:28 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=na0/QTFO; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 044C86173C; Wed, 8 Feb 2023 17:58:28 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0E684C433D2; Wed, 8 Feb 2023 17:58:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1675879107; bh=fjzjD6C5LVVc+5Cz2yxp0WA/0hBMh9OtIyqgdbj3LsU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=na0/QTFOovUT70CTdWTW50U/OrZPndjjXSgWq87GTnykuW9cXu50kMATD4sRWGZ8R ch8gpH3JRW11jPsLq0eFsZxGHAkByWLWi8boI37RKfs0L9VBRcVkBcM9btKopd5Oj+ 6xktOccowsw9JJjXbwxRGqUJn03e3U899O0fgHBplyxyLIzOIhBt59o+QC87dCfWmE uaR9byH/3yyNn6sjbJsM3AJh5ARdwBJbT9srRfr3yfJ8RCKNeQ327Hj+Y0KKs5JbJc agLqMa8vU7cRYpzyfWf+HZx1k1X63479ZhgxssnOYiigBjjkq/QpO1RVCB3Effq+ri rurWEH5VPNsIQ== From: "Ard Biesheuvel" To: devel@edk2.groups.io Cc: Ard Biesheuvel , Michael Kinney , Liming Gao , Jiewen Yao , Michael Kubacki , Sean Brogan , Rebecca Cran , Leif Lindholm , Sami Mujawar , Taylor Beebe , =?UTF-8?q?Marvin=20H=C3=A4user?= Subject: [PATCH 2/3] ArmPkg/CpuDxe: Perform preliminary NX remap of free memory Date: Wed, 8 Feb 2023 18:58:11 +0100 Message-Id: <20230208175812.700129-3-ardb@kernel.org> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230208175812.700129-1-ardb@kernel.org> References: <20230208175812.700129-1-ardb@kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable The DXE core implementation of PcdDxeNxMemoryProtectionPolicy already contains an assertion that EfiConventionalMemory and EfiBootServicesData are subjected to the same policy when it comes to the use of NX permissions. The reason for this is that we may otherwise end up with unbounded recursion in the page table code, given that allocating a page table would then involve a permission attribute change, and this could result in the need for a block entry to be split, which would trigger the allocation of a page table recursively. For the same reason, a shortcut exists in ApplyMemoryProtectionPolicy() where, instead of setting the memory attributes unconditionally, we compare the NX policies and avoid touching the page tables if they are the same for the old and the new memory types. Without this shortcut, we may end up in a situation where, as the CPU arch protocol DXE driver is ramping up, the same unbounded recursion is triggered, due to the fact that the NX policy for EfiConventionalMemory has not been applied yet. To break this cycle, let's remap all EfiConventionalMemory regions according to the NX policy for EfiBootServicesData before exposing the CPU arch protocol to the DXE core and other drivers. This ensures that creating EfiBootServicesData allocations does not result in memory attribute changes, and therefore no recursion. Signed-off-by: Ard Biesheuvel --- ArmPkg/Drivers/CpuDxe/CpuDxe.c | 77 ++++++++++++++++++++ ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 2 + 2 files changed, 79 insertions(+) diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.c b/ArmPkg/Drivers/CpuDxe/CpuDxe.c index d04958e79e52..83fd6fd4e476 100644 --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.c +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.c @@ -11,6 +11,8 @@ =0D #include =0D =0D +#include =0D +=0D BOOLEAN mIsFlushingGCD;=0D =0D /**=0D @@ -227,6 +229,69 @@ InitializeDma ( CpuArchProtocol->DmaBufferAlignment =3D ArmCacheWritebackGranule ();=0D }=0D =0D +STATIC=0D +VOID=0D +RemapUnusedMemoryNx (=0D + VOID=0D + )=0D +{=0D + UINT64 TestBit;=0D + UINTN MemoryMapSize;=0D + UINTN MapKey;=0D + UINTN DescriptorSize;=0D + UINT32 DescriptorVersion;=0D + EFI_MEMORY_DESCRIPTOR *MemoryMap;=0D + EFI_MEMORY_DESCRIPTOR *MemoryMapEntry;=0D + EFI_MEMORY_DESCRIPTOR *MemoryMapEnd;=0D + EFI_STATUS Status;=0D +=0D + TestBit =3D LShiftU64 (1, EfiBootServicesData);=0D + if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & TestBit) =3D=3D 0) {=0D + return;=0D + }=0D +=0D + MemoryMapSize =3D 0;=0D + MemoryMap =3D NULL;=0D +=0D + Status =3D gBS->GetMemoryMap (=0D + &MemoryMapSize,=0D + MemoryMap,=0D + &MapKey,=0D + &DescriptorSize,=0D + &DescriptorVersion=0D + );=0D + ASSERT (Status =3D=3D EFI_BUFFER_TOO_SMALL);=0D + do {=0D + MemoryMap =3D (EFI_MEMORY_DESCRIPTOR *)AllocatePool (MemoryMapSize);=0D + ASSERT (MemoryMap !=3D NULL);=0D + Status =3D gBS->GetMemoryMap (=0D + &MemoryMapSize,=0D + MemoryMap,=0D + &MapKey,=0D + &DescriptorSize,=0D + &DescriptorVersion=0D + );=0D + if (EFI_ERROR (Status)) {=0D + FreePool (MemoryMap);=0D + }=0D + } while (Status =3D=3D EFI_BUFFER_TOO_SMALL);=0D +=0D + ASSERT_EFI_ERROR (Status);=0D +=0D + MemoryMapEntry =3D MemoryMap;=0D + MemoryMapEnd =3D (EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + Memory= MapSize);=0D + while ((UINTN)MemoryMapEntry < (UINTN)MemoryMapEnd) {=0D + if (MemoryMapEntry->Type =3D=3D EfiConventionalMemory) {=0D + ArmSetMemoryRegionNoExec (=0D + MemoryMapEntry->PhysicalStart,=0D + EFI_PAGES_TO_SIZE (MemoryMapEntry->NumberOfPages)=0D + );=0D + }=0D +=0D + MemoryMapEntry =3D NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorS= ize);=0D + }=0D +}=0D +=0D EFI_STATUS=0D CpuDxeInitialize (=0D IN EFI_HANDLE ImageHandle,=0D @@ -240,6 +305,18 @@ CpuDxeInitialize ( =0D InitializeDma (&mCpu);=0D =0D + //=0D + // Once we install the CPU arch protocol, the DXE core's memory=0D + // protection routines will invoke them to manage the permissions of pag= e=0D + // allocations as they are created. Given that this includes pages=0D + // allocated for page tables by this driver, we must ensure that unused= =0D + // memory is mapped with the same permissions as boot services data=0D + // regions. Otherwise, we may end up with unbounded recursion, due to th= e=0D + // fact that updating permissions on a newly allocated page table may tr= igger=0D + // a block entry split, which triggers a page table allocation, etc etc= =0D + //=0D + RemapUnusedMemoryNx ();=0D +=0D Status =3D gBS->InstallMultipleProtocolInterfaces (=0D &mCpuHandle,=0D &gEfiCpuArchProtocolGuid,=0D diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf b/ArmPkg/Drivers/CpuDxe/CpuDx= e.inf index e732e21cb94a..8fd0f4133088 100644 --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf @@ -48,6 +48,7 @@ [LibraryClasses] DefaultExceptionHandlerLib=0D DxeServicesTableLib=0D HobLib=0D + MemoryAllocationLib=0D PeCoffGetEntryPointLib=0D UefiDriverEntryPoint=0D UefiLib=0D @@ -64,6 +65,7 @@ [Guids] =0D [Pcd.common]=0D gArmTokenSpaceGuid.PcdVFPEnabled=0D + gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy=0D =0D [FeaturePcd.common]=0D gArmTokenSpaceGuid.PcdDebuggerExceptionSupport=0D --=20 2.39.1