[SLOF] [PATCH 2/2] tcgbios: Only measure size indicated in UEFI partition table header

[Stefan Berger]

On 4/3/25 11:46 PM, Alexey Kardashevskiy wrote:
> 
> 
> On Tue, 25 Mar 2025, at 04:29, Stefan Berger wrote:
>> From: Stefan Berger <stefanb at linux.ibm.com>
>>
>> Remove the reserved byte array from the UEFI_PARTITION_TABLE_HEADER
>> to avoid measuring zero bytes. Only log and measure the header as
>> indicated by its size. This size is expected to be exactly the size
>> of UEFI_PARTITION_TABLE_HEADER (= 92 bytes).
>>
>> Fixes: 16a1867425ee ("tcgbios: Measure the GPT table")
>> Signed-off-by: Stefan Berger <stefanb at linux.ibm.com>
>> ---
>> lib/libtpm/tcgbios.c     | 19 +++++++++++++++++--
>> lib/libtpm/tcgbios_int.h |  1 -
>> 2 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/libtpm/tcgbios.c b/lib/libtpm/tcgbios.c
>> index 9a10dbe..8fd404e 100644
>> --- a/lib/libtpm/tcgbios.c
>> +++ b/lib/libtpm/tcgbios.c
>> @@ -1080,6 +1080,9 @@ uint32_t tpm_measure_bcv_mbr(uint32_t bootdrv, const uint8_t *addr,
>>    */
>> void tpm_gpt_set_lba1(const uint8_t *addr, uint32_t length)
>> {
>> + UEFI_PARTITION_TABLE_HEADER *upth;
> 
> You could initialize it right here.
> 
>> + size_t to_copy;
>> +
>> if (!tpm_is_working())
>> return;
>>   
>> @@ -1090,8 +1093,20 @@ void tpm_gpt_set_lba1(const uint8_t *addr, uint32_t length)
>> if (!uefi_gpt_data)
>> return;
>>   
>> - memcpy(&uefi_gpt_data->EfiPartitionHeader,
>> -        addr, MIN(sizeof(uefi_gpt_data->EfiPartitionHeader), length));
>> + upth = (UEFI_PARTITION_TABLE_HEADER *)addr;
>> +
>> + /* check whether header.size is part of @addr array */
>> + if (length < offset_of(UEFI_PARTITION_TABLE_HEADER, header.size) +
>> + sizeof(upth->header.size))
> 
> It should really be "length < sizeof(uefi_gpt_data->EfiPartitionHeader)" here and drop "length < to_copy" below.

Yes, that's what it is finally going to have to be.

> 
>> + return;
>> + to_copy = le32_to_cpu(upth->header.size);
>> +
>> + /* Expecting to copy exactly the size of EfiPartitionHeader (92bytes) */
>> + if (length < to_copy ||
>> +     to_copy != sizeof(uefi_gpt_data->EfiPartitionHeader))
> 
> if ( le32_to_cpu(upth->header.size) != sizeof(uefi_gpt_data->EfiPartitionHeader))
>     return;

Ack.

> 
>> + return;
>> +
>> + memcpy(&uefi_gpt_data->EfiPartitionHeader, addr, to_copy);
> 
> s/to_copy/sizeof(uefi_gpt_data->EfiPartitionHeader)/
> 
> and then you can ditch @to_copy. Thanks,
> 
>> uefi_gpt_data->NumberOfPartitions = cpu_to_le64(0);
>> }
>>   
>> diff --git a/lib/libtpm/tcgbios_int.h b/lib/libtpm/tcgbios_int.h
>> index cc38455..5bbd9d5 100644
>> --- a/lib/libtpm/tcgbios_int.h
>> +++ b/lib/libtpm/tcgbios_int.h
>> @@ -116,7 +116,6 @@ typedef struct {
>> uint32_t numPartEntry;
>> uint32_t partEntrySize;
>> uint32_t partArrayCrc32;
>> - uint8_t reserved[420];
>> } __attribute__((packed)) UEFI_PARTITION_TABLE_HEADER;
>>   
>> typedef struct {
>> -- 
>> 2.25.1
>>
>>