* [PATCH] SecurityPkg Tcg2Dxe: Get correct digest list size @ 2016-11-17 9:26 Star Zeng 2016-11-17 12:32 ` Yao, Jiewen 0 siblings, 1 reply; 4+ messages in thread From: Star Zeng @ 2016-11-17 9:26 UTC (permalink / raw) To: edk2-devel; +Cc: Star Zeng, Jiewen Yao, Chao Zhang Current code uses GetDigestListSize(DigestList) to get digest list size, that is incorrect. The code should get digest list size of digests copied into event2 log, those digests are compacted, so GetDigestListBinSize() should be used. Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Chao Zhang <chao.b.zhang@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Star Zeng <star.zeng@intel.com> --- SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c index d5a32307db6e..f4740a34444c 100644 --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c @@ -970,6 +970,7 @@ TcgDxeLogHashEvent ( EFI_STATUS RetStatus; TCG_PCR_EVENT2 TcgPcrEvent2; UINT8 *DigestBuffer; + UINT32 *EventSizePtr; DEBUG ((EFI_D_INFO, "SupportedEventLogs - 0x%08x\n", mTcgDxeData.BsCap.SupportedEventLogs)); @@ -1006,9 +1007,8 @@ TcgDxeLogHashEvent ( TcgPcrEvent2.PCRIndex = NewEventHdr->PCRIndex; TcgPcrEvent2.EventType = NewEventHdr->EventType; DigestBuffer = (UINT8 *)&TcgPcrEvent2.Digest; - DigestBuffer = CopyDigestListToBuffer (DigestBuffer, DigestList, mTcgDxeData.BsCap.ActivePcrBanks); - CopyMem (DigestBuffer, &NewEventHdr->EventSize, sizeof(NewEventHdr->EventSize)); - DigestBuffer = DigestBuffer + sizeof(NewEventHdr->EventSize); + EventSizePtr = CopyDigestListToBuffer (DigestBuffer, DigestList, mTcgDxeData.BsCap.ActivePcrBanks); + CopyMem (EventSizePtr, &NewEventHdr->EventSize, sizeof(NewEventHdr->EventSize)); // // Enter critical region @@ -1017,7 +1017,7 @@ TcgDxeLogHashEvent ( Status = TcgDxeLogEvent ( mTcg2EventInfo[Index].LogFormat, &TcgPcrEvent2, - sizeof(TcgPcrEvent2.PCRIndex) + sizeof(TcgPcrEvent2.EventType) + GetDigestListSize (DigestList) + sizeof(TcgPcrEvent2.EventSize), + sizeof(TcgPcrEvent2.PCRIndex) + sizeof(TcgPcrEvent2.EventType) + GetDigestListBinSize (DigestBuffer) + sizeof(TcgPcrEvent2.EventSize), NewEventData, NewEventHdr->EventSize ); -- 2.7.0.windows.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] SecurityPkg Tcg2Dxe: Get correct digest list size 2016-11-17 9:26 [PATCH] SecurityPkg Tcg2Dxe: Get correct digest list size Star Zeng @ 2016-11-17 12:32 ` Yao, Jiewen 2016-11-18 0:51 ` Zeng, Star 0 siblings, 1 reply; 4+ messages in thread From: Yao, Jiewen @ 2016-11-17 12:32 UTC (permalink / raw) To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Zhang, Chao B Hi star I am sorry that I am confused on this patch. 1) Below update is not related to the GIT message. I think there is nothing wrong with previous code. May I know if you observe something? > - DigestBuffer = CopyDigestListToBuffer (DigestBuffer, DigestList, > mTcgDxeData.BsCap.ActivePcrBanks); > - CopyMem (DigestBuffer, &NewEventHdr->EventSize, > sizeof(NewEventHdr->EventSize)); > - DigestBuffer = DigestBuffer + sizeof(NewEventHdr->EventSize); > + EventSizePtr = CopyDigestListToBuffer (DigestBuffer, DigestList, > mTcgDxeData.BsCap.ActivePcrBanks); > + CopyMem (EventSizePtr, &NewEventHdr->EventSize, > sizeof(NewEventHdr->EventSize)); 2) I believe "GetDigestListSize (DigestList)" should be same as "GetDigestListBinSize (DigestBuffer)" May I know how did you observe such error? Thank you Yao Jiewen > -----Original Message----- > From: Zeng, Star > Sent: Thursday, November 17, 2016 5:27 PM > To: edk2-devel@lists.01.org > Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; > Zhang, Chao B <chao.b.zhang@intel.com> > Subject: [PATCH] SecurityPkg Tcg2Dxe: Get correct digest list size > > Current code uses GetDigestListSize(DigestList) to get > digest list size, that is incorrect. > The code should get digest list size of digests copied > into event2 log, those digests are compacted, so > GetDigestListBinSize() should be used. > > Cc: Jiewen Yao <jiewen.yao@intel.com> > Cc: Chao Zhang <chao.b.zhang@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Star Zeng <star.zeng@intel.com> > --- > SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c > b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c > index d5a32307db6e..f4740a34444c 100644 > --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c > +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c > @@ -970,6 +970,7 @@ TcgDxeLogHashEvent ( > EFI_STATUS RetStatus; > TCG_PCR_EVENT2 TcgPcrEvent2; > UINT8 *DigestBuffer; > + UINT32 *EventSizePtr; > > DEBUG ((EFI_D_INFO, "SupportedEventLogs - 0x%08x\n", > mTcgDxeData.BsCap.SupportedEventLogs)); > > @@ -1006,9 +1007,8 @@ TcgDxeLogHashEvent ( > TcgPcrEvent2.PCRIndex = NewEventHdr->PCRIndex; > TcgPcrEvent2.EventType = NewEventHdr->EventType; > DigestBuffer = (UINT8 *)&TcgPcrEvent2.Digest; > - DigestBuffer = CopyDigestListToBuffer (DigestBuffer, DigestList, > mTcgDxeData.BsCap.ActivePcrBanks); > - CopyMem (DigestBuffer, &NewEventHdr->EventSize, > sizeof(NewEventHdr->EventSize)); > - DigestBuffer = DigestBuffer + sizeof(NewEventHdr->EventSize); > + EventSizePtr = CopyDigestListToBuffer (DigestBuffer, DigestList, > mTcgDxeData.BsCap.ActivePcrBanks); > + CopyMem (EventSizePtr, &NewEventHdr->EventSize, > sizeof(NewEventHdr->EventSize)); > > // > // Enter critical region > @@ -1017,7 +1017,7 @@ TcgDxeLogHashEvent ( > Status = TcgDxeLogEvent ( > mTcg2EventInfo[Index].LogFormat, > &TcgPcrEvent2, > - sizeof(TcgPcrEvent2.PCRIndex) + > sizeof(TcgPcrEvent2.EventType) + GetDigestListSize (DigestList) + > sizeof(TcgPcrEvent2.EventSize), > + sizeof(TcgPcrEvent2.PCRIndex) + > sizeof(TcgPcrEvent2.EventType) + GetDigestListBinSize (DigestBuffer) + > sizeof(TcgPcrEvent2.EventSize), > NewEventData, > NewEventHdr->EventSize > ); > -- > 2.7.0.windows.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] SecurityPkg Tcg2Dxe: Get correct digest list size 2016-11-17 12:32 ` Yao, Jiewen @ 2016-11-18 0:51 ` Zeng, Star 2016-11-18 2:20 ` Yao, Jiewen 0 siblings, 1 reply; 4+ messages in thread From: Zeng, Star @ 2016-11-18 0:51 UTC (permalink / raw) To: Yao, Jiewen, edk2-devel@lists.01.org; +Cc: Zhang, Chao B On 2016/11/17 20:32, Yao, Jiewen wrote: > Hi star > I am sorry that I am confused on this patch. > > 1) Below update is not related to the GIT message. > I think there is nothing wrong with previous code. May I know if you observe something? The following GetDigestListBinSize (DigestBuffer) needs *DigestBuffer points to real digest buffer*. > >> - DigestBuffer = CopyDigestListToBuffer (DigestBuffer, DigestList, >> mTcgDxeData.BsCap.ActivePcrBanks); >> - CopyMem (DigestBuffer, &NewEventHdr->EventSize, >> sizeof(NewEventHdr->EventSize)); >> - DigestBuffer = DigestBuffer + sizeof(NewEventHdr->EventSize); >> + EventSizePtr = CopyDigestListToBuffer (DigestBuffer, DigestList, >> mTcgDxeData.BsCap.ActivePcrBanks); >> + CopyMem (EventSizePtr, &NewEventHdr->EventSize, >> sizeof(NewEventHdr->EventSize)); > > 2) I believe "GetDigestListSize (DigestList)" should be same as "GetDigestListBinSize (DigestBuffer)" > May I know how did you observe such error? DigestList points to the original full un-compacted digest list, DigestBuffer points to the copied partial compacted digest list. Thanks, Star > > > Thank you > Yao Jiewen > > > >> -----Original Message----- >> From: Zeng, Star >> Sent: Thursday, November 17, 2016 5:27 PM >> To: edk2-devel@lists.01.org >> Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; >> Zhang, Chao B <chao.b.zhang@intel.com> >> Subject: [PATCH] SecurityPkg Tcg2Dxe: Get correct digest list size >> >> Current code uses GetDigestListSize(DigestList) to get >> digest list size, that is incorrect. >> The code should get digest list size of digests copied >> into event2 log, those digests are compacted, so >> GetDigestListBinSize() should be used. >> >> Cc: Jiewen Yao <jiewen.yao@intel.com> >> Cc: Chao Zhang <chao.b.zhang@intel.com> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Star Zeng <star.zeng@intel.com> >> --- >> SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c >> b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c >> index d5a32307db6e..f4740a34444c 100644 >> --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c >> +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c >> @@ -970,6 +970,7 @@ TcgDxeLogHashEvent ( >> EFI_STATUS RetStatus; >> TCG_PCR_EVENT2 TcgPcrEvent2; >> UINT8 *DigestBuffer; >> + UINT32 *EventSizePtr; >> >> DEBUG ((EFI_D_INFO, "SupportedEventLogs - 0x%08x\n", >> mTcgDxeData.BsCap.SupportedEventLogs)); >> >> @@ -1006,9 +1007,8 @@ TcgDxeLogHashEvent ( >> TcgPcrEvent2.PCRIndex = NewEventHdr->PCRIndex; >> TcgPcrEvent2.EventType = NewEventHdr->EventType; >> DigestBuffer = (UINT8 *)&TcgPcrEvent2.Digest; >> - DigestBuffer = CopyDigestListToBuffer (DigestBuffer, DigestList, >> mTcgDxeData.BsCap.ActivePcrBanks); >> - CopyMem (DigestBuffer, &NewEventHdr->EventSize, >> sizeof(NewEventHdr->EventSize)); >> - DigestBuffer = DigestBuffer + sizeof(NewEventHdr->EventSize); >> + EventSizePtr = CopyDigestListToBuffer (DigestBuffer, DigestList, >> mTcgDxeData.BsCap.ActivePcrBanks); >> + CopyMem (EventSizePtr, &NewEventHdr->EventSize, >> sizeof(NewEventHdr->EventSize)); >> >> // >> // Enter critical region >> @@ -1017,7 +1017,7 @@ TcgDxeLogHashEvent ( >> Status = TcgDxeLogEvent ( >> mTcg2EventInfo[Index].LogFormat, >> &TcgPcrEvent2, >> - sizeof(TcgPcrEvent2.PCRIndex) + >> sizeof(TcgPcrEvent2.EventType) + GetDigestListSize (DigestList) + >> sizeof(TcgPcrEvent2.EventSize), >> + sizeof(TcgPcrEvent2.PCRIndex) + >> sizeof(TcgPcrEvent2.EventType) + GetDigestListBinSize (DigestBuffer) + >> sizeof(TcgPcrEvent2.EventSize), >> NewEventData, >> NewEventHdr->EventSize >> ); >> -- >> 2.7.0.windows.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] SecurityPkg Tcg2Dxe: Get correct digest list size 2016-11-18 0:51 ` Zeng, Star @ 2016-11-18 2:20 ` Yao, Jiewen 0 siblings, 0 replies; 4+ messages in thread From: Yao, Jiewen @ 2016-11-18 2:20 UTC (permalink / raw) To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Zhang, Chao B Yes, that is correct. I misunderstand the patch. Reviewed-by: Jiewen.yao@intel.com<mailto:Jiewen.yao@intel.com> At same time, I would recommend we update PEI with comment on why we can use GetDigestListSize (DigestList) in PEI, but not use GetDigestListSize in DXE, because people may compare these 2 version. Thank you Yao Jiewen From: Zeng, Star Sent: Friday, November 18, 2016 8:51 AM To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org Cc: Zhang, Chao B <chao.b.zhang@intel.com> Subject: Re: [edk2] [PATCH] SecurityPkg Tcg2Dxe: Get correct digest list size On 2016/11/17 20:32, Yao, Jiewen wrote: > Hi star > I am sorry that I am confused on this patch. > > 1) Below update is not related to the GIT message. > I think there is nothing wrong with previous code. May I know if you observe something? The following GetDigestListBinSize (DigestBuffer) needs *DigestBuffer points to real digest buffer*. > >> - DigestBuffer = CopyDigestListToBuffer (DigestBuffer, DigestList, >> mTcgDxeData.BsCap.ActivePcrBanks); >> - CopyMem (DigestBuffer, &NewEventHdr->EventSize, >> sizeof(NewEventHdr->EventSize)); >> - DigestBuffer = DigestBuffer + sizeof(NewEventHdr->EventSize); >> + EventSizePtr = CopyDigestListToBuffer (DigestBuffer, DigestList, >> mTcgDxeData.BsCap.ActivePcrBanks); >> + CopyMem (EventSizePtr, &NewEventHdr->EventSize, >> sizeof(NewEventHdr->EventSize)); > > 2) I believe "GetDigestListSize (DigestList)" should be same as "GetDigestListBinSize (DigestBuffer)" > May I know how did you observe such error? DigestList points to the original full un-compacted digest list, DigestBuffer points to the copied partial compacted digest list. Thanks, Star > > > Thank you > Yao Jiewen > > > >> -----Original Message----- >> From: Zeng, Star >> Sent: Thursday, November 17, 2016 5:27 PM >> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> >> Cc: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; >> Zhang, Chao B <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>> >> Subject: [PATCH] SecurityPkg Tcg2Dxe: Get correct digest list size >> >> Current code uses GetDigestListSize(DigestList) to get >> digest list size, that is incorrect. >> The code should get digest list size of digests copied >> into event2 log, those digests are compacted, so >> GetDigestListBinSize() should be used. >> >> Cc: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> >> Cc: Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>> >> --- >> SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c >> b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c >> index d5a32307db6e..f4740a34444c 100644 >> --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c >> +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c >> @@ -970,6 +970,7 @@ TcgDxeLogHashEvent ( >> EFI_STATUS RetStatus; >> TCG_PCR_EVENT2 TcgPcrEvent2; >> UINT8 *DigestBuffer; >> + UINT32 *EventSizePtr; >> >> DEBUG ((EFI_D_INFO, "SupportedEventLogs - 0x%08x\n", >> mTcgDxeData.BsCap.SupportedEventLogs)); >> >> @@ -1006,9 +1007,8 @@ TcgDxeLogHashEvent ( >> TcgPcrEvent2.PCRIndex = NewEventHdr->PCRIndex; >> TcgPcrEvent2.EventType = NewEventHdr->EventType; >> DigestBuffer = (UINT8 *)&TcgPcrEvent2.Digest; >> - DigestBuffer = CopyDigestListToBuffer (DigestBuffer, DigestList, >> mTcgDxeData.BsCap.ActivePcrBanks); >> - CopyMem (DigestBuffer, &NewEventHdr->EventSize, >> sizeof(NewEventHdr->EventSize)); >> - DigestBuffer = DigestBuffer + sizeof(NewEventHdr->EventSize); >> + EventSizePtr = CopyDigestListToBuffer (DigestBuffer, DigestList, >> mTcgDxeData.BsCap.ActivePcrBanks); >> + CopyMem (EventSizePtr, &NewEventHdr->EventSize, >> sizeof(NewEventHdr->EventSize)); >> >> // >> // Enter critical region >> @@ -1017,7 +1017,7 @@ TcgDxeLogHashEvent ( >> Status = TcgDxeLogEvent ( >> mTcg2EventInfo[Index].LogFormat, >> &TcgPcrEvent2, >> - sizeof(TcgPcrEvent2.PCRIndex) + >> sizeof(TcgPcrEvent2.EventType) + GetDigestListSize (DigestList) + >> sizeof(TcgPcrEvent2.EventSize), >> + sizeof(TcgPcrEvent2.PCRIndex) + >> sizeof(TcgPcrEvent2.EventType) + GetDigestListBinSize (DigestBuffer) + >> sizeof(TcgPcrEvent2.EventSize), >> NewEventData, >> NewEventHdr->EventSize >> ); >> -- >> 2.7.0.windows.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-11-18 2:20 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-11-17 9:26 [PATCH] SecurityPkg Tcg2Dxe: Get correct digest list size Star Zeng 2016-11-17 12:32 ` Yao, Jiewen 2016-11-18 0:51 ` Zeng, Star 2016-11-18 2:20 ` Yao, Jiewen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox