linux-kernel - Re: [PATCH v2] efivarfs: Fix slab-out-of-bounds in efivarfs_d

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] [day] [month] [year] [list]

Message-ID: <e1212545-2013-775d-e3a8-07b3be2cfc00@huaweicloud.com>
Date: Wed, 27 Aug 2025 10:23:44 +0800
From: Li Nan <linan666@...weicloud.com>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: jk@...abs.org, linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org,
 yangerkun@...wei.com, yi.zhang@...wei.com
Subject: Re: [PATCH v2] efivarfs: Fix slab-out-of-bounds in efivarfs_d_compare



在 2025/8/26 21:32, Ard Biesheuvel 写道:
> On Tue, 19 Aug 2025 at 11:16, <linan666@...weicloud.com> wrote:
>>
>> From: Li Nan <linan122@...wei.com>
>>
>> Observed on kernel 6.6 (present on master as well):
>>
>>    BUG: KASAN: slab-out-of-bounds in memcmp+0x98/0xd0
>>    Call trace:
>>     kasan_check_range+0xe8/0x190
>>     __asan_loadN+0x1c/0x28
>>     memcmp+0x98/0xd0
>>     efivarfs_d_compare+0x68/0xd8
>>     __d_lookup_rcu_op_compare+0x178/0x218
>>     __d_lookup_rcu+0x1f8/0x228
>>     d_alloc_parallel+0x150/0x648
>>     lookup_open.isra.0+0x5f0/0x8d0
>>     open_last_lookups+0x264/0x828
>>     path_openat+0x130/0x3f8
>>     do_filp_open+0x114/0x248
>>     do_sys_openat2+0x340/0x3c0
>>     __arm64_sys_openat+0x120/0x1a0
>>
>> If dentry->d_name.len < EFI_VARIABLE_GUID_LEN , 'guid' can become
>> negative, leadings to oob. The issue can be triggered as below:
>>
>>    T1                    T2
>>    lookup_open
>>     ->lookup
>>      simple_lookup
>>       d_add
>>       // invalid dentry is added to hash list
>>
>>                          lookup_open
>>                           d_alloc_parallel
>>                            __d_lookup_rcu
>>                             __d_lookup_rcu_op_compare
>>                              hlist_bl_for_each_entry_rcu
>>                              // invalid dentry can be retrieved
>>                               ->d_compare
>>                                efivarfs_d_compare
>>
>> Fix it by checking len before cmp.
>>
>> Fixes: da27a24383b2 ("efivarfs: guid part of filenames are case-insensitive")
>> Signed-off-by: Li Nan <linan122@...wei.com>
>> Signed-off-by: Wu Guanghao <wuguanghao3@...wei.com>
>> ---
>> v2: optimize commit message
>>
> 
> Thanks for the fix, and for the elaborate description.
> 
> IIUC, two parallel lookups using an invalid filename can reproduce this?
> 

Thansk for your review.

Yes, the filename is invalid. I'll add that to the commit message in the 
next version.

>>   fs/efivarfs/super.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
>> index 23ff4e873651..c30d758e303a 100644
>> --- a/fs/efivarfs/super.c
>> +++ b/fs/efivarfs/super.c
>> @@ -152,7 +152,7 @@ static int efivarfs_d_compare(const struct dentry *dentry,
>>   {
>>          int guid = len - EFI_VARIABLE_GUID_LEN;
>>
> 
> Could we do a separate
> 
> if (guid <= 0)
>    return 1;
> 
> here, with a comment describing how that condition might become true?
> 

Okay, I will update it in v3.

>> -       if (name->len != len)
>> +       if (name->len != len || len <= EFI_VARIABLE_GUID_LEN)
> 
> ... and drop this change.
> 
>>                  return 1;
>>
>>          /* Case-sensitive compare for the variable name */
>> --
>> 2.39.2
>>
>>

-- 
Thanks,
Nan