Re: [PATCH 3/3] efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check

[Heinrich Schuchardt]

On 9/3/21 3:55 AM, Masahisa Kojima wrote:

TCG EFI Protocol Specification defines that PCRIndex parameter
passed from caller must be 0 to 23.
TPM2_MAX_PCRS is currently used to check the range of PCRIndex,
but TPM2_MAX_PCRS is tpm2 device dependent and may have larger value.
This commit newly adds EFI_TCG2_MAX_PCR_INDEX macro, it is used to
check the range of PCRIndex parameter.

In the U-Boot code we have TPM2_MAX_PCRS hard coded as 32. Can the value
be less?

Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org>
---
  include/efi_tcg2.h        | 2 ++
  lib/efi_loader/efi_tcg2.c | 2 +-
  2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index 45788d55d5..b647361d44 100644
--- a/include/efi_tcg2.h
+++ b/include/efi_tcg2.h
@@ -28,6 +28,8 @@
  #define EFI_TCG2_EXTEND_ONLY 0x0000000000000001
  #define PE_COFF_IMAGE 0x0000000000000010

+#define EFI_TCG2_MAX_PCR_INDEX 23
+
  /* Algorithm Registry */
  #define EFI_TCG2_BOOT_HASH_ALG_SHA1    0x00000001
  #define EFI_TCG2_BOOT_HASH_ALG_SHA256  0x00000002
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index c4e9f61fd6..b268a02976 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -958,7 +958,7 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol 
*this, u64 flags,
                goto out;
        }

-       if (efi_tcg_event->header.pcr_index > TPM2_MAX_PCRS) {

If TPM2_MAX_PCRS were device dependent and could be less then 23 we
would need a check against both constants.

Could you, please, clarify the issue. If TPM2_MAX_PCRS is always greater
then 23, please, mention this in the commit message and perhaps add a
comment in the code here.

Best regards

Heinrich

+       if (efi_tcg_event->header.pcr_index > EFI_TCG2_MAX_PCR_INDEX) {
                ret = EFI_INVALID_PARAMETER;
                goto out;
        }