Re: [RFC PATCH 3/7] MdeModulePkg/Dxe/Page: take MAX_ALLOC_ADDRESS into account

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Mon, 10 Dec 2018 at 03:04, Wang, Jian J <jian.j.wang@intel.com> wrote:
>
> Hi Ard,
>
> I think MAX_ALLOC_ADDRESS will affect other archs besides ARM. Please do enough
> test for them (IA32/X64 for my concern).
>

For all other architectures, MAX_ADDRESS == MAX_ALLOC_ADDRESS is
always true, so these changes only affect AARCH64.

> In addition, do you think it's safer to replace MAX_ADDRESS with MAX_ALLOC_ADDRESS
> in MemoryAllocationLib like following situation?
>
> (MdeModulePkg\Library\DxeCoreMemoryAllocationLib\MemoryAllocationLib.c)
> VOID *
> InternalAllocateCopyPool (
>   IN EFI_MEMORY_TYPE  PoolType,
>   IN UINTN            AllocationSize,
>   IN CONST VOID       *Buffer
>   )
> {
>   VOID  *Memory;
>
>   ASSERT (Buffer != NULL);
>   ASSERT (AllocationSize <= (MAX_ADDRESS - (UINTN) Buffer + 1));
>   ...

This assert ensures that the copied buffer does not extend across the
end of the address space and wraps. This is a separate concern, and is
similar to numerous other occurrences of MAX_ADDRESS that maybe we
should update as well at some point. However, it does not affect page
allocation at all, it only puts an upper bound on the *size* of the
allocation. So the changes as they are will be sufficient to ensure
that AllocateCopyPool() does not allocate from a region that is not
addressable by the CPU.