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 05B76D80127 for ; Wed, 9 Aug 2023 21:35:03 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=5U20hsAEatC+adDiJC4yKpALy7fGaSyUbX1IyUtDANE=; c=relaxed/simple; d=groups.io; h=DKIM-Filter:From:To:Cc:Subject:Date:Message-Id: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=20140610; t=1691616902; v=1; b=lVYgZonNwghVK1JVRWBMI8IwEhyWViPaYr+Ju3EvozW2PKnCi2NuV0UaskWb7zsKfEqtKqmf shR8by95eGRnLcevsj6H7igzpoQjwfrUgkmFs/it2wl258W+C4FxyPGxnAP6wgRJcyvvcdTKCZa aOhs3Yr+XBwwwt6HKPjHQQOA= X-Received: by 127.0.0.2 with SMTP id Be2dYY7687511xr15AFA5BnL; Wed, 09 Aug 2023 14:35:02 -0700 X-Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web10.3665.1691616902152756516 for ; Wed, 09 Aug 2023 14:35:02 -0700 X-Received: from OSD-Desktop.redmond.corp.microsoft.com (unknown [131.107.1.171]) by linux.microsoft.com (Postfix) with ESMTPSA id D3EB520FC4D2; Wed, 9 Aug 2023 14:35:01 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com D3EB520FC4D2 From: "Oliver Smith-Denny" To: devel@edk2.groups.io Cc: Leif Lindholm , Ard Biesheuvel , Jian J Wang , Liming Gao , Dandan Bi Subject: [edk2-devel][PATCH v1 1/1] MdeModulePkg: HeapGuard: Don't Assume Pool Head Allocated In First Page Date: Wed, 9 Aug 2023 14:34:57 -0700 Message-Id: <20230809213457.18664-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: C8w37bwC60CqkM4QuisLOnoyx7686176AA= Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=lVYgZonN; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=linux.microsoft.com (policy=none) Currently, HeapGuard, when in the GuardAlignedToTail mode, assumes that the pool head has been allocated in the first page of memory that was allocated. This is not the case for ARM64 platforms when allocating runtime pools, as RUNTIME_PAGE_ALLOCATION_GRANULARITY is 64k, unlike X64, which has RUNTIME_PAGE_ALLOCATION_GRANULARITY as 4k. When a runtime pool is allocated on ARM64, the minimum number of pages allocated is 16, to match the runtime granularity. When a small pool is allocated and GuardAlignedToTail is true, HeapGuard instructs the pool head to be placed as (MemoryAllocated + EFI_PAGES_TO_SIZE(Number of Pages= ) - SizeRequiredForPool). This gives this scenario: |Head Guard|Large Free Number of Pages|PoolHead|TailGuard| When this pool goes to be freed, HeapGuard instructs the pool code to free from (PoolHead & ~EFI_PAGE_MASK). However, this assumes that the PoolHead is in the first page allocated, which as shown above is not true in this case. For the 4k granularity case (i.e. where the correct number = of pages are allocated for this pool), this logic does work. In this failing case, HeapGuard then instructs the pool code to free 16 (or more depending) pages from the page the pool head was allocated on, which as seen above means we overrun the pool and attempt to free memory far past the pool. We end up running into the tail guard and getting an access flag fault. This causes ArmVirtQemu to fail to boot with an access flag fault when GuardAlignedToTail is set to true (and pool guard enabled for runtime memory). It should also cause all ARM64 platforms to fail in this configuration, for exactly the same reason, as this is core code making the assumption. This patch removes HeapGuard's assumption that the pool head is allocated on the first page and instead undoes the same logic that HeapGuard did when allocating the pool head in the first place. With this patch in place, ArmVirtQemu boots with GuardAlignedToTail set to true (and when it is false, also). BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4521 Github PR: https://github.com/tianocore/edk2/pull/4731 Cc: Leif Lindholm Cc: Ard Biesheuvel Cc: Jian J Wang Cc: Liming Gao Cc: Dandan Bi Signed-off-by: Oliver Smith-Denny --- MdeModulePkg/Core/Dxe/Mem/HeapGuard.h | 7 ++++++- MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 14 +++++++++++--- MdeModulePkg/Core/Dxe/Mem/Pool.c | 2 +- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.h b/MdeModulePkg/Core/Dx= e/Mem/HeapGuard.h index 9a32b4dd51d5..24b4206c0e02 100644 --- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.h +++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.h @@ -378,12 +378,17 @@ AdjustPoolHeadA ( Get the page base address according to pool head address. =20 @param[in] Memory Head address of pool to free. + @param[in] NoPages Number of pages actually allocated. + @param[in] Size Size of memory requested. + (plus pool head/tail overhead) =20 @return Address of pool head. **/ VOID * AdjustPoolHeadF ( - IN EFI_PHYSICAL_ADDRESS Memory + IN EFI_PHYSICAL_ADDRESS Memory, + IN UINTN NoPages, + IN UINTN Size ); =20 /** diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c b/MdeModulePkg/Core/Dx= e/Mem/HeapGuard.c index 9377f620c5a5..0c0ca61872b4 100644 --- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c +++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c @@ -1037,12 +1037,17 @@ AdjustPoolHeadA ( Get the page base address according to pool head address. =20 @param[in] Memory Head address of pool to free. + @param[in] NoPages Number of pages actually allocated. + @param[in] Size Size of memory requested. + (plus pool head/tail overhead) =20 @return Address of pool head. **/ VOID * AdjustPoolHeadF ( - IN EFI_PHYSICAL_ADDRESS Memory + IN EFI_PHYSICAL_ADDRESS Memory, + IN UINTN NoPages, + IN UINTN Size ) { if ((Memory =3D=3D 0) || ((PcdGet8 (PcdHeapGuardPropertyMask) & BIT7) = !=3D 0)) { @@ -1053,9 +1058,12 @@ AdjustPoolHeadF ( } =20 // - // Pool head is put near the tail Guard + // Pool head is put near the tail Guard. We need to exactly undo the a= ddition done in AdjustPoolHeadA + // because we may not have allocated the pool head on the first alloca= ted page, since we are aligned to + // the tail and on some architectures, the runtime page allocation gra= nularity is > one page. So we allocate + // more pages than we need and put the pool head somewhere past the fi= rst page. // - return (VOID *)(UINTN)(Memory & ~EFI_PAGE_MASK); + return (VOID *)(UINTN)(Memory + Size - EFI_PAGES_TO_SIZE (NoPages)); } =20 /** diff --git a/MdeModulePkg/Core/Dxe/Mem/Pool.c b/MdeModulePkg/Core/Dxe/Mem= /Pool.c index b20cbfdedbab..716dd045f9fd 100644 --- a/MdeModulePkg/Core/Dxe/Mem/Pool.c +++ b/MdeModulePkg/Core/Dxe/Mem/Pool.c @@ -783,7 +783,7 @@ CoreFreePoolI ( NoPages =3D EFI_SIZE_TO_PAGES (Size) + EFI_SIZE_TO_PAGES (Granulari= ty) - 1; NoPages &=3D ~(UINTN)(EFI_SIZE_TO_PAGES (Granularity) - 1); if (IsGuarded) { - Head =3D AdjustPoolHeadF ((EFI_PHYSICAL_ADDRESS)(UINTN)Head); + Head =3D AdjustPoolHeadF ((EFI_PHYSICAL_ADDRESS)(UINTN)Head, NoPag= es, Size); CoreFreePoolPagesWithGuard ( Pool->MemoryType, (EFI_PHYSICAL_ADDRESS)(UINTN)Head, --=20 2.40.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107656): https://edk2.groups.io/g/devel/message/107656 Mute This Topic: https://groups.io/mt/100652659/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-