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

Message-ID: <CAMj1kXHvEOTHHshS9uoL4RSZgJZgd9OR9qzPHYJ5xW2Vgeefcg@mail.gmail.com>
Date: Tue, 26 Aug 2025 15:32:14 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: linan666@...weicloud.com
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

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?

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