On Tue, 16 Apr 2019 at 23:07, Wu, Hao A <[email protected]> wrote: > > > -----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? >
Yes, that would be even better, actually, I did not realize it was optional -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#39204): https://edk2.groups.io/g/devel/message/39204 Mute This Topic: https://groups.io/mt/31204960/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
