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

[Laszlo Ersek <lersek@redhat.com>]

On 01/29/19 06:13, Bi, Dandan wrote:
> 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.

I agree, checking Size against 0 before calling
PeCoffLoaderImageAddress() is best; we do the same in
PeCoffLoaderRelocateImage():

  if ((RelocDir != NULL) && (RelocDir->Size > 0)) {

Thanks,
Laszlo

> 
> 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
>