* [Patch] MdeModulePkg/PiSmmCore: Check valid memory range. @ 2018-08-21 6:45 Eric Dong 2018-08-21 9:35 ` Zeng, Star 0 siblings, 1 reply; 3+ messages in thread From: Eric Dong @ 2018-08-21 6:45 UTC (permalink / raw) To: edk2-devel; +Cc: Star Zeng 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.zeng@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong <eric.dong@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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Patch] MdeModulePkg/PiSmmCore: Check valid memory range. 2018-08-21 6:45 [Patch] MdeModulePkg/PiSmmCore: Check valid memory range Eric Dong @ 2018-08-21 9:35 ` Zeng, Star 2018-08-21 12:27 ` Dong, Eric 0 siblings, 1 reply; 3+ messages in thread From: Zeng, Star @ 2018-08-21 9:35 UTC (permalink / raw) To: Dong, Eric, edk2-devel@lists.01.org; +Cc: Zeng, Star 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.zeng@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.zeng@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong <eric.dong@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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Patch] MdeModulePkg/PiSmmCore: Check valid memory range. 2018-08-21 9:35 ` Zeng, Star @ 2018-08-21 12:27 ` Dong, Eric 0 siblings, 0 replies; 3+ messages in thread From: Dong, Eric @ 2018-08-21 12:27 UTC (permalink / raw) To: Zeng, Star, edk2-devel@lists.01.org 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.dong@intel.com>; edk2-devel@lists.01.org > Cc: Zeng, Star <star.zeng@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.zeng@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.zeng@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong <eric.dong@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 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-08-21 12:28 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-08-21 6:45 [Patch] MdeModulePkg/PiSmmCore: Check valid memory range Eric Dong 2018-08-21 9:35 ` Zeng, Star 2018-08-21 12:27 ` Dong, Eric
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox