Hi Star, Good catch. I tried to boot an test platform and it passed. I assume it has cases to free the smm memory. Seems like actually it doesn't.
I will add test code to verify the patch and send v2 change. Thanks, Eric > -----Original Message----- > From: Zeng, Star > Sent: Tuesday, August 21, 2018 5:35 PM > To: Dong, Eric <eric.d...@intel.com>; edk2-devel@lists.01.org > Cc: Zeng, Star <star.z...@intel.com> > Subject: RE: [Patch] MdeModulePkg/PiSmmCore: Check valid memory range. > > Hi Eric, > > About > + EFI_PHYSICAL_ADDRESS Last; > + Last = Memory + EFI_PAGES_TO_SIZE (NumberOfPages); > > Shouldn't "-1" be also added like the code below in > ConvertSmmMemoryMapEntry()? > EFI_PHYSICAL_ADDRESS End; > End = Memory + EFI_PAGES_TO_SIZE(NumberOfPages) - 1; > > Could you double check the normal functionality (SMM AllocatePages + > FreePages) with the patch? > > > Thanks, > Star > -----Original Message----- > From: Dong, Eric > Sent: Tuesday, August 21, 2018 2:46 PM > To: edk2-devel@lists.01.org > Cc: Zeng, Star <star.z...@intel.com> > Subject: [Patch] MdeModulePkg/PiSmmCore: Check valid memory range. > > Call BS.AllocatePages in DXE driver and call SMM FreePages with the address > of the buffer allocated in the DXE driver. SMM FreePages success and add a > non-SMRAM range into SMM heap list. This is not an expected behavior. > SMM FreePages should return error for this case and not free the pages. > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1098 > > Cc: Star Zeng <star.z...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong <eric.d...@intel.com> > --- > MdeModulePkg/Core/PiSmmCore/Page.c | 39 > ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/MdeModulePkg/Core/PiSmmCore/Page.c > b/MdeModulePkg/Core/PiSmmCore/Page.c > index 3699af7424..630678ccc3 100644 > --- a/MdeModulePkg/Core/PiSmmCore/Page.c > +++ b/MdeModulePkg/Core/PiSmmCore/Page.c > @@ -983,6 +983,41 @@ SmmInternalFreePages ( > return SmmInternalFreePagesEx (Memory, NumberOfPages, FALSE); } > > +/** > + Check whether the input range is in smram. > + > + @param Memory Base address of memory being inputed. > + @param NumberOfPages The number of pages. > + > + @retval TRUE In the smram. > + @retval FALSE Not in the smram. > + > +**/ > +BOOLEAN > +InSmmRange ( > + IN EFI_PHYSICAL_ADDRESS Memory, > + IN UINTN NumberOfPages > + ) > +{ > + LIST_ENTRY *Link; > + MEMORY_MAP *Entry; > + EFI_PHYSICAL_ADDRESS Last; > + > + Last = Memory + EFI_PAGES_TO_SIZE (NumberOfPages); > + > + Link = gMemoryMap.ForwardLink; > + while (Link != &gMemoryMap) { > + Entry = CR (Link, MEMORY_MAP, Link, MEMORY_MAP_SIGNATURE); > + Link = Link->ForwardLink; > + > + if ((Entry->Start <= Memory) && (Entry->End >= Last)) { > + return TRUE; > + } > + } > + > + return FALSE; > +} > + > /** > Frees previous allocated pages. > > @@ -1004,6 +1039,10 @@ SmmFreePages ( > EFI_STATUS Status; > BOOLEAN IsGuarded; > > + if (!InSmmRange(Memory, NumberOfPages)) { > + return EFI_NOT_FOUND; > + } > + > IsGuarded = IsHeapGuardEnabled () && IsMemoryGuarded (Memory); > Status = SmmInternalFreePages (Memory, NumberOfPages, IsGuarded); > if (!EFI_ERROR (Status)) { > -- > 2.15.0.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
