Re: [edk2-devel] [PATCH] MdeModulePkg/DxeCore: Use correct type for alignment mask

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hi Ard,

Thank you.  This is a really good find.  How did you find it?

I am guessing this case may not have been noticed on IA32/X64 because
those archs tend to have FLASH device mapped just below 4GB and as a
result there is no system memory in the range just below 4GB. This 
means the case where a page allocation with an end descriptor at
4GB is not observed on IA32/X64.

Thanks,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel
> Sent: Wednesday, December 7, 2022 10:01 AM
> To: devel@edk2.groups.io
> Cc: Bi, Dandan <dandan.bi@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Wang, Jian J <jian.j.wang@intel.com>; Ard
> Biesheuvel <ardb@kernel.org>
> Subject: [edk2-devel] [PATCH] MdeModulePkg/DxeCore: Use correct type for alignment mask
> 
> The page allocator code in CoreFindFreePagesI() uses a mask derived from
> its UINTN Alignment argument to align the descriptor end address of a
> MEMORY_MAP entry to the requested alignment, in order to check whether
> the descriptor covers enough sufficiently aligned area to satisfy the
> request.
> 
> However, on 32-bit architectures, 'Alignment' is a 32-bit type, whereas
> DescEnd is a 64-bit type, and so the resulting operation performed on
> the end address comes down to masking with 0xfffff000 instead of the
> intended 0xffffffff_fffff000. Given the -1 at the end of the expression,
> the resulting address is 0xffffffff_fffffffff for any descriptor that
> ends on a 4G aligned boundary, and this is certainly not what was
> intended.
> 
> So cast Alignment to UINT64 to ensure that the mask has the right size.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  MdeModulePkg/Core/Dxe/Mem/Page.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
> index 160289c1f9ec..5903ce7ab525 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> @@ -1097,7 +1097,7 @@ CoreFindFreePagesI (
>        DescEnd = MaxAddress;
> 
>      }
> 
> 
> 
> -    DescEnd = ((DescEnd + 1) & (~(Alignment - 1))) - 1;
> 
> +    DescEnd = ((DescEnd + 1) & (~((UINT64)Alignment - 1))) - 1;
> 
> 
> 
>      // Skip if DescEnd is less than DescStart after alignment clipping
> 
>      if (DescEnd < DescStart) {
> 
> --
> 2.35.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#97101): https://edk2.groups.io/g/devel/message/97101
> Mute This Topic: https://groups.io/mt/95520976/1643496
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
> -=-=-=-=-=-=
>