Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE:Step:15
ovmf.28635
ovmf-bsc1183579-fix-fv-recursion.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File ovmf-bsc1183579-fix-fv-recursion.patch of Package ovmf.28635
From 58d623915eadfb5059bab97369c09691ca368ae0 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek <lersek@redhat.com> Date: Thu, 19 Nov 2020 11:53:39 +0100 Subject: [PATCH 1/2] MdeModulePkg/Core/Dxe: assert SectionInstance invariant in FindChildNode() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FindChildNode() has two callers: GetSection(), and FindChildNode() itself. - At the GetSection() call site, a positive (i.e., nonzero) SectionInstance is passed. This is because GetSection() takes a zero-based (UINTN) SectionInstance, and then passes Instance=(SectionInstance+1) to FindChildNode(). - For reaching the recursive FindChildNode() call site, a section type mismatch, or a section instance mismatch, is necessary. This means, respectively, that SectionInstance will either not have been decreased, or not to zero anyway, at the recursive FindChildNode() call site. Add two ASSERT()s to FindChildNode(), for expressing the (SectionSize>0) invariant. In turn, the invariant provides the explanation why, after the recursive call, a zero SectionInstance implies success. Capture it in a comment. Cc: Dandan Bi <dandan.bi@intel.com> Cc: Hao A Wu <hao.a.wu@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20201119105340.16225-2-lersek@redhat.com> (cherry picked from commit b9bdfc72853fe97bd24401f8873ca61524dd2dc6) --- .../SectionExtraction/CoreSectionExtraction.c | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c b/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c index 9561ae176655..990ae5e91220 100644 --- a/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c +++ b/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c @@ -958,8 +958,8 @@ CreateChildNode ( search. @param SearchType Indicates the type of section to search for. @param SectionInstance Indicates which instance of section to find. - This is an in/out parameter to deal with - recursions. + This is an in/out parameter and it is 1-based, + to deal with recursions. @param SectionDefinitionGuid Guid of section definition @param FoundChild Output indicating the child node that is found. @param FoundStream Output indicating which section stream the child @@ -994,6 +994,8 @@ FindChildNode ( EFI_STATUS ErrorStatus; EFI_STATUS Status; + ASSERT (*SectionInstance > 0); + CurrentChildNode = NULL; ErrorStatus = EFI_NOT_FOUND; @@ -1043,6 +1045,11 @@ FindChildNode ( } } + // + // Type mismatch, or we haven't found the desired instance yet. + // + ASSERT (*SectionInstance > 0); + if (CurrentChildNode->EncapsulatedStreamHandle != NULL_STREAM_HANDLE) { // // If the current node is an encapsulating node, recurse into it... @@ -1056,16 +1063,20 @@ FindChildNode ( &RecursedFoundStream, AuthenticationStatus ); - // - // If the status is not EFI_SUCCESS, just save the error code and continue - // to find the request child node in the rest stream. - // if (*SectionInstance == 0) { + // + // The recursive FindChildNode() call decreased (*SectionInstance) to + // zero. + // ASSERT_EFI_ERROR (Status); *FoundChild = RecursedChildNode; *FoundStream = RecursedFoundStream; return EFI_SUCCESS; } else { + // + // If the status is not EFI_SUCCESS, just save the error code and + // continue to find the request child node in the rest stream. + // ErrorStatus = Status; } } else if ((CurrentChildNode->Type == EFI_SECTION_GUID_DEFINED) && (SearchType != EFI_SECTION_GUID_DEFINED)) { -- 2.29.2 From 44f75eda3db3883f8481ef29354d0a72bfd50251 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek <lersek@redhat.com> Date: Thu, 19 Nov 2020 11:53:40 +0100 Subject: [PATCH 2/2] MdeModulePkg/Core/Dxe: limit FwVol encapsulation section recursion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DXE Core sets up a protocol notify function in its entry point, for instances of the Firmware Volume Block2 Protocol: DxeMain() [DxeMain/DxeMain.c] FwVolDriverInit() [FwVol/FwVol.c] Assume that a 3rd party UEFI driver or application installs an FVB instance, with crafted contents. The notification function runs: NotifyFwVolBlock() [FwVol/FwVol.c] installing an instance of the Firmware Volume 2 Protocol on the handle. (Alternatively, assume that a 3rd party application calls gDS->ProcessFirmwareVolume(), which may also produce a Firmware Volume 2 Protocol instance.) The EFI_FIRMWARE_VOLUME2_PROTOCOL.ReadSection() member performs "a depth-first, left-to-right search algorithm through all sections found in the specified file" (quoting the PI spec), as follows: FvReadFileSection() [FwVol/FwVolRead.c] GetSection() [SectionExtraction/CoreSectionExtraction.c] FindChildNode() [SectionExtraction/CoreSectionExtraction.c] FindChildNode() // recursive call FindChildNode() is called recursively for encapsulation sections. Currently this recursion is not limited. Introduce a new PCD (fixed-at-build, or patchable-in-module), and make FindChildNode() track the section nesting depth against that PCD. Cc: Dandan Bi <dandan.bi@intel.com> Cc: Hao A Wu <hao.a.wu@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1743 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20201119105340.16225-3-lersek@redhat.com> (cherry picked from commit 47343af30435302c087027177613412a1a83e919) --- MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + .../SectionExtraction/CoreSectionExtraction.c | 33 +++++++++++++++++-- MdeModulePkg/MdeModulePkg.dec | 6 ++++ MdeModulePkg/MdeModulePkg.uni | 6 ++++ 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf index 9793333a4491..7b3da70a2815 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain.inf +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf @@ -197,6 +197,7 @@ [Pcd] gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth ## CONSUMES # [Hob] # RESOURCE_DESCRIPTOR ## CONSUMES diff --git a/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c b/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c index 990ae5e91220..a0e0aa782542 100644 --- a/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c +++ b/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c @@ -961,6 +961,9 @@ CreateChildNode ( This is an in/out parameter and it is 1-based, to deal with recursions. @param SectionDefinitionGuid Guid of section definition + @param Depth Nesting depth of encapsulation sections. + Callers different from FindChildNode() are + responsible for passing in a zero Depth. @param FoundChild Output indicating the child node that is found. @param FoundStream Output indicating which section stream the child was found in. If this stream was generated as a @@ -974,6 +977,9 @@ CreateChildNode ( @retval EFI_NOT_FOUND Requested child node does not exist. @retval EFI_PROTOCOL_ERROR a required GUIDED section extraction protocol does not exist + @retval EFI_ABORTED Recursion aborted because Depth has been + greater than or equal to + PcdFwVolDxeMaxEncapsulationDepth. **/ EFI_STATUS @@ -982,6 +988,7 @@ FindChildNode ( IN EFI_SECTION_TYPE SearchType, IN OUT UINTN *SectionInstance, IN EFI_GUID *SectionDefinitionGuid, + IN UINT32 Depth, OUT CORE_SECTION_CHILD_NODE **FoundChild, OUT CORE_SECTION_STREAM_NODE **FoundStream, OUT UINT32 *AuthenticationStatus @@ -996,6 +1003,10 @@ FindChildNode ( ASSERT (*SectionInstance > 0); + if (Depth >= PcdGet32 (PcdFwVolDxeMaxEncapsulationDepth)) { + return EFI_ABORTED; + } + CurrentChildNode = NULL; ErrorStatus = EFI_NOT_FOUND; @@ -1059,6 +1070,7 @@ FindChildNode ( SearchType, SectionInstance, SectionDefinitionGuid, + Depth + 1, &RecursedChildNode, &RecursedFoundStream, AuthenticationStatus @@ -1073,9 +1085,17 @@ FindChildNode ( *FoundStream = RecursedFoundStream; return EFI_SUCCESS; } else { + if (Status == EFI_ABORTED) { + // + // If the recursive call was aborted due to nesting depth, stop + // looking for the requested child node. The skipped subtree could + // throw off the instance counting. + // + return Status; + } // - // If the status is not EFI_SUCCESS, just save the error code and - // continue to find the request child node in the rest stream. + // Save the error code and continue to find the requested child node in + // the rest of the stream. // ErrorStatus = Status; } @@ -1278,11 +1298,20 @@ GetSection ( *SectionType, &Instance, SectionDefinitionGuid, + 0, // encapsulation depth &ChildNode, &ChildStreamNode, &ExtractedAuthenticationStatus ); if (EFI_ERROR (Status)) { + if (Status == EFI_ABORTED) { + DEBUG ((DEBUG_ERROR, "%a: recursion aborted due to nesting depth\n", + __FUNCTION__)); + // + // Map "aborted" to "not found". + // + Status = EFI_NOT_FOUND; + } goto GetSection_Done; } diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 40d5cd5fb6d9..f405795c54f1 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -1383,6 +1383,12 @@ [PcdsFixedAtBuild, PcdsPatchableInModule] # @Prompt CapsuleMax value in capsule report variable. gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleMax|0xFFFF|UINT16|0x00000107 + ## Maximum permitted encapsulation levels of sections in a firmware volume, + # in the DXE phase. Minimum value is 1. Sections nested more deeply are + # rejected. + # @Prompt Maximum permitted FwVol section nesting depth (exclusive). + gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth|0x10|UINT32|0x00000030 + [PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] ## This PCD defines the Console output row. The default value is 25 according to UEFI spec. # This PCD could be set to 0 then console output would be at max column and max row. diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index 827fbbccf95a..93b37eda908c 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -1127,6 +1127,12 @@ "enabled on AMD processors supporting the Secure Encrypted Virtualization (SEV) feature.\n" "This mask should be applied when creating 1:1 virtual to physical mapping tables." +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdFwVolDxeMaxEncapsulationDepth_PROMPT #language en-US "Maximum permitted FwVol section nesting depth (exclusive)." + +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdFwVolDxeMaxEncapsulationDepth_HELP #language en-US "Maximum permitted encapsulation levels of sections in a firmware volume,<BR>" + "in the DXE phase. Minimum value is 1. Sections nested more deeply are<BR>" + "rejected." + #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNullPointerDetectionPropertyMask_PROMPT #language en-US "Enable NULL pointer detection" #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNullPointerDetectionPropertyMask_HELP #language en-US "Mask to control the NULL address detection in code for different phases.\n" -- 2.29.2
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
