From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.31; helo=mga06.intel.com; envelope-from=dandan.bi@intel.com; receiver=edk2-devel@lists.01.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 9D60F211BA47D for ; Wed, 30 Jan 2019 16:27:17 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 30 Jan 2019 16:27:16 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,542,1539673200"; d="scan'208";a="139348238" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga002.fm.intel.com with ESMTP; 30 Jan 2019 16:27:16 -0800 Received: from fmsmsx152.amr.corp.intel.com (10.18.125.5) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.408.0; Wed, 30 Jan 2019 16:27:16 -0800 Received: from shsmsx105.ccr.corp.intel.com (10.239.4.158) by FMSMSX152.amr.corp.intel.com (10.18.125.5) with Microsoft SMTP Server (TLS) id 14.3.408.0; Wed, 30 Jan 2019 16:27:15 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.102]) by SHSMSX105.ccr.corp.intel.com ([169.254.11.253]) with mapi id 14.03.0415.000; Thu, 31 Jan 2019 08:27:13 +0800 From: "Bi, Dandan" To: "Hsueh, Hong-chihX" , "edk2-devel@lists.01.org" CC: "Kinney, Michael D" , "Gao, Liming" , Laszlo Ersek Thread-Topic: [PATCH] MdePkg/BasePeCoffLib: skip runtime relocation if reloc info is invalid Thread-Index: AQHUuDngzqAzS0JzaUe1sgHDTStz/KXIhdPA Date: Thu, 31 Jan 2019 00:27:12 +0000 Message-ID: <3C0D5C461C9E904E8F62152F6274C0BB40B8EFC5@SHSMSX104.ccr.corp.intel.com> References: <20190130011936.5804-1-hong-chihx.hsueh@intel.com> In-Reply-To: <20190130011936.5804-1-hong-chihx.hsueh@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] MdePkg/BasePeCoffLib: skip runtime relocation if reloc info is invalid X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 31 Jan 2019 00:27:17 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Bi Dandan Thanks, Dandan > -----Original Message----- > From: Hsueh, Hong-chihX > Sent: Wednesday, January 30, 2019 9:20 AM > To: edk2-devel@lists.01.org > Cc: Kinney, Michael D ; Gao, Liming > ; Bi, Dandan ; Laszlo Ersek > > Subject: [PATCH] MdePkg/BasePeCoffLib: skip runtime relocation if reloc i= nfo > is invalid >=20 > Skip runtime relocation for PE images that provide invalid relocation > infomation (ex: RelocDir->Size =3D 0) to fix a hang observed while bootin= g > Windows. >=20 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Neo Hsueh > Cc: Michael D Kinney > Cc: Liming Gao > Cc: Dandan Bi > Cc: Laszlo Ersek > --- > MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 30 > ++++++++++++++++++++---------- > 1 file changed, 20 insertions(+), 10 deletions(-) >=20 > diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c > b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c > index 1bd079ad6a..e2c62e1932 100644 > --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c > +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c > @@ -1002,7 +1002,7 @@ PeCoffLoaderRelocateImage ( > = RelocDir->VirtualAddress + RelocDir- > >Size - 1, > = TeStrippedOffset > = ); > - if (RelocBase =3D=3D NULL || RelocBaseEnd =3D=3D NULL || RelocBaseEn= d < > RelocBase) { > + if (RelocBase =3D=3D NULL || RelocBaseEnd =3D=3D NULL || (UINTN) > + RelocBaseEnd < (UINTN) RelocBase) { > ImageContext->ImageError =3D IMAGE_ERROR_FAILED_RELOCATION; > return RETURN_LOAD_ERROR; > } > @@ -1022,7 +1022,7 @@ PeCoffLoaderRelocateImage ( > // Run the relocation information and apply the fixups > // > FixupData =3D ImageContext->FixupData; > - while (RelocBase < RelocBaseEnd) { > + while ((UINTN) RelocBase < (UINTN) RelocBaseEnd) { >=20 > Reloc =3D (UINT16 *) ((CHAR8 *) RelocBase + sizeof > (EFI_IMAGE_BASE_RELOCATION)); > // > @@ -1051,7 +1051,7 @@ PeCoffLoaderRelocateImage ( > // > // Run this relocation record > // > - while (Reloc < RelocEnd) { > + while ((UINTN) Reloc < (UINTN) RelocEnd) { > Fixup =3D PeCoffLoaderImageAddress (ImageContext, RelocBase- > >VirtualAddress + (*Reloc & 0xFFF), TeStrippedOffset); > if (Fixup =3D=3D NULL) { > ImageContext->ImageError =3D IMAGE_ERROR_FAILED_RELOCATION; > @@ -1739,13 +1739,23 @@ PeCoffLoaderRelocateImageForRuntime ( > // is present in the image. You have to check the NumberOfRvaAndSizes = in > // the optional header to verify a desired directory entry is there. > // > + RelocBase =3D NULL; > + RelocBaseEnd =3D NULL; > if (NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) { > RelocDir =3D DataDirectory + EFI_IMAGE_DIRECTORY_ENTRY_BASERELO= C; > - RelocBase =3D (EFI_IMAGE_BASE_RELOCATION *) > PeCoffLoaderImageAddress (&ImageContext, RelocDir->VirtualAddress, 0); > - RelocBaseEnd =3D (EFI_IMAGE_BASE_RELOCATION *) > PeCoffLoaderImageAddress (&ImageContext, > - = RelocDir->VirtualAddress + RelocDir- > >Size - 1, > - = 0 > - = ); > + if ((RelocDir !=3D NULL) && (RelocDir->Size > 0)) { > + RelocBase =3D (EFI_IMAGE_BASE_RELOCATION *) > PeCoffLoaderImageAddress (&ImageContext, RelocDir->VirtualAddress, 0); > + RelocBaseEnd =3D (EFI_IMAGE_BASE_RELOCATION *) > PeCoffLoaderImageAddress (&ImageContext, > + = RelocDir->VirtualAddress + RelocDir- > >Size - 1, > + = 0 > + = ); > + } > + if (RelocBase =3D=3D NULL || RelocBaseEnd =3D=3D NULL || (UINTN) Rel= ocBaseEnd > < (UINTN) RelocBase) { > + // > + // relocation block is not valid, just return > + // > + return; > + } > } else { > // > // Cannot find relocations, cannot continue to relocate the image, A= SSERT > for this invalid image. > @@ -1769,7 +1779,7 @@ PeCoffLoaderRelocateImageForRuntime ( > // > FixupData =3D RelocationData; > RelocBaseOrig =3D RelocBase; > - while (RelocBase < RelocBaseEnd) { > + while ((UINTN) RelocBase < (UINTN) RelocBaseEnd) { > // > // Add check for RelocBase->SizeOfBlock field. > // > @@ -1794,7 +1804,7 @@ PeCoffLoaderRelocateImageForRuntime ( > // > // Run this relocation record > // > - while (Reloc < RelocEnd) { > + while ((UINTN) Reloc < (UINTN) RelocEnd) { >=20 > Fixup =3D PeCoffLoaderImageAddress (&ImageContext, RelocBase- > >VirtualAddress + (*Reloc & 0xFFF), 0); > if (Fixup =3D=3D NULL) { > -- > 2.16.2.windows.1