Re: [PATCH] MdePkg/BasePeCoffLib: skip runtime relocation if relocation info is invalid.

["Bi, Dandan" <dandan.bi@intel.com>]

Hi Neo,

Thank you very much for the patch.

Some minor comments
1) Besides the skip check in this patch, could you help to add additional  check for RelocDir->Size before calling PeCoffLoaderImageAddress to calculate the RelocBase and RelocBaseEnd?
Since when RelocDir->Size==0, we can just return, don't need to do the calculation.

2) Please use the PatchCheck.py in edk2\BaseTools\Scripts to check the patch format before committing  the patch.
Can refer following link for more info:
https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format


Thanks,
Dandan

> -----Original Message-----
> From: Hsueh, Hong-chihX
> Sent: Tuesday, January 29, 2019 2:41 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Laszlo Ersek
> <lersek@redhat.com>
> Subject: [PATCH] MdePkg/BasePeCoffLib: skip runtime relocation if
> relocation 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 <hong-chihx.hsueh@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
>  MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> index 1bd079ad6a..f01c691dea 100644
> --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> @@ -1746,6 +1746,12 @@ PeCoffLoaderRelocateImageForRuntime (
>                                                                              RelocDir->VirtualAddress + RelocDir-
> >Size - 1,
>                                                                              0
>                                                                              );
> +    if (RelocBase == NULL || RelocBaseEnd == NULL || RelocBaseEnd <
> RelocBase) {
> +      //
> +      // relocation block is not valid, just return
> +      //
> +      return;
> +    }
>    } else {
>      //
>      // Cannot find relocations, cannot continue to relocate the image, ASSERT
> for this invalid image.
> --
> 2.16.2.windows.1