Hi Laszlo & Dandan, Here is the updated patch for your review. Thank you! https://lists.01.org/pipermail/edk2-devel/2019-January/035954.html
Regards, Neo > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Tuesday, January 29, 2019 2:57 AM > To: Hsueh, Hong-chihX <hong-chihx.hs...@intel.com>; edk2-devel@lists.01.org > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Gao, Liming > <liming....@intel.com>; Bi, Dandan <dandan...@intel.com> > Subject: Re: [PATCH] MdePkg/BasePeCoffLib: skip runtime relocation if > relocation info is invalid. > > On 01/29/19 00:22, Neo Hsueh wrote: > > Skip runtime relocation for PE images that provide invalid relocation > > infomation > > (ex: RelocDir->Size = 0) to fix a hang observed while booting Windows. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > Signed-off-by: Neo Hsueh <hong-chihx.hs...@intel.com> > > Cc: Michael D Kinney <michael.d.kin...@intel.com> > > Cc: Liming Gao <liming....@intel.com> > > Cc: Dandan Bi <dandan...@intel.com> > > Cc: Laszlo Ersek <ler...@redhat.com> > > --- > > MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 16 +++++++++++----- > > 1 file changed, 11 insertions(+), 5 deletions(-) > > > > diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c > > b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c > > index 1bd079ad6a..6d6c37bd61 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 == NULL || RelocBaseEnd == NULL || RelocBaseEnd < > RelocBase) { > > + if (RelocBase == NULL || RelocBaseEnd == NULL || (UINTN) > > + RelocBaseEnd < (UINTN) RelocBase) { > > ImageContext->ImageError = IMAGE_ERROR_FAILED_RELOCATION; > > return RETURN_LOAD_ERROR; > > } > > @@ -1022,7 +1022,7 @@ PeCoffLoaderRelocateImage ( > > // Run the relocation information and apply the fixups > > // > > FixupData = ImageContext->FixupData; > > - while (RelocBase < RelocBaseEnd) { > > + while ((UINTN) RelocBase < (UINTN) RelocBaseEnd) { > > > > Reloc = (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 = PeCoffLoaderImageAddress (ImageContext, RelocBase- > >VirtualAddress + (*Reloc & 0xFFF), TeStrippedOffset); > > if (Fixup == NULL) { > > ImageContext->ImageError = IMAGE_ERROR_FAILED_RELOCATION; > > @@ -1746,6 +1746,12 @@ PeCoffLoaderRelocateImageForRuntime ( > > > > RelocDir->VirtualAddress + > RelocDir->Size - 1, > > > > 0 > > > > ); > > + if (RelocBase == NULL || RelocBaseEnd == NULL || (UINTN) RelocBaseEnd > < (UINTN) RelocBase) { > > + // > > + // relocation block is not valid, just return > > + // > > + return; > > + } > > } else { > > // > > // Cannot find relocations, cannot continue to relocate the image, > > ASSERT > for this invalid image. > > @@ -1769,7 +1775,7 @@ PeCoffLoaderRelocateImageForRuntime ( > > // > > FixupData = RelocationData; > > RelocBaseOrig = RelocBase; > > - while (RelocBase < RelocBaseEnd) { > > + while ((UINTN) RelocBase < (UINTN) RelocBaseEnd) { > > // > > // Add check for RelocBase->SizeOfBlock field. > > // > > @@ -1794,7 +1800,7 @@ PeCoffLoaderRelocateImageForRuntime ( > > // > > // Run this relocation record > > // > > - while (Reloc < RelocEnd) { > > + while ((UINTN) Reloc < (UINTN) RelocEnd) { > > > > Fixup = PeCoffLoaderImageAddress (&ImageContext, RelocBase- > >VirtualAddress + (*Reloc & 0xFFF), 0); > > if (Fixup == NULL) { > > > > I think this patch is good enough, but it seems to miss the size==0 check that > Dandan asked for. > > Thanks, > laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
