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

Heinrich Schuchardt xypron.glpk at gmx.de
Fri Sep 3 09:11:38 CEST 2021


On 9/3/21 8:51 AM, Ilias Apalodimas wrote:
> Hi Heinrich,
>
> On Fri, Sep 03, 2021 at 08:22:30AM +0200, Heinrich Schuchardt wrote:
>> 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?
>
> This is defined in [1]
> [1] https://trustedcomputinggroup.org/wp-content/uploads/TSS_Overview_Common_v0p9_r10_12june2021.pdf
>
>>
>>>
>>> Signed-off-by: Masahisa Kojima <masahisa.kojima at 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.
>
> You don't need that (I think). If the spec says you have to check against
> 23, then that's what Kojima-san fixes here.
> If the device supports less than 23, then the current code as-is will
> return EFI_DEVICE_ERROR, in case the device has less than 23 PCRs.
>
> We could ofc just check against the value we get from GetCapability, which
> would be correct as well?

Acked-by: Heinrich Schuchardt <xypron.glpk at gmx.de>

>
>>
>> Best regards
>>
>> Heinrich
>>
>>> +	if (efi_tcg_event->header.pcr_index > EFI_TCG2_MAX_PCR_INDEX) {
>>>    		ret = EFI_INVALID_PARAMETER;
>>>    		goto out;
>>>    	}
>>>
>
> [1] https://trustedcomputinggroup.org/wp-content/uploads/TSS_Overview_Common_v0p9_r10_12june2021.pdf
>
> Regards
> /Ilias
>


More information about the U-Boot mailing list