1. Patchew
  2. linux
  3. View series

[PATCH v2] efivarfs: Fix slab-out-of-bounds in efivarfs_d_compare

linan666@huaweicloud.com posted 1 patch 1 month, 2 weeks ago
There is a newer version of this series
fs/efivarfs/super.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v2] efivarfs: Fix slab-out-of-bounds in efivarfs_d_compare
Posted by linan666@huaweicloud.com 1 month, 2 weeks ago 
From: Li Nan <linan122@huawei.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@huawei.com>
Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
---
v2: optimize commit message

 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;
 
-	if (name->len != len)
+	if (name->len != len || len <= EFI_VARIABLE_GUID_LEN)
 		return 1;
 
 	/* Case-sensitive compare for the variable name */
-- 
2.39.2
Re: [PATCH v2] efivarfs: Fix slab-out-of-bounds in efivarfs_d_compare
Posted by Ard Biesheuvel 1 month, 1 week ago 
On Tue, 19 Aug 2025 at 11:16, <linan666@huaweicloud.com> wrote:
>
> From: Li Nan <linan122@huawei.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@huawei.com>
> Signed-off-by: Wu Guanghao <wuguanghao3@huawei.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?

>  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?

> -       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
>
>
Re: [PATCH v2] efivarfs: Fix slab-out-of-bounds in efivarfs_d_compare
Posted by Li Nan 1 month, 1 week ago 

在 2025/8/26 21:32, Ard Biesheuvel 写道:
> On Tue, 19 Aug 2025 at 11:16, <linan666@huaweicloud.com> wrote:
>>
>> From: Li Nan <linan122@huawei.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@huawei.com>
>> Signed-off-by: Wu Guanghao <wuguanghao3@huawei.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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.