From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id B869E1A1E0B for ; Mon, 26 Sep 2016 22:08:17 -0700 (PDT) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga104.fm.intel.com with ESMTP; 26 Sep 2016 22:08:17 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,403,1470726000"; d="scan'208";a="1036954843" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga001.jf.intel.com with ESMTP; 26 Sep 2016 22:08:16 -0700 Received: from fmsmsx116.amr.corp.intel.com (10.18.116.20) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 26 Sep 2016 22:08:16 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx116.amr.corp.intel.com (10.18.116.20) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 26 Sep 2016 22:08:16 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.234]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.96]) with mapi id 14.03.0248.002; Tue, 27 Sep 2016 13:08:12 +0800 From: "Long, Qin" To: "Zhang, Chao B" , "edk2-devel@lists.01.org" CC: "Yao, Jiewen" Thread-Topic: [PATCH] SecurityPkg: Tcg2: Fix TCG2 PP issues Thread-Index: AQHSGGHVuq67hYtaGkaFjdOtRwml8aCMyWFA Date: Tue, 27 Sep 2016 05:08:11 +0000 Message-ID: References: <1474941152-15460-1-git-send-email-chao.b.zhang@intel.com> In-Reply-To: <1474941152-15460-1-git-send-email-chao.b.zhang@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] SecurityPkg: Tcg2: Fix TCG2 PP issues X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Sep 2016 05:08:18 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Qin Long > -----Original Message----- > From: Zhang, Chao B > Sent: Tuesday, September 27, 2016 9:53 AM > To: edk2-devel@lists.01.org > Cc: Yao, Jiewen; Long, Qin; Zhang, Chao B > Subject: [PATCH] SecurityPkg: Tcg2: Fix TCG2 PP issues >=20 > Several issues exist in TCG2 PP > 1. TCG2 PP use NVS PPRQ/PPRM as PP parameter as well as current PP state > cache. But it doesn't handle PP set failure case 2. TCG2 PP Submit TPM > Operation Request to Pre-OS Environment forgets to clean PPRM 3. Potentia= l > alignment issue >=20 > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Chao Zhang > --- > .../Include/Library/Tcg2PhysicalPresenceLib.h | 24 +++++- > .../SmmTcg2PhysicalPresenceLib.c | 99 ++++++++++++++++= +----- > SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | 14 ++- > SecurityPkg/Tcg/Tcg2Smm/Tpm.asl | 1 + > 4 files changed, 111 insertions(+), 27 deletions(-) >=20 > diff --git a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h > b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h > index ce45f17..1bee13a 100644 > --- a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h > +++ b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h > @@ -2,7 +2,7 @@ > Ihis library is intended to be used by BDS modules. > This library will execute TPM2 request. >=20 > -Copyright (c) 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
> This program and the accompanying materials are licensed and made > available under the terms and conditions of the BSD License which > accompanies this distribution. The full text of the license may be found= at > @@ -116,6 +116,26 @@ > Tcg2PhysicalPresenceLibReturnOperationResponseToOsFunction ( > OUT UINT32 *Response > ); >=20 > +/** > + The handler for TPM physical presence function: > + Submit TPM Operation Request to Pre-OS Environment and > + Submit TPM Operation Request to Pre-OS Environment 2. > + > + This API should be invoked in OS runtime phase to interface with ACPI > method. > + > + Caution: This function may receive untrusted input. > + > + @param[in out] Pointer to OperationRequest TPM physical presence > operation request. > + @param[in out] Pointer to RequestParameter TPM physical presence > operation request parameter. > + > + @return Return Code for Submit TPM Operation Request to Pre-OS > Environment and > + Submit TPM Operation Request to Pre-OS Environment 2. > + **/ > +UINT32 > +Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx ( > + IN OUT UINT32 *OperationRequest, > + IN OUT UINT32 *RequestParameter > + ); >=20 > /** > The handler for TPM physical presence function: > @@ -125,7 +145,7 @@ > Tcg2PhysicalPresenceLibReturnOperationResponseToOsFunction ( > This API should be invoked in OS runtime phase to interface with ACPI > method. >=20 > Caution: This function may receive untrusted input. > - > + > @param[in] OperationRequest TPM physical presence operation reque= st. > @param[in] RequestParameter TPM physical presence operation > request parameter. >=20 > diff --git > a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPrese > nceLib.c > b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPrese > nceLib.c > index 081ec6c..8fcce74 100644 > --- > a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPrese > nceLib.c > +++ > b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPres > +++ enceLib.c > @@ -28,6 +28,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY > KIND, EITHER EXPRESS OR IMPLIED. > #include >=20 > #include > +#include > #include > #include >=20 > @@ -89,26 +90,27 @@ > Tcg2PhysicalPresenceLibReturnOperationResponseToOsFunction ( > This API should be invoked in OS runtime phase to interface with ACPI > method. >=20 > Caution: This function may receive untrusted input. > - > - @param[in] OperationRequest TPM physical presence operation > request. > - @param[in] RequestParameter TPM physical presence operation > request parameter. > + > + @param[in out] Pointer to OperationRequest TPM physical presence > operation request. > + @param[in out] Pointer to RequestParameter TPM physical presence > operation request parameter. >=20 > @return Return Code for Submit TPM Operation Request to Pre-OS > Environment and > - Submit TPM Operation Request to Pre-OS Environment 2. > -**/ > + Submit TPM Operation Request to Pre-OS Environment 2. > + **/ > UINT32 > -EFIAPI > -Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction ( > - IN UINT32 OperationRequest, > - IN UINT32 RequestParameter > +Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx ( > + IN OUT UINT32 *OperationRequest, > + IN OUT UINT32 *RequestParameter > ) > { > EFI_STATUS Status; > + UINT32 ReturnCode; > UINTN DataSize; > EFI_TCG2_PHYSICAL_PRESENCE PpData; > EFI_TCG2_PHYSICAL_PRESENCE_FLAGS Flags; >=20 > - DEBUG ((EFI_D_INFO, "[TPM2] SubmitRequestToPreOSFunction, Request > =3D %x, %x\n", OperationRequest, RequestParameter)); > + DEBUG ((EFI_D_INFO, "[TPM2] SubmitRequestToPreOSFunction, Request > =3D > + %x, %x\n", *OperationRequest, *RequestParameter)); ReturnCode =3D > + TCG_PP_SUBMIT_REQUEST_TO_PREOS_SUCCESS; >=20 > // > // Get the Physical Presence variable @@ -123,21 +125,23 @@ > Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction ( > ); > if (EFI_ERROR (Status)) { > DEBUG ((EFI_D_ERROR, "[TPM2] Get PP variable failure! Status =3D %r\= n", > Status)); > - return TCG_PP_SUBMIT_REQUEST_TO_PREOS_GENERAL_FAILURE; > + ReturnCode =3D > TCG_PP_SUBMIT_REQUEST_TO_PREOS_GENERAL_FAILURE; > + goto EXIT; > } >=20 > - if ((OperationRequest > TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) > && > - (OperationRequest < > TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) ) { > + if ((*OperationRequest > TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) > && > + (*OperationRequest < > + TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) ) { > // > // This command requires UI to prompt user for Auth data. > // > - return TCG_PP_SUBMIT_REQUEST_TO_PREOS_NOT_IMPLEMENTED; > + ReturnCode =3D > TCG_PP_SUBMIT_REQUEST_TO_PREOS_NOT_IMPLEMENTED; > + goto EXIT; > } >=20 > - if ((PpData.PPRequest !=3D OperationRequest) || > - (PpData.PPRequestParameter !=3D RequestParameter)) { > - PpData.PPRequest =3D (UINT8)OperationRequest; > - PpData.PPRequestParameter =3D RequestParameter; > + if ((PpData.PPRequest !=3D *OperationRequest) || > + (PpData.PPRequestParameter !=3D *RequestParameter)) { > + PpData.PPRequest =3D (UINT8)*OperationRequest; > + PpData.PPRequestParameter =3D *RequestParameter; > DataSize =3D sizeof (EFI_TCG2_PHYSICAL_PRESENCE); > Status =3D mTcg2PpSmmVariable->SmmSetVariable ( > TCG2_PHYSICAL_PRESENCE_VARIABLE, @@ -= 150,10 > +154,11 @@ Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction ( >=20 > if (EFI_ERROR (Status)) { > DEBUG ((EFI_D_ERROR, "[TPM2] Set PP variable failure! Status =3D %r\= n", > Status)); > - return TCG_PP_SUBMIT_REQUEST_TO_PREOS_GENERAL_FAILURE; > + ReturnCode =3D > TCG_PP_SUBMIT_REQUEST_TO_PREOS_GENERAL_FAILURE; > + goto EXIT; > } >=20 > - if (OperationRequest >=3D > TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) { > + if (*OperationRequest >=3D > + TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) { > DataSize =3D sizeof (EFI_TCG2_PHYSICAL_PRESENCE_FLAGS); > Status =3D mTcg2PpSmmVariable->SmmGetVariable ( > TCG2_PHYSICAL_PRESENCE_FLAGS_VARIABLE= , > @@ -165,10 +170,60 @@ > Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction ( > if (EFI_ERROR (Status)) { > Flags.PPFlags =3D TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT; > } > - return Tcg2PpVendorLibSubmitRequestToPreOSFunction > (OperationRequest, Flags.PPFlags, RequestParameter); > + ReturnCode =3D Tcg2PpVendorLibSubmitRequestToPreOSFunction > + (*OperationRequest, Flags.PPFlags, *RequestParameter); > } >=20 > - return TCG_PP_SUBMIT_REQUEST_TO_PREOS_SUCCESS; > +EXIT: > + // > + // Sync PPRQ/PPRM from PP Variable if PP submission fails > + // > + if (ReturnCode !=3D TCG_PP_SUBMIT_REQUEST_TO_PREOS_SUCCESS) { > + DEBUG ((EFI_D_ERROR, "[TPM2] Submit PP Request failure! Sync > PPRQ/PPRM with PP variable.\n", Status)); > + DataSize =3D sizeof (EFI_TCG2_PHYSICAL_PRESENCE); > + ZeroMem(&PpData, DataSize); > + Status =3D mTcg2PpSmmVariable->SmmGetVariable ( > + TCG2_PHYSICAL_PRESENCE_VARIABLE, > + &gEfiTcg2PhysicalPresenceGuid, > + NULL, > + &DataSize, > + &PpData > + ); > + *OperationRequest =3D (UINT32)PpData.PPRequest; > + *RequestParameter =3D PpData.PPRequestParameter; > + } > + > + return ReturnCode; > +} > + > +/** > + The handler for TPM physical presence function: > + Submit TPM Operation Request to Pre-OS Environment and > + Submit TPM Operation Request to Pre-OS Environment 2. > + > + This API should be invoked in OS runtime phase to interface with ACPI > method. > + > + Caution: This function may receive untrusted input. > + > + @param[in] OperationRequest TPM physical presence operation > request. > + @param[in] RequestParameter TPM physical presence operation > request parameter. > + > + @return Return Code for Submit TPM Operation Request to Pre-OS > Environment and > + Submit TPM Operation Request to Pre-OS Environment 2. > +**/ > +UINT32 > +EFIAPI > +Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction ( > + IN UINT32 OperationRequest, > + IN UINT32 RequestParameter > + ) > +{ > + UINT32 TempOperationRequest; > + UINT32 TempRequestParameter; > + > + TempOperationRequest =3D OperationRequest; TempRequestParameter =3D > + RequestParameter; > + > + return > + > Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx(&TempOperatio > nRe > + quest, &TempRequestParameter); > } >=20 > /** > diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c > b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c > index f3b7641..d02123d 100644 > --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c > +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c > @@ -119,6 +119,9 @@ PhysicalPresenceCallback ( { > UINT32 MostRecentRequest; > UINT32 Response; > + UINT32 OperationRequest; > + UINT32 RequestParameter; > + >=20 > if (mTcgNvs->PhysicalPresence.Parameter =3D=3D > TCG_ACPI_FUNCTION_RETURN_REQUEST_RESPONSE_TO_OS) { > mTcgNvs->PhysicalPresence.ReturnCode =3D > Tcg2PhysicalPresenceLibReturnOperationResponseToOsFunction ( @@ - > 130,10 +133,15 @@ PhysicalPresenceCallback ( > return EFI_SUCCESS; > } else if ((mTcgNvs->PhysicalPresence.Parameter =3D=3D > TCG_ACPI_FUNCTION_SUBMIT_REQUEST_TO_BIOS) > || (mTcgNvs->PhysicalPresence.Parameter =3D=3D > TCG_ACPI_FUNCTION_SUBMIT_REQUEST_TO_BIOS_2)) { > - mTcgNvs->PhysicalPresence.ReturnCode =3D > Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction ( > - mTcgNvs->PhysicalPresence.R= equest, > - mTcgNvs->PhysicalPresence.R= equestParameter > + > + OperationRequest =3D mTcgNvs->PhysicalPresence.Request; > + RequestParameter =3D mTcgNvs->PhysicalPresence.RequestParameter; > + mTcgNvs->PhysicalPresence.ReturnCode =3D > Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx ( > + &OperationRequest, > + &RequestParameter > ); > + mTcgNvs->PhysicalPresence.Request =3D OperationRequest; > + mTcgNvs->PhysicalPresence.RequestParameter =3D RequestParameter; > } else if (mTcgNvs->PhysicalPresence.Parameter =3D=3D > TCG_ACPI_FUNCTION_GET_USER_CONFIRMATION_STATUS_FOR_REQUEST) > { > mTcgNvs->PhysicalPresence.ReturnCode =3D > Tcg2PhysicalPresenceLibGetUserConfirmationStatusFunction (mTcgNvs- > >PPRequestUserConfirm); > } > diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl > b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl index 84143cf..2083a3e 100644 > --- a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl > +++ b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl > @@ -198,6 +198,7 @@ DefinitionBlock ( > // >=20 > Store (DerefOf (Index (Arg2, 0x00)), PPRQ) > + Store (0, PPRM) > Store (0x02, PPIP) >=20 > // > -- > 1.9.5.msysgit.1