* [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