[edk2,1/5] ArmPkg/ArmMmuLib AARCH64: fix out of bounds access

Message ID 20190107071504.2431-2-ard.biesheuvel@linaro.org
State Accepted
Commit 76c23f9e0d0d65866e4195b0bc12c1ca2763ced2
Headers show
Series
  • memory/MMU hardening for AArch64
Related show

Commit Message

Ard Biesheuvel Jan. 7, 2019, 7:15 a.m.
Take care not to dereference BlockEntry if it may be pointing past
the end of the page table we are manipulating. It is only a read,
and thus harmless, but HeapGuard triggers on it so let's fix it.

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

---
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.20.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Jan. 14, 2019, noon | #1
On Mon, Jan 07, 2019 at 08:15:00AM +0100, Ard Biesheuvel wrote:
> Take care not to dereference BlockEntry if it may be pointing past

> the end of the page table we are manipulating. It is only a read,

> and thus harmless, but HeapGuard triggers on it so let's fix 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/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> index e41044142ef4..d66df3e17a02 100644

> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> @@ -382,7 +382,7 @@ UpdateRegionMapping (

>  

>        // Break the inner loop when next block is a table

>        // Rerun GetBlockEntryListFromAddress to avoid page table memory leak

> -      if (TableLevel != 3 &&

> +      if (TableLevel != 3 && BlockEntry <= LastBlockEntry &&

>            (*BlockEntry & TT_TYPE_MASK) == TT_TYPE_TABLE_ENTRY) {

>              break;

>        }

> -- 

> 2.20.1

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Jan. 14, 2019, 6:48 p.m. | #2
On Mon, 14 Jan 2019 at 13:00, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>

> On Mon, Jan 07, 2019 at 08:15:00AM +0100, Ard Biesheuvel wrote:

> > Take care not to dereference BlockEntry if it may be pointing past

> > the end of the page table we are manipulating. It is only a read,

> > and thus harmless, but HeapGuard triggers on it so let's fix 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>

>


Thanks

Pushed as d08575759e5a..76c23f9e0d0d

> > ---

> >  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 2 +-

> >  1 file changed, 1 insertion(+), 1 deletion(-)

> >

> > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> > index e41044142ef4..d66df3e17a02 100644

> > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> > @@ -382,7 +382,7 @@ UpdateRegionMapping (

> >

> >        // Break the inner loop when next block is a table

> >        // Rerun GetBlockEntryListFromAddress to avoid page table memory leak

> > -      if (TableLevel != 3 &&

> > +      if (TableLevel != 3 && BlockEntry <= LastBlockEntry &&

> >            (*BlockEntry & TT_TYPE_MASK) == TT_TYPE_TABLE_ENTRY) {

> >              break;

> >        }

> > --

> > 2.20.1

> >

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index e41044142ef4..d66df3e17a02 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -382,7 +382,7 @@  UpdateRegionMapping (
 
       // Break the inner loop when next block is a table
       // Rerun GetBlockEntryListFromAddress to avoid page table memory leak
-      if (TableLevel != 3 &&
+      if (TableLevel != 3 && BlockEntry <= LastBlockEntry &&
           (*BlockEntry & TT_TYPE_MASK) == TT_TYPE_TABLE_ENTRY) {
             break;
       }