* [PATCH v2] efivarfs: Fix slab-out-of-bounds in efivarfs_d_compare
@ 2025-08-19 9:08 linan666
2025-08-26 13:32 ` Ard Biesheuvel
0 siblings, 1 reply; 3+ messages in thread
From: linan666 @ 2025-08-19 9:08 UTC (permalink / raw)
To: jk, ardb, matt.fleming
Cc: linux-efi, linux-kernel, linan666, yangerkun, yi.zhang
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] efivarfs: Fix slab-out-of-bounds in efivarfs_d_compare
2025-08-19 9:08 [PATCH v2] efivarfs: Fix slab-out-of-bounds in efivarfs_d_compare linan666
@ 2025-08-26 13:32 ` Ard Biesheuvel
2025-08-27 2:23 ` Li Nan
0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2025-08-26 13:32 UTC (permalink / raw)
To: linan666; +Cc: jk, linux-efi, linux-kernel, yangerkun, yi.zhang
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
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] efivarfs: Fix slab-out-of-bounds in efivarfs_d_compare
2025-08-26 13:32 ` Ard Biesheuvel
@ 2025-08-27 2:23 ` Li Nan
0 siblings, 0 replies; 3+ messages in thread
From: Li Nan @ 2025-08-27 2:23 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: jk, linux-efi, linux-kernel, yangerkun, yi.zhang
在 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-08-27 2:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-19 9:08 [PATCH v2] efivarfs: Fix slab-out-of-bounds in efivarfs_d_compare linan666
2025-08-26 13:32 ` Ard Biesheuvel
2025-08-27 2:23 ` Li Nan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).