Re: [PATCH] efi: Replace efi_memory_attributes_table_t 0-sized array with flexible array

["Gustavo A. R. Silva" <gustavo@embeddedor.com>]

On 10/07/24 17:32, Gustavo A. R. Silva wrote:
> 
> 
> On 10/07/24 16:55, Kees Cook wrote:
>> While efi_memory_attributes_table_t::entry isn't used directly as an
>> array, it is used as a base for pointer arithmetic. The type is wrong
>> as it's not technically an array of efi_memory_desc_t's; they could be
>> larger. Regardless, leave the type unchanged and remove the old style
>> "0" array size. Additionally replace the open-coded entry offset code
>> with the existing efi_early_memdesc_ptr() helper.
>>
>> Signed-off-by: Kees Cook <kees@kernel.org>
>> ---
>> Cc: Ard Biesheuvel <ardb@kernel.org>
>> Cc: linux-efi@vger.kernel.org
>> ---
>>   drivers/firmware/efi/memattr.c | 2 +-
>>   include/linux/efi.h            | 6 +++++-
>>   2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
>> index ab85bf8e165a..01142604e8df 100644
>> --- a/drivers/firmware/efi/memattr.c
>> +++ b/drivers/firmware/efi/memattr.c
>> @@ -164,7 +164,7 @@ int __init efi_memattr_apply_permissions(struct mm_struct *mm,
>>           bool valid;
>>           char buf[64];
>> -        valid = entry_is_valid((void *)tbl->entry + i * tbl->desc_size,
>> +        valid = entry_is_valid(efi_early_memdesc_ptr(tbl->entry, tbl->desc_size, i),
>>                          &md);
>>           size = md.num_pages << EFI_PAGE_SHIFT;
>>           if (efi_enabled(EFI_DBG) || !valid)
>> diff --git a/include/linux/efi.h b/include/linux/efi.h
>> index 418e555459da..b06639c4f6a5 100644
>> --- a/include/linux/efi.h
>> +++ b/include/linux/efi.h
>> @@ -607,7 +607,11 @@ typedef struct {
>>       u32 num_entries;
>>       u32 desc_size;
>>       u32 flags;
>> -    efi_memory_desc_t entry[0];
>> +    /*
>> +     * There are @num_entries following, each of size @desc_size bytes,
>> +     * including an efi_memory_desc_t header.
>> +     */
>> +    efi_memory_desc_t entry[];
> 
> a candidate for future __counted_by(num_entries * desc_size) ? :p

ah no, this rather be something more like __sized_by(num_entries * desc_size).

--
Gustavo

> 
> Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> 
> Thanks