From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mx.groups.io with SMTP id smtpd.web09.152.1578290391177563205 for ; Sun, 05 Jan 2020 21:59:51 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.31, mailfrom: jian.j.wang@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Jan 2020 21:59:50 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,401,1571727600"; d="scan'208";a="422026824" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga006.fm.intel.com with ESMTP; 05 Jan 2020 21:59:50 -0800 Received: from fmsmsx609.amr.corp.intel.com (10.18.126.89) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sun, 5 Jan 2020 21:59:50 -0800 Received: from fmsmsx609.amr.corp.intel.com (10.18.126.89) by fmsmsx609.amr.corp.intel.com (10.18.126.89) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Sun, 5 Jan 2020 21:59:50 -0800 Received: from shsmsx105.ccr.corp.intel.com (10.239.4.158) by fmsmsx609.amr.corp.intel.com (10.18.126.89) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Sun, 5 Jan 2020 21:59:49 -0800 Received: from shsmsx107.ccr.corp.intel.com ([169.254.9.210]) by SHSMSX105.ccr.corp.intel.com ([169.254.11.28]) with mapi id 14.03.0439.000; Mon, 6 Jan 2020 13:59:48 +0800 From: "Wang, Jian J" To: "devel@edk2.groups.io" , "Yao, Jiewen" CC: "Zhang, Chao B" Subject: Re: [edk2-devel] [PATCH 2/6] SecurityPkg/Tcg2Dxe: Add Tcg2Dxe to support 800-155 event. Thread-Topic: [edk2-devel] [PATCH 2/6] SecurityPkg/Tcg2Dxe: Add Tcg2Dxe to support 800-155 event. Thread-Index: AQHVv6XElWdD+YRxAE6ONl1CVPOw/6fdKE4Q Date: Mon, 6 Jan 2020 05:59:47 +0000 Message-ID: References: <20191231064412.22988-1-jiewen.yao@intel.com> <20191231064412.22988-3-jiewen.yao@intel.com> In-Reply-To: <20191231064412.22988-3-jiewen.yao@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMDg4MmJjNGUtYTkzOS00Y2RiLTk1Y2ItM2E3Njc1OTE3ZGM0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiWlNaaXdrYjFFXC8zVExJRlAzY0hjazBWUiszVEExVlpQd2xXY3IzZ2tKaDZlUzZPT3V4bzZ6UDVVb1wvMXlaXC9BayJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: jian.j.wang@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Jiewen, Just two coding style comments below. With it addressed, Reviewed-by: Jian J Wang > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Yao, Jiew= en > Sent: Tuesday, December 31, 2019 2:44 PM > To: devel@edk2.groups.io > Cc: Wang, Jian J ; Zhang, Chao B > > Subject: [edk2-devel] [PATCH 2/6] SecurityPkg/Tcg2Dxe: Add Tcg2Dxe to > support 800-155 event. >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2439 >=20 > The TCG2 DXE supports to parse the 800-155 event GUID from PEI > and puts to the beginning of the TCG2 event. >=20 > The TCG2 DXE also supports a DXE driver produces 800-155 event > and let TCG2 DXE driver record. >=20 > The 800-155 is a NO-ACTION event which does not need extend > anything to TPM2. The TCG2 DXE also supports that. >=20 > Multiple 800-155 events are supported. All of them will be put > to the beginning of the TCG2 event, just after the SpecId event. >=20 > Cc: Jian J Wang > Cc: Chao Zhang > Signed-off-by: Jiewen Yao > --- > SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 157 +++++++++++++++++++++++----- > SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf | 1 + > 2 files changed, 129 insertions(+), 29 deletions(-) >=20 > diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c > b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c > index 3cd16c2fa3..b185b56703 100644 > --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c > +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c > @@ -75,6 +75,7 @@ typedef struct { > UINT8 *LastEvent; > BOOLEAN EventLogStarted; > BOOLEAN EventLogTruncated; > + UINTN Next800155EventOffset; > } TCG_EVENT_LOG_AREA_STRUCT; >=20 > typedef struct _TCG_DXE_DATA { > @@ -771,16 +772,42 @@ Tcg2GetEventLog ( > return EFI_SUCCESS; > } >=20 > +/* > + Return if this is a Tcg800155PlatformIdEvent. > + > + @param[in] NewEventHdr Pointer to a > TCG_PCR_EVENT_HDR/TCG_PCR_EVENT_EX data structure. > + @param[in] NewEventHdrSize New event header size. > + @param[in] NewEventData Pointer to the new event data. > + @param[in] NewEventSize New event data size. > + > + @retval TRUE This is a Tcg800155PlatformIdEvent. > + @retval FALSE This is NOT a Tcg800155PlatformIdEvent. > + > +*/ > +BOOLEAN > +Is800155Event ( > + IN VOID *NewEventHdr, > + IN UINT32 NewEventHdrSize, > + IN UINT8 *NewEventData, > + IN UINT32 NewEventSize > + ) > +{ > + if ((((TCG_PCR_EVENT2_HDR *)NewEventHdr)->EventType =3D=3D EV_NO_ACTI= ON) > && > + (NewEventSize >=3D sizeof(TCG_Sp800_155_PlatformId_Event2)) && > + (CompareMem (NewEventData, > TCG_Sp800_155_PlatformId_Event2_SIGNATURE, > sizeof(TCG_Sp800_155_PlatformId_Event2_SIGNATURE) - 1) =3D=3D 0)) { The above line is too long to read in one screen. You could wrap the last parameter of CompareMem in new line or add a new local variable to keep the size. > + return TRUE; > + } > + return FALSE; > +} > + > /** > Add a new entry to the Event Log. >=20 > - @param[in, out] EventLogPtr Pointer to the Event Log data. > - @param[in, out] LogSize Size of the Event Log. > - @param[in] MaxSize Maximum size of the Event Log. > - @param[in] NewEventHdr Pointer to a > TCG_PCR_EVENT_HDR/TCG_PCR_EVENT_EX data structure. > - @param[in] NewEventHdrSize New event header size. > - @param[in] NewEventData Pointer to the new event data. > - @param[in] NewEventSize New event data size. > + @param[in, out] EventLogAreaStruct The event log area data structure > + @param[in] NewEventHdr Pointer to a > TCG_PCR_EVENT_HDR/TCG_PCR_EVENT_EX data structure. > + @param[in] NewEventHdrSize New event header size. > + @param[in] NewEventData Pointer to the new event data. > + @param[in] NewEventSize New event data size. >=20 > @retval EFI_SUCCESS The new event log entry was added. > @retval EFI_OUT_OF_RESOURCES No enough memory to log the new event. > @@ -788,9 +815,7 @@ Tcg2GetEventLog ( > **/ > EFI_STATUS > TcgCommLogEvent ( > - IN OUT UINT8 **EventLogPtr, > - IN OUT UINTN *LogSize, > - IN UINTN MaxSize, > + IN OUT TCG_EVENT_LOG_AREA_STRUCT *EventLogAreaStruct, > IN VOID *NewEventHdr, > IN UINT32 NewEventHdrSize, > IN UINT8 *NewEventData, > @@ -798,6 +823,7 @@ TcgCommLogEvent ( > ) > { > UINTN NewLogSize; > + BOOLEAN Record800155Event; >=20 > if (NewEventSize > MAX_ADDRESS - NewEventHdrSize) { > return EFI_OUT_OF_RESOURCES; > @@ -805,23 +831,55 @@ TcgCommLogEvent ( >=20 > NewLogSize =3D NewEventHdrSize + NewEventSize; >=20 > - if (NewLogSize > MAX_ADDRESS - *LogSize) { > + if (NewLogSize > MAX_ADDRESS - EventLogAreaStruct->EventLogSize) { > return EFI_OUT_OF_RESOURCES; > } >=20 > - if (NewLogSize + *LogSize > MaxSize) { > - DEBUG ((EFI_D_INFO, " MaxSize - 0x%x\n", MaxSize)); > - DEBUG ((EFI_D_INFO, " NewLogSize - 0x%x\n", NewLogSize)); > - DEBUG ((EFI_D_INFO, " LogSize - 0x%x\n", *LogSize)); > - DEBUG ((EFI_D_INFO, "TcgCommLogEvent - %r\n", > EFI_OUT_OF_RESOURCES)); > + if (NewLogSize + EventLogAreaStruct->EventLogSize > EventLogAreaStruc= t- > >Laml) { > + DEBUG ((DEBUG_INFO, " Laml - 0x%x\n", EventLogAreaStruct->La= ml)); > + DEBUG ((DEBUG_INFO, " NewLogSize - 0x%x\n", NewLogSize)); > + DEBUG ((DEBUG_INFO, " LogSize - 0x%x\n", EventLogAreaStruct- > >EventLogSize)); > + DEBUG ((DEBUG_INFO, "TcgCommLogEvent - %r\n", > EFI_OUT_OF_RESOURCES)); > return EFI_OUT_OF_RESOURCES; > } >=20 > - *EventLogPtr +=3D *LogSize; > - *LogSize +=3D NewLogSize; > - CopyMem (*EventLogPtr, NewEventHdr, NewEventHdrSize); > + // > + // Check 800-155 event > + // Record to 800-155 event offset only. > + // If the offset is 0, no need to record. > + // > + Record800155Event =3D Is800155Event (NewEventHdr, NewEventHdrSize, > NewEventData, NewEventSize); > + if (Record800155Event) { > + if (EventLogAreaStruct->Next800155EventOffset !=3D 0) { > + CopyMem ( > + (UINT8 *)(UINTN)EventLogAreaStruct->Lasa + EventLogAreaStruct- > >Next800155EventOffset + NewLogSize, > + (UINT8 *)(UINTN)EventLogAreaStruct->Lasa + EventLogAreaStruct- > >Next800155EventOffset, > + EventLogAreaStruct->EventLogSize - EventLogAreaStruct- > >Next800155EventOffset > + ); > + > + CopyMem ( > + (UINT8 *)(UINTN)EventLogAreaStruct->Lasa + EventLogAreaStruct- > >Next800155EventOffset, > + NewEventHdr, > + NewEventHdrSize > + ); > + CopyMem ( > + (UINT8 *)(UINTN)EventLogAreaStruct->Lasa + EventLogAreaStruct- > >Next800155EventOffset + NewEventHdrSize, > + NewEventData, > + NewEventSize > + ); > + > + EventLogAreaStruct->Next800155EventOffset +=3D NewLogSize; > + EventLogAreaStruct->LastEvent +=3D NewLogSize; > + EventLogAreaStruct->EventLogSize +=3D NewLogSize; > + } > + return EFI_SUCCESS; > + } > + > + EventLogAreaStruct->LastEvent =3D (UINT8 *)(UINTN)EventLogAreaStruct-= >Lasa > + EventLogAreaStruct->EventLogSize; > + EventLogAreaStruct->EventLogSize +=3D NewLogSize; > + CopyMem (EventLogAreaStruct->LastEvent, NewEventHdr, NewEventHdrSize)= ; > CopyMem ( > - *EventLogPtr + NewEventHdrSize, > + EventLogAreaStruct->LastEvent + NewEventHdrSize, > NewEventData, > NewEventSize > ); > @@ -873,11 +931,8 @@ TcgDxeLogEvent ( > return EFI_VOLUME_FULL; > } >=20 > - EventLogAreaStruct->LastEvent =3D (UINT8*)(UINTN)EventLogAreaStruct->= Lasa; > Status =3D TcgCommLogEvent ( > - &EventLogAreaStruct->LastEvent, > - &EventLogAreaStruct->EventLogSize, > - (UINTN)EventLogAreaStruct->Laml, > + EventLogAreaStruct, > NewEventHdr, > NewEventHdrSize, > NewEventData, > @@ -907,11 +962,8 @@ TcgDxeLogEvent ( > return EFI_VOLUME_FULL; > } >=20 > - EventLogAreaStruct->LastEvent =3D (UINT8*)(UINTN)EventLogAreaStruct= - > >Lasa; > Status =3D TcgCommLogEvent ( > - &EventLogAreaStruct->LastEvent, > - &EventLogAreaStruct->EventLogSize, > - (UINTN)EventLogAreaStruct->Laml, > + EventLogAreaStruct, > NewEventHdr, > NewEventHdrSize, > NewEventData, > @@ -1138,11 +1190,25 @@ TcgDxeHashLogExtendEvent ( > { > EFI_STATUS Status; > TPML_DIGEST_VALUES DigestList; > + TCG_PCR_EVENT2_HDR NoActionEvent; >=20 > if (!mTcgDxeData.BsCap.TPMPresentFlag) { > return EFI_DEVICE_ERROR; > } >=20 > + if (NewEventHdr->EventType =3D=3D EV_NO_ACTION) { > + // > + // Do not do TPM extend for EV_NO_ACTION > + // > + Status =3D EFI_SUCCESS; > + InitNoActionEvent (&NoActionEvent, NewEventHdr->EventSize); > + if ((Flags & EFI_TCG2_EXTEND_ONLY) =3D=3D 0) { > + Status =3D TcgDxeLogHashEvent (&(NoActionEvent.Digests), NewEvent= Hdr, > NewEventData); > + } > + > + return Status; > + } > + > Status =3D HashAndExtend ( > NewEventHdr->PCRIndex, > HashData, > @@ -1202,7 +1268,13 @@ Tcg2HashLogExtendEvent ( >=20 > DEBUG ((DEBUG_VERBOSE, "Tcg2HashLogExtendEvent ...\n")); >=20 > - if ((This =3D=3D NULL) || (DataToHash =3D=3D 0) || (Event =3D=3D NULL= )) { > + if ((This =3D=3D NULL) || (Event =3D=3D NULL)) { > + return EFI_INVALID_PARAMETER; > + } > + // > + // Do not check hash data size for EV_NO_ACTION event. > + // > + if ((Event->Header.EventType !=3D EV_NO_ACTION) && (DataToHash =3D=3D= 0)) { > return EFI_INVALID_PARAMETER; > } >=20 > @@ -1487,6 +1559,7 @@ SetupEventLog ( > } > mTcgDxeData.EventLogAreaStruct[Index].Lasa =3D Lasa; > mTcgDxeData.EventLogAreaStruct[Index].Laml =3D PcdGet32 > (PcdTcgLogAreaMinLen); > + mTcgDxeData.EventLogAreaStruct[Index].Next800155EventOffset =3D 0= ; >=20 > if ((PcdGet8(PcdTpm2AcpiTableRev) >=3D 4) || > (mTcg2EventInfo[Index].LogFormat =3D=3D > EFI_TCG2_EVENT_LOG_FORMAT_TCG_2)) { > @@ -1577,6 +1650,30 @@ SetupEventLog ( > (UINT8 *)TcgEfiSpecIdEventStruct, > SpecIdEvent.EventSize > ); > + // > + // record the offset at the end of 800-155 event. > + // the future 800-155 event can be inserted here. > + // > + mTcgDxeData.EventLogAreaStruct[Index].Next800155EventOffset =3D > mTcgDxeData.EventLogAreaStruct[Index].EventLogSize; > + The above line is too long to read in one screen. Suggest to wrap after '= =3D'. Regards, Jian > + // > + // Tcg800155PlatformIdEvent. Event format is TCG_PCR_EVENT2 > + // > + GuidHob.Guid =3D GetFirstGuidHob (&gTcg800155PlatformIdEventHob= Guid); > + while (GuidHob.Guid !=3D NULL) { > + InitNoActionEvent(&NoActionEvent, GET_GUID_HOB_DATA_SIZE > (GuidHob.Guid)); > + > + Status =3D TcgDxeLogEvent ( > + mTcg2EventInfo[Index].LogFormat, > + &NoActionEvent, > + sizeof(NoActionEvent.PCRIndex) + sizeof(NoActionEv= ent.EventType) > + GetDigestListBinSize (&NoActionEvent.Digests) + > sizeof(NoActionEvent.EventSize), The above line is too long to read in one screen. Suggest to wrap around s= econd '+'. > + GET_GUID_HOB_DATA (GuidHob.Guid), > + GET_GUID_HOB_DATA_SIZE (GuidHob.Guid) > + ); > + > + GuidHob.Guid =3D GET_NEXT_HOB (GuidHob); > + GuidHob.Guid =3D GetNextGuidHob (&gTcg800155PlatformIdEventHo= bGuid, > GuidHob.Guid); > + } >=20 > // > // EfiStartupLocalityEvent. Event format is TCG_PCR_EVENT2 > @@ -1643,6 +1740,7 @@ SetupEventLog ( > mTcgDxeData.FinalEventLogAreaStruct[Index].LastEvent =3D (VOID > *)(UINTN)mTcgDxeData.FinalEventLogAreaStruct[Index].Lasa; > mTcgDxeData.FinalEventLogAreaStruct[Index].EventLogStarted =3D = FALSE; > mTcgDxeData.FinalEventLogAreaStruct[Index].EventLogTruncated = =3D FALSE; > + mTcgDxeData.FinalEventLogAreaStruct[Index].Next800155EventOffse= t =3D 0; >=20 > // > // Install to configuration table for > EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 > @@ -1663,6 +1761,7 @@ SetupEventLog ( > mTcgDxeData.FinalEventLogAreaStruct[Index].LastEvent =3D 0; > mTcgDxeData.FinalEventLogAreaStruct[Index].EventLogStarted =3D = FALSE; > mTcgDxeData.FinalEventLogAreaStruct[Index].EventLogTruncated = =3D FALSE; > + mTcgDxeData.FinalEventLogAreaStruct[Index].Next800155EventOffse= t =3D 0; > } > } > } > diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf > b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf > index 0127a31e97..576cf80d06 100644 > --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf > +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf > @@ -85,6 +85,7 @@ >=20 > gTcgEvent2EntryHobGuid ## SOMETIMES_CONSU= MES ## HOB > gTpm2StartupLocalityHobGuid ## SOMETIMES_CONSU= MES ## > HOB > + gTcg800155PlatformIdEventHobGuid ## SOMETIMES_CONSU= MES > ## HOB >=20 > [Protocols] > gEfiTcg2ProtocolGuid ## PRODUCES > -- > 2.19.2.windows.1 >=20 >=20 >=20