回复: [edk2-devel] [Proposed Fix][PATCH v1 1/1] MdeModulePkg/DxeCapsuleLibFmp: Fix crash with VirtualAddressMap omitted

["gaoliming via groups.io" <gaoliming=byosoft.com.cn@groups.io>]

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Ard
> Biesheuvel
> 发送时间: 2023年11月23日 16:36
> 收件人: Nhi Pham <nhi@os.amperecomputing.com>
> 抄送: devel@edk2.groups.io; gaoliming@byosoft.com.cn;
> ardb+tianocore@kernel.org
> 主题: Re: [edk2-devel] [Proposed Fix][PATCH v1 1/1]
> MdeModulePkg/DxeCapsuleLibFmp: Fix crash with VirtualAddressMap
> omitted
> 
> On Thu, 23 Nov 2023 at 05:49, Nhi Pham <nhi@os.amperecomputing.com>
> wrote:
> >
> > If the SetVirtualAddressMap() is not called, mIsVirtualAddrConverted
> > is FALSE and the kernel crash occurs in IsNestedFmpCapsule() when
> > executing gBS->LocateProtocol () in the else case.
> >
> > To serve the omitted SetVirtualAddressMap() call, we could just check
> > mEsrtTable presence instead of relying on mIsVirtualAddrConverted.
> >
> > Signed-off-by: Nhi Pham <nhi@os.amperecomputing.com>
> 
> Agreed: SetVirtualAddressMap() is optional, so whether it has been
> called or not should not be used as a proxy to decide whether or not
> we are still running in the boot services context.
> 
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> 
> 
> > ---
> >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c     | 17
> +++++++----------
> >  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c |  2 --
> >  2 files changed, 7 insertions(+), 12 deletions(-)
> >
> > diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> > index 197af267aff3..2433c76a8c6f 100644
> > --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> > +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> > @@ -41,8 +41,7 @@
> >  #include <Protocol/FirmwareManagementProgress.h>
> >  #include <Protocol/DevicePath.h>
> >
> > -EFI_SYSTEM_RESOURCE_TABLE  *mEsrtTable             = NULL;
> > -BOOLEAN                    mIsVirtualAddrConverted = FALSE;
> > +EFI_SYSTEM_RESOURCE_TABLE  *mEsrtTable = NULL;
> >
> >  BOOLEAN    mDxeCapsuleLibEndOfDxe      = FALSE;
> >  EFI_EVENT  mDxeCapsuleLibEndOfDxeEvent = NULL;
> > @@ -1394,14 +1393,12 @@ IsNestedFmpCapsule (
> >    EFI_SYSTEM_RESOURCE_ENTRY  Entry;
> >
> >    EsrtGuidFound = FALSE;
> > -  if (mIsVirtualAddrConverted) {
> > -    if (mEsrtTable != NULL) {
> > -      EsrtEntry = (EFI_SYSTEM_RESOURCE_ENTRY *)(mEsrtTable + 1);
> > -      for (Index = 0; Index < mEsrtTable->FwResourceCount; Index++,
> EsrtEntry++) {
> > -        if (CompareGuid (&EsrtEntry->FwClass,
> &CapsuleHeader->CapsuleGuid)) {
> > -          EsrtGuidFound = TRUE;
> > -          break;
> > -        }
> > +  if (mEsrtTable != NULL) {
> > +    EsrtEntry = (EFI_SYSTEM_RESOURCE_ENTRY *)(mEsrtTable + 1);
> > +    for (Index = 0; Index < mEsrtTable->FwResourceCount; Index++,
> EsrtEntry++) {
> > +      if (CompareGuid (&EsrtEntry->FwClass,
> &CapsuleHeader->CapsuleGuid)) {
> > +        EsrtGuidFound = TRUE;
> > +        break;
> >        }
> >      }
> >    } else {
> > diff --git
> a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> > index 9ceb44fe4d79..44f30c16c284 100644
> > --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> > +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
> > @@ -21,7 +21,6 @@
> >  #include <Library/MemoryAllocationLib.h>
> >
> >  extern EFI_SYSTEM_RESOURCE_TABLE  *mEsrtTable;
> > -extern BOOLEAN                    mIsVirtualAddrConverted;
> >  EFI_EVENT
> mDxeRuntimeCapsuleLibVirtualAddressChangeEvent = NULL;
> >  EFI_EVENT
> mDxeRuntimeCapsuleLibReadyToBootEvent          = NULL;
> >
> > @@ -40,7 +39,6 @@ DxeCapsuleLibVirtualAddressChangeEvent (
> >    )
> >  {
> >    gRT->ConvertPointer (EFI_OPTIONAL_PTR, (VOID **)&mEsrtTable);
> > -  mIsVirtualAddrConverted = TRUE;
> >  }
> >
> >  /**
> > --
> > 2.25.1
> >
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111812): https://edk2.groups.io/g/devel/message/111812
Mute This Topic: https://groups.io/mt/102851947/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-