Skip to content

Conversation

@lersek
Copy link
Member

@lersek lersek commented Nov 21, 2020

msgid [email protected]
https://edk2.groups.io/g/devel/message/67707
https://www.redhat.com/archives/edk2-devel-archive/2020-November/msg00865.html

Repo:   https://pagure.io/lersek/edk2.git
Branch: tianocore_1743_v2_resend
Ref:    https://bugzilla.tianocore.org/show_bug.cgi?id=1743

"RESEND" because I'm publicly posting the patches from
<https://bugzilla.tianocore.org/show_bug.cgi?id=1743#c19>.

The Reviewed-by tags on the patches originate from
<https://bugzilla.tianocore.org/show_bug.cgi?id=1743#c20> and
<https://bugzilla.tianocore.org/show_bug.cgi?id=1743#c22>.

Retested with Liming's reproducer; see
<https://bugzilla.tianocore.org/show_bug.cgi?id=1743#c16> and
<https://bugzilla.tianocore.org/show_bug.cgi?id=1743#c18>.

This series targets edk2-stable202011. I plan to merge it later this
week, based on Liming's R-b.

Liming, highlighting TianoCore#1743 in the "proposed features" list
could be useful.

Cc: Dandan Bi <[email protected]>
Cc: Hao A Wu <[email protected]>
Cc: Jian J Wang <[email protected]>
Cc: Liming Gao <[email protected]>
Cc: Philippe Mathieu-Daudé <[email protected]>

Thanks!
Laszlo

Laszlo Ersek (2):
  MdeModulePkg/Core/Dxe: assert SectionInstance invariant in
    FindChildNode()
  MdeModulePkg/Core/Dxe: limit FwVol encapsulation section recursion

 MdeModulePkg/MdeModulePkg.dec                                   |  6 +++
 MdeModulePkg/MdeModulePkg.uni                                   |  6 +++
 MdeModulePkg/Core/Dxe/DxeMain.inf                               |  1 +
 MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c | 52 +++++++++++++++++---
 4 files changed, 59 insertions(+), 6 deletions(-)

Laszlo Ersek added 2 commits November 21, 2020 02:33
…ode()

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 <[email protected]>
Cc: Hao A Wu <[email protected]>
Cc: Jian J Wang <[email protected]>
Cc: Liming Gao <[email protected]>
Cc: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Laszlo Ersek <[email protected]>
Reviewed-by: Liming Gao <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Message-Id: <[email protected]>
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 <[email protected]>
Cc: Hao A Wu <[email protected]>
Cc: Jian J Wang <[email protected]>
Cc: Liming Gao <[email protected]>
Cc: Philippe Mathieu-Daudé <[email protected]>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1743
Signed-off-by: Laszlo Ersek <[email protected]>
Reviewed-by: Liming Gao <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Message-Id: <[email protected]>
@lersek lersek added the push Auto push patch series in PR if all checks pass label Nov 21, 2020
@mergify mergify bot merged commit 47343af into tianocore:master Nov 21, 2020
@lersek lersek deleted the tianocore_1743_v2_resend_push branch November 21, 2020 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

push Auto push patch series in PR if all checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant