As reported by Gary, the recent LoadImage/StartImage changes to
accommodate dispatching PE/COFF images built for foreign architectures
may result in a crash when loading an IA32 option ROM into a X64 VM
running OVMF:

  Loading driver at 0x0007E537000 EntryPoint=0x0007E53C06D 8086100e.efi
  InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 7F003B98
  ProtectUefiImageCommon - 0x7F002BC0
    - 0x000000007E537000 - 0x000000000009F900
  Image type IA32 can't be started on X64 UEFI system.
  ASSERT MdeModulePkg/Core/Dxe/Mem/Pool.c(698): Head->Signature == ((('p') |
              ('h' << 8)) | ((('d') | ('0' << 8)) << 16)) || Head->Signature
              == ((('p') | ('h' << 8)) | ((('d') | ('1' << 8)) << 16))

This turns out to be caused by the deferred image loading code in BDS,
which doesn't check the result code of gBS->StartImage(), and ends up
trying to free an uninitialized pointer.

Given that ExitData is never actually used, let's just get rid of it
entirely. While we're at it, drop the pointless assignment of Status
as well.

Tested-by: Gary Lin <g...@suse.com>
Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
---
v2: drop ExitData entirely instead of initializing it to NULL

 MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
index fc8775dfa419..6b8fb4d92461 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
@@ -439,8 +439,6 @@ EfiBootManagerDispatchDeferredImages (
   UINTN                              ImageSize;
   BOOLEAN                            BootOption;
   EFI_HANDLE                         ImageHandle;
-  UINTN                              ExitDataSize;
-  CHAR16                             *ExitData;
   UINTN                              ImageCount;
   UINTN                              LoadCount;
 
@@ -502,10 +500,7 @@ EfiBootManagerDispatchDeferredImages (
         // a 5 Minute period
         //
         gBS->SetWatchdogTimer (5 * 60, 0x0000, 0x00, NULL);
-        Status = gBS->StartImage (ImageHandle, &ExitDataSize, &ExitData);
-        if (ExitData != NULL) {
-          FreePool (ExitData);
-        }
+        gBS->StartImage (ImageHandle, NULL, NULL);
 
         //
         // Clear the Watchdog Timer after the image returns.
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#39234): https://edk2.groups.io/g/devel/message/39234
Mute This Topic: https://groups.io/mt/31209297/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to