> -----Original Message-----
> From: Ard Biesheuvel [mailto:[email protected]]
> Sent: Wednesday, April 17, 2019 4:30 AM
> To: [email protected]
> Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; [email protected]; Ard Biesheuvel
> Subject: [PATCH] MdeModulePkg/UefiBootManagerLib: fix crash on
> uninitialized ExitData
> 
> 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. So ensure ExitData is initialized
> before the call.
> 
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
> index fc8775dfa419..cf99de5b924a 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
> @@ -502,6 +502,7 @@ EfiBootManagerDispatchDeferredImages (
>          // a 5 Minute period
>          //
>          gBS->SetWatchdogTimer (5 * 60, 0x0000, 0x00, NULL);
> +        ExitData = NULL;
>          Status = gBS->StartImage (ImageHandle, &ExitDataSize, &ExitData);
>          if (ExitData != NULL) {
>            FreePool (ExitData);

Looks like the 'ExitData' is not being used at all here.

Ray and Ard,

Do you see any concern to just pass 'NULL' as the 3rd parameter
(eliminates 'ExitData') here?

Best Regards,
Hao Wu

> --
> 2.17.1


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

View/Reply Online (#39202): https://edk2.groups.io/g/devel/message/39202
Mute This Topic: https://groups.io/mt/31204960/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to