Re: [edk2-devel] [PATCH] FmpDevicePkg/FmpDxe: Fix uninitialized pointer dereference

["Guomin Jiang" <guomin.jiang@intel.com>]

Hi Xuwei, QinKun,

Have you indeed encounter this issue or just think it is potential issue.

I think  below code will always initialize the mFmpImageInfoBuf[] and make sure it is valid.
Line 585 - mFmpImageInfoBuf[Index] = AllocateZeroPool (ImageInfoSize);

If the second GetImageInfo() is runned, I think it will always have correct mfmpImageInfoBuf[] address.

Of course, it is ok to use AllocateZeroPool to ensure zero buffer is allocated.

Thanks

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Michael D Kinney
> Sent: Wednesday, March 18, 2020 11:15 PM
> To: Xu, Wei6 <wei6.xu@intel.com>; devel@edk2.groups.io; Kinney, Michael
> D <michael.d.kinney@intel.com>
> Cc: Kun Qin <kuqin@microsoft.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2-devel] [PATCH] FmpDevicePkg/FmpDxe: Fix uninitialized
> pointer dereference
> 
> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> 
> > -----Original Message-----
> > From: Xu, Wei6 <wei6.xu@intel.com>
> > Sent: Tuesday, March 17, 2020 11:12 PM
> > To: devel@edk2.groups.io
> > Cc: Kun Qin <kuqin@microsoft.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
> > Subject: [edk2-devel] [PATCH] FmpDevicePkg/FmpDxe: Fix uninitialized
> > pointer dereference
> >
> > From: Kun Qin <kuqin@microsoft.com>
> >
> > REF:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2602
> >
> > Zero the allocated buffer in case GetImageInfo `continue` in the
> > middle of a loop. This will cause unexpected GetImageInfo failure not
> > clearing the corresponding entry and lead to GP faults when
> > dereferencing this entry.
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Signed-off-by: Wei6 Xu <wei6.xu@intel.com>
> > ---
> >  FmpDevicePkg/FmpDxe/Dependency.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/FmpDevicePkg/FmpDxe/Dependency.c
> > b/FmpDevicePkg/FmpDxe/Dependency.c
> > index 8f97c42916..65c23989c6 100644
> > --- a/FmpDevicePkg/FmpDxe/Dependency.c
> > +++ b/FmpDevicePkg/FmpDxe/Dependency.c
> > @@ -550,11 +550,11 @@ EvaluateImageDependencies (
> >                  );
> >    if (EFI_ERROR (Status)) {
> >      return EFI_ABORTED;
> >    }
> >
> > -  mFmpImageInfoBuf = AllocatePool
> > (sizeof(EFI_FIRMWARE_IMAGE_DESCRIPTOR *) *
> mNumberOfFmpInstance);
> > +  mFmpImageInfoBuf = AllocateZeroPool
> > (sizeof(EFI_FIRMWARE_IMAGE_DESCRIPTOR *) *
> mNumberOfFmpInstance);
> >    if (mFmpImageInfoBuf == NULL) {
> >      return EFI_OUT_OF_RESOURCES;
> >    }
> >
> >    for (Index = 0; Index < mNumberOfFmpInstance; Index
> > ++) {
> > --
> > 2.16.2.windows.1
> 
> 
>