linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] efivarfs: Fix slab-out-of-bounds in efivarfs_d_compare
@ 2025-08-27  7:39 linan666
  2025-08-28  6:40 ` Ard Biesheuvel
  0 siblings, 1 reply; 2+ messages in thread
From: linan666 @ 2025-08-27  7:39 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 by parallel
lookups using invalid filename:

  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
			      // oob

Fix it by checking 'guid' 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>
---
v3: Check guid directly, add comment. Emphasize 'invalid filename' in
    commit message.

 fs/efivarfs/super.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index c4a139911356..4bb4002e3cdf 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -152,6 +152,10 @@ static int efivarfs_d_compare(const struct dentry *dentry,
 {
 	int guid = len - EFI_VARIABLE_GUID_LEN;
 
+	/* Parallel lookups may produce a temporary invalid filename */
+	if (guid <= 0)
+		return 1;
+
 	if (name->len != len)
 		return 1;
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v3] efivarfs: Fix slab-out-of-bounds in efivarfs_d_compare
  2025-08-27  7:39 [PATCH v3] efivarfs: Fix slab-out-of-bounds in efivarfs_d_compare linan666
@ 2025-08-28  6:40 ` Ard Biesheuvel
  0 siblings, 0 replies; 2+ messages in thread
From: Ard Biesheuvel @ 2025-08-28  6:40 UTC (permalink / raw)
  To: linan666; +Cc: jk, matt.fleming, linux-efi, linux-kernel, yangerkun, yi.zhang

On Wed, 27 Aug 2025 at 09:48, <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 by parallel
> lookups using invalid filename:
>
>   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
>                               // oob
>
> Fix it by checking 'guid' 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>
> ---
> v3: Check guid directly, add comment. Emphasize 'invalid filename' in
>     commit message.
>
>  fs/efivarfs/super.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
> index c4a139911356..4bb4002e3cdf 100644
> --- a/fs/efivarfs/super.c
> +++ b/fs/efivarfs/super.c
> @@ -152,6 +152,10 @@ static int efivarfs_d_compare(const struct dentry *dentry,
>  {
>         int guid = len - EFI_VARIABLE_GUID_LEN;
>
> +       /* Parallel lookups may produce a temporary invalid filename */
> +       if (guid <= 0)
> +               return 1;
> +
>         if (name->len != len)
>                 return 1;
>

Queued up as a fix now - thanks.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-08-28  6:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-27  7:39 [PATCH v3] efivarfs: Fix slab-out-of-bounds in efivarfs_d_compare linan666
2025-08-28  6:40 ` Ard Biesheuvel

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).