From: "Long, Qin" <qin.long@intel.com>
To: "Zhang, Chao B" <chao.b.zhang@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Yao, Jiewen" <jiewen.yao@intel.com>
Subject: Re: [PATCH] SecurityPkg: Tcg2: Fix TCG2 PP issues
Date: Tue, 27 Sep 2016 05:08:11 +0000 [thread overview]
Message-ID: <BF2CCE9263284D428840004653A28B6E51534348@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <1474941152-15460-1-git-send-email-chao.b.zhang@intel.com>
Reviewed-by: Qin Long <qin.long@intel.com>
> -----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
>
> 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. Potential
> alignment issue
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
> ---
> .../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(-)
>
> 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.
>
> -Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
> 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
> );
>
> +/**
> + 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
> + );
>
> /**
> 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.
>
> Caution: This function may receive untrusted input.
> -
> +
> @param[in] OperationRequest TPM physical presence operation request.
> @param[in] RequestParameter TPM physical presence operation
> request parameter.
>
> 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 <Protocol/SmmVariable.h>
>
> #include <Library/DebugLib.h>
> +#include <Library/BaseMemorylib.h>
> #include <Library/Tcg2PpVendorLib.h>
> #include <Library/SmmServicesTableLib.h>
>
> @@ -89,26 +90,27 @@
> Tcg2PhysicalPresenceLibReturnOperationResponseToOsFunction (
> 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.
> +
> + @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.
> -**/
> + 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;
>
> - DEBUG ((EFI_D_INFO, "[TPM2] SubmitRequestToPreOSFunction, Request
> = %x, %x\n", OperationRequest, RequestParameter));
> + DEBUG ((EFI_D_INFO, "[TPM2] SubmitRequestToPreOSFunction, Request
> =
> + %x, %x\n", *OperationRequest, *RequestParameter)); ReturnCode =
> + TCG_PP_SUBMIT_REQUEST_TO_PREOS_SUCCESS;
>
> //
> // Get the Physical Presence variable @@ -123,21 +125,23 @@
> Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction (
> );
> if (EFI_ERROR (Status)) {
> DEBUG ((EFI_D_ERROR, "[TPM2] Get PP variable failure! Status = %r\n",
> Status));
> - return TCG_PP_SUBMIT_REQUEST_TO_PREOS_GENERAL_FAILURE;
> + ReturnCode =
> TCG_PP_SUBMIT_REQUEST_TO_PREOS_GENERAL_FAILURE;
> + goto EXIT;
> }
>
> - 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 =
> TCG_PP_SUBMIT_REQUEST_TO_PREOS_NOT_IMPLEMENTED;
> + goto EXIT;
> }
>
> - if ((PpData.PPRequest != OperationRequest) ||
> - (PpData.PPRequestParameter != RequestParameter)) {
> - PpData.PPRequest = (UINT8)OperationRequest;
> - PpData.PPRequestParameter = RequestParameter;
> + if ((PpData.PPRequest != *OperationRequest) ||
> + (PpData.PPRequestParameter != *RequestParameter)) {
> + PpData.PPRequest = (UINT8)*OperationRequest;
> + PpData.PPRequestParameter = *RequestParameter;
> DataSize = sizeof (EFI_TCG2_PHYSICAL_PRESENCE);
> Status = mTcg2PpSmmVariable->SmmSetVariable (
> TCG2_PHYSICAL_PRESENCE_VARIABLE, @@ -150,10
> +154,11 @@ Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction (
>
> if (EFI_ERROR (Status)) {
> DEBUG ((EFI_D_ERROR, "[TPM2] Set PP variable failure! Status = %r\n",
> Status));
> - return TCG_PP_SUBMIT_REQUEST_TO_PREOS_GENERAL_FAILURE;
> + ReturnCode =
> TCG_PP_SUBMIT_REQUEST_TO_PREOS_GENERAL_FAILURE;
> + goto EXIT;
> }
>
> - if (OperationRequest >=
> TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
> + if (*OperationRequest >=
> + TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
> DataSize = sizeof (EFI_TCG2_PHYSICAL_PRESENCE_FLAGS);
> Status = mTcg2PpSmmVariable->SmmGetVariable (
> TCG2_PHYSICAL_PRESENCE_FLAGS_VARIABLE,
> @@ -165,10 +170,60 @@
> Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction (
> if (EFI_ERROR (Status)) {
> Flags.PPFlags = TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT;
> }
> - return Tcg2PpVendorLibSubmitRequestToPreOSFunction
> (OperationRequest, Flags.PPFlags, RequestParameter);
> + ReturnCode = Tcg2PpVendorLibSubmitRequestToPreOSFunction
> + (*OperationRequest, Flags.PPFlags, *RequestParameter);
> }
>
> - return TCG_PP_SUBMIT_REQUEST_TO_PREOS_SUCCESS;
> +EXIT:
> + //
> + // Sync PPRQ/PPRM from PP Variable if PP submission fails
> + //
> + if (ReturnCode != 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 = sizeof (EFI_TCG2_PHYSICAL_PRESENCE);
> + ZeroMem(&PpData, DataSize);
> + Status = mTcg2PpSmmVariable->SmmGetVariable (
> + TCG2_PHYSICAL_PRESENCE_VARIABLE,
> + &gEfiTcg2PhysicalPresenceGuid,
> + NULL,
> + &DataSize,
> + &PpData
> + );
> + *OperationRequest = (UINT32)PpData.PPRequest;
> + *RequestParameter = 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 = OperationRequest; TempRequestParameter =
> + RequestParameter;
> +
> + return
> +
> Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx(&TempOperatio
> nRe
> + quest, &TempRequestParameter);
> }
>
> /**
> 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;
> +
>
> if (mTcgNvs->PhysicalPresence.Parameter ==
> TCG_ACPI_FUNCTION_RETURN_REQUEST_RESPONSE_TO_OS) {
> mTcgNvs->PhysicalPresence.ReturnCode =
> Tcg2PhysicalPresenceLibReturnOperationResponseToOsFunction ( @@ -
> 130,10 +133,15 @@ PhysicalPresenceCallback (
> return EFI_SUCCESS;
> } else if ((mTcgNvs->PhysicalPresence.Parameter ==
> TCG_ACPI_FUNCTION_SUBMIT_REQUEST_TO_BIOS)
> || (mTcgNvs->PhysicalPresence.Parameter ==
> TCG_ACPI_FUNCTION_SUBMIT_REQUEST_TO_BIOS_2)) {
> - mTcgNvs->PhysicalPresence.ReturnCode =
> Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction (
> - mTcgNvs->PhysicalPresence.Request,
> - mTcgNvs->PhysicalPresence.RequestParameter
> +
> + OperationRequest = mTcgNvs->PhysicalPresence.Request;
> + RequestParameter = mTcgNvs->PhysicalPresence.RequestParameter;
> + mTcgNvs->PhysicalPresence.ReturnCode =
> Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx (
> + &OperationRequest,
> + &RequestParameter
> );
> + mTcgNvs->PhysicalPresence.Request = OperationRequest;
> + mTcgNvs->PhysicalPresence.RequestParameter = RequestParameter;
> } else if (mTcgNvs->PhysicalPresence.Parameter ==
> TCG_ACPI_FUNCTION_GET_USER_CONFIRMATION_STATUS_FOR_REQUEST)
> {
> mTcgNvs->PhysicalPresence.ReturnCode =
> 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 (
> //
>
> Store (DerefOf (Index (Arg2, 0x00)), PPRQ)
> + Store (0, PPRM)
> Store (0x02, PPIP)
>
> //
> --
> 1.9.5.msysgit.1
prev parent reply other threads:[~2016-09-27 5:08 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-27 1:52 [PATCH] SecurityPkg: Tcg2: Fix TCG2 PP issues Zhang, Chao B
2016-09-27 5:08 ` Long, Qin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BF2CCE9263284D428840004653A28B6E51534348@SHSMSX103.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox