| Message ID | 20181130112829.12173-2-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 36a87fec6879b4bd96de88a7234f9a98c69af8e1 |
| Headers | show |
| Series | ArmVirtQemu: unmap page #0 to catch NULL pointer dereferences | expand |
On Fri, Nov 30, 2018 at 12:28:26PM +0100, 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. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > --- > 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) || > -- > 2.19.1 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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) || >
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) ||
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. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 3 +++ 1 file changed, 3 insertions(+) -- 2.19.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel