NULL Pointer Check is needed for InternalImageHandleToFvHandle of DxeServicesLib

NULL Pointer Check is needed for InternalImageHandleToFvHandle of DxeServicesLib

[Cai, Weihao]

[-- Attachment #1.1: Type: text/plain, Size: 915 bytes --]

Hi EDK2 Dev,

We met a system hang issue recently and after investigation, it was caused by invalid data returned by "InternalImageHandleToFvHandle" of DxeServicesLib.

We call GetSectionFromAnyFv-> InternalImageHandleToFvHandle to get handle.
But InternalImageHandleToFvHandle cannot locate LoadedImageProtocol (This is expected). The returned status is "Invalid Parameter" and "LoadedImage" pointer is NULL.
The problem is InternalImageHandleToFvHandle does not check returned status or "LoadedImage" pointer, just return "LoadedImage->DeviceHandle".
Since "LoadedImage" pointer is NULL, "LoadedImage->DeviceHandle" is invalid handle (random data, but not NULL) which cause system hang.

I think InternalImageHandleToFvHandle should check returned status or "LoadedImage" pointer. If it is failed, just return NULL, not any random data.
[cid:image001.jpg@01D6B9DF.9634B310]

Best Regards
Rick Cai


[-- Attachment #1.2: Type: text/html, Size: 3657 bytes --]

[-- Attachment #2: image001.jpg --]
[-- Type: image/jpeg, Size: 38891 bytes --]

Re: [edk2-devel] NULL Pointer Check is needed for InternalImageHandleToFvHandle of DxeServicesLib

[Laszlo Ersek]

On 11/13/20 10:08, Cai, Weihao wrote:
> Hi EDK2 Dev,
> 
> We met a system hang issue recently and after investigation, it was caused by invalid data returned by "InternalImageHandleToFvHandle" of DxeServicesLib.
> 
> We call GetSectionFromAnyFv-> InternalImageHandleToFvHandle to get handle.
> But InternalImageHandleToFvHandle cannot locate LoadedImageProtocol (This is expected). The returned status is "Invalid Parameter" and "LoadedImage" pointer is NULL.
> The problem is InternalImageHandleToFvHandle does not check returned status or "LoadedImage" pointer, just return "LoadedImage->DeviceHandle".
> Since "LoadedImage" pointer is NULL, "LoadedImage->DeviceHandle" is invalid handle (random data, but not NULL) which cause system hang.
> 
> I think InternalImageHandleToFvHandle should check returned status or "LoadedImage" pointer. If it is failed, just return NULL, not any random data.
> [cid:image001.jpg@01D6B9DF.9634B310]
> 
> Best Regards
> Rick Cai

Please file a new ticket at <https://bugzilla.tianocore.org/> with this
information, and please consider submitting a patch to the mailing list
for review.

Please talk to your colleagues about the expected git settings in your
edk2 clone, and the patch submission process.

Thanks
Laszlo