On 08/31/20 10:15, Zhiguang Liu wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2945 > > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Jian J Wang <jian.j.w...@intel.com> > Cc: Qi Zhang <qi1.zh...@intel.com> > Cc: Rahul Kumar <rahul1.ku...@intel.com> > Signed-off-by: Zhiguang Liu <zhiguang....@intel.com> > --- > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > index 0e770f4485..5e883f0cc5 100644 > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > @@ -449,6 +449,7 @@ HashLogExtendEvent ( > EFI_STATUS Status; > TPML_DIGEST_VALUES DigestList; > > + Status = EFI_SUCCESS; > if (GetFirstGuidHob (&gTpmErrorHobGuid) != NULL) { > return EFI_DEVICE_ERROR; > } >
I agree that there is a path in the code where Status is read without having been set. It seems that using EFI_SUCCESS as default value makes sense (assuming the LogHashEvent() call is the important one). I'll let SecurityPkg maintainers decide about this. I think it would be nicer to set Status to EFI_SUCCESS either on the (currently missing) "else" branch, or else just before the EDKII_TCG_PRE_HASH check. In particular, setting Status so early that we may still exit with EFI_DEVICE_ERROR is wasteful. So at least I'd move the assignment past the "return EFI_DEVICE_ERROR" statement. But... it's late. I agree that this patch qualifies for the stable tag. So I'm not asking for a repost. I just wish more thought had been given to the placement of the assignment. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#64840): https://edk2.groups.io/g/devel/message/64840 Mute This Topic: https://groups.io/mt/76530112/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
