Re: [edk2] [PATCH] MdePkg/BasePeCoffLib: skip runtime relocation if reloc info is invalid
Push at a824c7ebde0a431413329049252b8c1d3770de82
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Gao,
> Liming
> Sent: Thursday, January 31, 2019 11:31 AM
> To: Bi, Dandan ; Hsueh, Hong-chihX
> ; edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Laszlo Ersek
>
> Subject: Re: [edk2] [PATCH] MdePkg/BasePeCoffLib: skip runtime relocation if
> reloc info is invalid
>
> Reviewed-by: Liming Gao
>
> > -Original Message-
> > From: Bi, Dandan
> > Sent: Thursday, January 31, 2019 8:27 AM
> > To: Hsueh, Hong-chihX ; edk2-devel@lists.01.org
> > Cc: Kinney, Michael D ; Gao, Liming
> > ; Laszlo Ersek
> > Subject: RE: [PATCH] MdePkg/BasePeCoffLib: skip runtime relocation if reloc
> > info is invalid
> >
> > 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
> > > info
> > > is invalid
> > >
> > > 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
> > > 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(-)
> > >
> > > 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 == 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;
> > > @@ -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 = NULL;
> > > + RelocBaseEnd = NULL;
> > >if (NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) {
> > > RelocDir = DataDirectory + EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC;
> > > -RelocBase = (EFI_IMAGE_BASE_RELOCATION *)
> > > PeCoffLoaderImageAddress (, RelocDir->Vi
Re: [edk2] [PATCH] MdePkg/BasePeCoffLib: skip runtime relocation if reloc info is invalid
Reviewed-by: Liming Gao
> -Original Message-
> From: Bi, Dandan
> Sent: Thursday, January 31, 2019 8:27 AM
> To: Hsueh, Hong-chihX ; edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Gao, Liming
> ; Laszlo Ersek
> Subject: RE: [PATCH] MdePkg/BasePeCoffLib: skip runtime relocation if reloc
> info is invalid
>
> 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 info
> > is invalid
> >
> > 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
> > 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(-)
> >
> > 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 == 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;
> > @@ -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 = NULL;
> > + RelocBaseEnd = NULL;
> >if (NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) {
> > RelocDir = DataDirectory + EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC;
> > -RelocBase = (EFI_IMAGE_BASE_RELOCATION *)
> > PeCoffLoaderImageAddress (, RelocDir->VirtualAddress, 0);
> > -RelocBaseEnd = (EFI_IMAGE_BASE_RELOCATION *)
> > PeCoffLoaderImageAddress (,
> > -
> > RelocDir->VirtualAddress + RelocDir-
> > >Size - 1,
> > -
> > 0
> > -
> > );
> > +if ((RelocDir != NULL) && (RelocDir->Size > 0)) {
> > + RelocBase = (EFI_IMAGE_BASE_RELOCATION *)
> > PeCoffLoaderImageAddress (, RelocDir->VirtualAddress, 0);
> > + RelocBaseEnd = (EFI_IMAGE_BASE_RELOCATION *)
> > PeCoffLoaderImageAddress (,
> > +
> >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 +1779,7 @@ PeCoffLoaderRelocateImageForRuntime (
> > //
> > FixupData =
Re: [edk2] [PATCH] MdePkg/BasePeCoffLib: skip runtime relocation if reloc info is invalid
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 info
> is invalid
>
> 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
> 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(-)
>
> 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 == 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;
> @@ -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 = NULL;
> + RelocBaseEnd = NULL;
>if (NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) {
> RelocDir = DataDirectory + EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC;
> -RelocBase = (EFI_IMAGE_BASE_RELOCATION *)
> PeCoffLoaderImageAddress (, RelocDir->VirtualAddress, 0);
> -RelocBaseEnd = (EFI_IMAGE_BASE_RELOCATION *)
> PeCoffLoaderImageAddress (,
> -
> RelocDir->VirtualAddress + RelocDir-
> >Size - 1,
> -0
> -
> );
> +if ((RelocDir != NULL) && (RelocDir->Size > 0)) {
> + RelocBase = (EFI_IMAGE_BASE_RELOCATION *)
> PeCoffLoaderImageAddress (, RelocDir->VirtualAddress, 0);
> + RelocBaseEnd = (EFI_IMAGE_BASE_RELOCATION *)
> PeCoffLoaderImageAddress (,
> +
> 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 +1779,7 @@ PeCoffLoaderRelocateImageForRuntime (
> //
> FixupData = RelocationData;
> RelocBaseOrig = 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) {
>
> Fixup = PeCoffLoaderImageAddress (, RelocBase-
> >VirtualAddress + (*Reloc & 0xFFF), 0);
>
Re: [edk2] [PATCH] MdePkg/BasePeCoffLib: skip runtime relocation if reloc info is invalid
Hi,
I think this is maybe the 3rd or 4th iteration of the patch. That's OK,
but if you don't add v2, v3, v4 identifies to the subject prefix, such
as [PATCH v2], [PATCH v3] etc, then it's too difficult to follow what
one should review vs. what is obsolete.
Anyway, I think this version is good:
On 01/30/19 02:19, 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
> 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(-)
>
> 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 == 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;
> @@ -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 = NULL;
> + RelocBaseEnd = NULL;
>if (NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) {
> RelocDir = DataDirectory + EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC;
> -RelocBase = (EFI_IMAGE_BASE_RELOCATION *) PeCoffLoaderImageAddress
> (, RelocDir->VirtualAddress, 0);
> -RelocBaseEnd = (EFI_IMAGE_BASE_RELOCATION *) PeCoffLoaderImageAddress
> (,
> -
> RelocDir->VirtualAddress + RelocDir->Size - 1,
> -0
> -
> );
> +if ((RelocDir != NULL) && (RelocDir->Size > 0)) {
> + RelocBase = (EFI_IMAGE_BASE_RELOCATION *) PeCoffLoaderImageAddress
> (, RelocDir->VirtualAddress, 0);
> + RelocBaseEnd = (EFI_IMAGE_BASE_RELOCATION *) PeCoffLoaderImageAddress
> (,
> +
> 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 +1779,7 @@ PeCoffLoaderRelocateImageForRuntime (
> //
> FixupData = RelocationData;
> RelocBaseOrig = 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) {
>
> Fixup = PeCoffLoaderImageAddress (,
> RelocBase->VirtualAddress + (*Reloc & 0xFFF), 0);
> if
Re: [edk2] [PATCH] MdePkg/BasePeCoffLib: skip runtime relocation if reloc info is invalid
On 01/29/19 19:50, 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
> Cc: Michael D Kinney
> Cc: Liming Gao
> Cc: Dandan Bi
> Cc: Laszlo Ersek
> ---
> MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 28 ++--
> 1 file changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> index 1bd079ad6a..3a46e626e4 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;
> @@ -1741,11 +1741,19 @@ PeCoffLoaderRelocateImageForRuntime (
>//
>if (NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) {
> RelocDir = DataDirectory + EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC;
> -RelocBase = (EFI_IMAGE_BASE_RELOCATION *) PeCoffLoaderImageAddress
> (, RelocDir->VirtualAddress, 0);
> -RelocBaseEnd = (EFI_IMAGE_BASE_RELOCATION *) PeCoffLoaderImageAddress
> (,
> -
> RelocDir->VirtualAddress + RelocDir->Size - 1,
> -0
> -
> );
> +if ((RelocDir != NULL) && (RelocDir->Size > 0)) {
> + RelocBase = (EFI_IMAGE_BASE_RELOCATION *) PeCoffLoaderImageAddress
> (, RelocDir->VirtualAddress, 0);
> + RelocBaseEnd = (EFI_IMAGE_BASE_RELOCATION *) PeCoffLoaderImageAddress
> (,
> +
> RelocDir->VirtualAddress + RelocDir->Size - 1,
> +
> 0
> +
> );
> +}
If the above check fails, then RelocBase and RelocBaseEnd will not be
assigned, and the expressions below will check the previous values of
those variables. What guarantees that those values are valid / not
indeterminate?
Thanks
Laszlo
> +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 +1777,7 @@ PeCoffLoaderRelocateImageForRuntime (
> //
> FixupData = RelocationData;
> RelocBaseOrig = RelocBase;
> -while (RelocBase < RelocBaseEnd) {
> +while ((UINTN) RelocBase < (UINTN) RelocBaseEnd) {
>//
>// Add check for RelocBase->SizeOfBlock field.
>//
> @@ -1794,7 +1802,7 @@ PeCoffLoaderRelocateImageForRuntime (
>//
>// Run this relocation record
>//
> - while (Reloc < RelocEnd) {
> + while ((UINTN) Reloc < (UINTN) RelocEnd) {
>
> Fixup = PeCoffLoaderImageAddress (,
> RelocBase->VirtualAddress + (*Reloc & 0xFFF), 0);
> if (Fixup == NULL) {
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
