linux-hardening - Re: [PATCH] efi: Replace efi_memory_attributes_table

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <175fc58f-b12a-4bc7-bc74-3365e5b0ee3e@embeddedor.com>
Date: Wed, 10 Jul 2024 17:34:35 -0600
From: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To: Kees Cook <kees@...nel.org>, Ard Biesheuvel <ardb@...nel.org>
Cc: linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-hardening@...r.kernel.org
Subject: Re: [PATCH] efi: Replace efi_memory_attributes_table_t 0-sized array
 with flexible array



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@...nel.org>
>> ---
>> Cc: Ard Biesheuvel <ardb@...nel.org>
>> Cc: linux-efi@...r.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@...nel.org>
> 
> Thanks