From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=NwUoRd5u; spf=pass (domain: linaro.org, ip: 209.85.166.194, mailfrom: ard.biesheuvel@linaro.org) Received: from mail-it1-f194.google.com (mail-it1-f194.google.com [209.85.166.194]) by groups.io with SMTP; Tue, 16 Apr 2019 23:23:38 -0700 Received: by mail-it1-f194.google.com with SMTP id s3so2776930itk.1 for ; Tue, 16 Apr 2019 23:23:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=N2YGQ5pRKDpAPvb5rYZxSpThsDQAmuvqAIaVekfYQAM=; b=NwUoRd5upTUKCty3EfucPOTeDzuUTuD3x1rUol75d4AliGFVoug7JnArcysCN3UP6Q YuClnC4x2PfW5SIaVciNKgpz/ymP8XzkusEXYAj0jbX1QEK8XI0I8412u5DbGjI0Jo+f SDMiyaZhhkec7Vapt3SQxmcx4Iutked+MJRoJ0CJankLwrJMHVeHD8lLgpgTAZTodKsa p54wCvDfwmnolD/dyeEbD5xezyB4AUzVNt3dj2ppHs70YxPqpkbE281doHaHHr6TFlqH 2kKmZjtEH8i8fPb831S4OwNx8iqQX4/8seuMDjYOlWNaYXxsHnBmMZ04+ZYxQXFqYVz7 NzHw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=N2YGQ5pRKDpAPvb5rYZxSpThsDQAmuvqAIaVekfYQAM=; b=CmvDrjIuMuEu+7PUsV3uKVHNy7nG9JMnrCY6skPbcua2Sx1tETAWEOktAUpLewJ8vW ACkD+Fsy1G91/ZcNeBMEYhMhPl9wcrU0z6bQLKfiIiCWMX8i8mnRXmHyruvrjxq1VvW5 ggHYH/O2vz7HDpWNc0OdD9Yvyo6QlWxCzF/joM5aczOvmt5b8iK8tsycwF0cULZlz9eX m0MDxY+H25elDg0njZRLBo1LFmXatCUF6AAX3Ek3b6XpfSvxZ3/nb9bqhosPx/51oJPs SJDBziKJwBWSIQucauVjSqj0C6H6Mbz/ofPhsYQUuja56OKAyNNC8Bx3jvF6XfS5SCLg RjmQ== X-Gm-Message-State: APjAAAW8ecO9XAVakl8Q1kAhXXPUbEbX7czj+/Tu5NlcCgFdYot8++44 aIzws0VosrKzuMiZhiS33DqY0GS3yZ/EIhEYwhDi4A== X-Google-Smtp-Source: APXvYqw2a0ROTPv7IMeeb8Q9jO6AZ0KApKWEo7oxgZuxkJrwENiA1Rnuwc3gQsd+nTHpnMuLVfU+2AWSB27Tha8HeQ0= X-Received: by 2002:a02:658b:: with SMTP id u133mr52758079jab.17.1555482217138; Tue, 16 Apr 2019 23:23:37 -0700 (PDT) MIME-Version: 1.0 References: <20190416202935.32297-1-ard.biesheuvel@linaro.org> In-Reply-To: From: "Ard Biesheuvel" Date: Tue, 16 Apr 2019 23:23:28 -0700 Message-ID: Subject: Re: [PATCH] MdeModulePkg/UefiBootManagerLib: fix crash on uninitialized ExitData To: "Wu, Hao A" Cc: "Ni, Ray" , "devel@edk2.groups.io" , "Wang, Jian J" , "glin@suse.com" Content-Type: text/plain; charset="UTF-8" On Tue, 16 Apr 2019 at 23:07, Wu, Hao A wrote: > > > -----Original Message----- > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > > Sent: Wednesday, April 17, 2019 4:30 AM > > To: devel@edk2.groups.io > > Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; glin@suse.com; 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 > > --- > > 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