linux-kernel - Re: [PATCH] mm/kfence: fix null pointer dereference on pointer meta

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: <CANpmjNP8uAexEZ3Qa-GfBfX6V8tAd7NK0vt3T3Xjh4CkzxfS-g@mail.gmail.com>
Date:   Sat, 23 Oct 2021 20:47:17 +0200
From:   Marco Elver <elver@...gle.com>
To:     Chengfeng Ye <cyeaa@...nect.ust.hk>
Cc:     glider@...gle.com, akpm@...ux-foundation.org, dvyukov@...gle.com,
        kasan-dev@...glegroups.com, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm/kfence: fix null pointer dereference on pointer meta

On Sat, 23 Oct 2021 at 19:20, Chengfeng Ye <cyeaa@...nect.ust.hk> wrote:
> The pointer meta return from addr_to_metadata could be null, so
> there is a potential null pointer dereference issue. Fix this
> by adding a null check before dereference.
>
> Fixes: 0ce20dd8 ("mm: add Kernel Electric-Fence infrastructure")
> Signed-off-by: Chengfeng Ye <cyeaa@...nect.ust.hk>
> ---
>  mm/kfence/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/kfence/core.c b/mm/kfence/core.c
> index 7a97db8bc8e7..7d2ec787e921 100644
> --- a/mm/kfence/core.c
> +++ b/mm/kfence/core.c
> @@ -811,7 +811,7 @@ void __kfence_free(void *addr)
>          * objects once it has been freed. meta->cache may be NULL if the cache
>          * was destroyed.
>          */
> -       if (unlikely(meta->cache && (meta->cache->flags & SLAB_TYPESAFE_BY_RCU)))
> +       if (unlikely(meta && meta->cache && (meta->cache->flags & SLAB_TYPESAFE_BY_RCU)))
>                 call_rcu(&meta->rcu_head, rcu_guarded_free);
>         else
>                 kfence_guarded_free(addr, meta, false);

Sorry -- Nack. What bug did you encounter?

Please see [1], and I'm afraid this attempt makes even less sense
because if it were (hypothetically) NULL like you say we just call
kfence_guarded_free() and crash there.

[1] https://lkml.kernel.org/r/CANpmjNMcgUsdvXrvQHn+-y1w-z-6QAS+WJ27RB2DCnVxORRcuw@mail.gmail.com

However, what I wrote in [1] equally applies here:

> [...]
> Adding a check like this could also hide genuine bugs, as meta should
> never be NULL in __kfence_free(). If it is, we'd like to see a crash.
>
> Did you read kfence_free() in include/linux/kfence.h? It already
> prevents __kfence_free() being called with a non-KFENCE address.
>
> Without a more thorough explanation, Nack.

May I ask which static analysis tool keeps flagging this?

Thanks,
-- Marco