Re: [PATCH v2 1/4] ArmPkg/ArmMmuLib ARM: handle unmapped section in GetMemoryRegion()

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 30/11/18 12:28, Ard Biesheuvel wrote:
> GetMemoryRegion() is used to obtain the attributes of an existing
> mapping, to permit permission attribute changes to be optimized
> away if the attributes don't actually change.
> 
> The current ARM code assumes that a section mapping or a page mapping
> exists for any region passed into GetMemoryRegion(), but the region
> may be unmapped entirely, in which case the code will crash. So check
> if a section mapping exists before dereferencing it.

Good catch!

> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
> index 12ca5b26673e..3b29d33d0a9c 100644
> --- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
> +++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
> @@ -457,6 +457,9 @@ GetMemoryRegion (
>  
>    // Get the section at the given index
>    SectionDescriptor = FirstLevelTable[TableIndex];
> +  if (!SectionDescriptor) {
> +    return EFI_NOT_FOUND;
> +  }
>  
>    // If 'BaseAddress' belongs to the section then round it to the section boundary
>    if (((SectionDescriptor & TT_DESCRIPTOR_SECTION_TYPE_MASK) == TT_DESCRIPTOR_SECTION_TYPE_SECTION) ||
>