Hi Kun, It is clear and i have no confusion.
Reviewed-by: Guomin Jiang <guomin.ji...@intel.com> Thanks guomin From: Kun Qin [mailto:kun....@microsoft.com] Sent: Monday, March 23, 2020 3:40 PM To: Jiang, Guomin <guomin.ji...@intel.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kin...@intel.com>; Xu, Wei6 <wei6...@intel.com> Cc: Gao, Liming <liming....@intel.com> Subject: RE: [edk2-devel] [PATCH] FmpDevicePkg/FmpDxe: Fix uninitialized pointer dereference Hi Guomin, Thanks for reaching out. I did encounter a GP fault because of this issue: If Line 582<https://github.com/tianocore/edk2/blob/4c0f6e349d32cf27a7104ddd3e729d6ebc88ea70/FmpDevicePkg/FmpDxe/Dependency.c#L582> is triggered when the first Fmp->GetImageInfo failed, this specific mFmpImageInfoBuf[Index] will remain to be uninitialized value (0xFAFAFAFAFAF in my case). Later on when it comes to line 632<https://github.com/tianocore/edk2/blob/4c0f6e349d32cf27a7104ddd3e729d6ebc88ea70/FmpDevicePkg/FmpDxe/Dependency.c#L632>, it will pass the null pointer check and try to dereference it, which leads to GP fault. Please let me know if you need further clarification. Thanks, Kun From: Jiang, Guomin<mailto:guomin.ji...@intel.com> Sent: Monday, March 23, 2020 12:21 AM To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D<mailto:michael.d.kin...@intel.com>; Xu, Wei6<mailto:wei6...@intel.com> Cc: Kun Qin<mailto:kun....@microsoft.com>; Gao, Liming<mailto:liming....@intel.com> Subject: [EXTERNAL] RE: [edk2-devel] [PATCH] FmpDevicePkg/FmpDxe: Fix uninitialized pointer dereference 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> > [mailto:devel@edk2.groups.io] On Behalf Of > Michael D Kinney > Sent: Wednesday, March 18, 2020 11:15 PM > To: Xu, Wei6 <wei6...@intel.com<mailto:wei6...@intel.com>>; > devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael > D <michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>> > Cc: Kun Qin <ku...@microsoft.com<mailto:ku...@microsoft.com>>; Gao, Liming > <liming....@intel.com<mailto:liming....@intel.com>> > Subject: Re: [edk2-devel] [PATCH] FmpDevicePkg/FmpDxe: Fix uninitialized > pointer dereference > > Reviewed-by: Michael D Kinney > <michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>> > > > -----Original Message----- > > From: Xu, Wei6 <wei6...@intel.com<mailto:wei6...@intel.com>> > > Sent: Tuesday, March 17, 2020 11:12 PM > > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> > > Cc: Kun Qin <ku...@microsoft.com<mailto:ku...@microsoft.com>>; Kinney, > > Michael D > > <michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>; Gao, > > Liming <liming....@intel.com<mailto:liming....@intel.com>> > > Subject: [edk2-devel] [PATCH] FmpDevicePkg/FmpDxe: Fix uninitialized > > pointer dereference > > > > From: Kun Qin <ku...@microsoft.com<mailto:ku...@microsoft.com>> > > > > REF: > > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2602&data=02%7C01%7CKun.Qin%40microsoft.com%7C3c1042cd095b42a51b9d08d7cefad022%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637205448946602054&sdata=95z6fDC0uceCCs2MuoeCR4MXgRhAI3dVssWeddsWT5s%3D&reserved=0 > > > > 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.kin...@intel.com<mailto:michael.d.kin...@intel.com>> > > Cc: Liming Gao <liming....@intel.com<mailto:liming....@intel.com>> > > Signed-off-by: Wei6 Xu <wei6...@intel.com<mailto:wei6...@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 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#56099): https://edk2.groups.io/g/devel/message/56099 Mute This Topic: https://groups.io/mt/72043533/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
