On Fri, Apr 13, 2018 at 11:43:27PM +0000, Chris Co wrote: > Mva address calculation should use the left-shifted current > section index instead of the left-shifted table base address. > > Using the table base address here has the side-effect of potentially > causing an access violation depending on the base address value. > > Cc: Leif Lindholm <leif.lindh...@linaro.org> > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Christopher Co <christopher...@microsoft.com> > --- > ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c > b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c > index 774a7ccf59..9bf4ba03fd 100644 > --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c > +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c > @@ -716,7 +716,7 @@ UpdateSectionEntries ( > Descriptor |= EntryValue; > > if (CurrentDescriptor != Descriptor) { > - Mva = (VOID *)(UINTN)(((UINTN)FirstLevelTable) << > TT_DESCRIPTOR_SECTION_BASE_SHIFT); > + Mva = (VOID *)(UINTN)(((UINTN)FirstLevelIdx + i) << > TT_DESCRIPTOR_SECTION_BASE_SHIFT);
So, this clearly looks like you've found a bug - thanks! But I am a little bit confused about the patch - should this not need to incorporate the descriptor size in some way? I.e. something like Mva = (VOID *)(UINTN)(((UINTN)FirstLevelIdx + (i * sizeof(UINTN))) << TT_DESCRIPTOR_SECTION_BASE_SHIFT); or ... &FirstLevelTable[FirstLevelIndex + i] ... ? Regards, Leif > > // Clean/invalidate the cache for this section, but only > // if we are modifying the memory type attributes > -- > 2.15.1.gvfs.2.39.g03d366a > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel