linux-kernel - Re: [PATCH] kfence: unpoison pool region before use

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: <CANpmjNOzaOJY5K+Sq78AF5N1_6=1kv3rXZ6w+XPuEf9G+cd8iA@mail.gmail.com>
Date:   Sat, 3 Apr 2021 12:03:19 +0200
From:   Marco Elver <elver@...gle.com>
To:     Peter Collingbourne <pcc@...gle.com>
Cc:     Dmitry Vyukov <dvyukov@...gle.com>,
        Alexander Potapenko <glider@...gle.com>,
        Evgenii Stepanov <eugenis@...gle.com>,
        Andrey Konovalov <andreyknvl@...il.com>,
        Linux Memory Management List <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] kfence: unpoison pool region before use

On Sat, 3 Apr 2021 at 07:13, Peter Collingbourne <pcc@...gle.com> wrote:
> If the memory region allocated by KFENCE had previously been poisoned,
> any validity checks done using kasan_byte_accessible() will fail. Fix
> it by unpoisoning the memory before using it as the pool region.
>
> Link: https://linux-review.googlesource.com/id/I0af99e9f1c25eaf7e1ec295836b5d148d76940c5
> Signed-off-by: Peter Collingbourne <pcc@...gle.com>

Thanks, at a high level this seems reasonable, because we always want
to ensure that KFENCE memory remains unpoisoned with KASAN on. FWIW I
subjected a config with KFENCE+KASAN (generic, SW_TAGS, and HW_TAGS)
to syzkaller testing and ran kfence_test:

  Tested-by: Marco Elver <elver@...gle.com>

However, it is unclear to me under which circumstances we actually
need this, i.e. something would grab some memblock memory, somehow
poison it, and then release the memory back during early boot (note,
kfence_alloc_pool() is called before slab setup). If we can somehow
understand what actually did this, perhaps it'd help tell us if this
actually needs fixing in KFENCE or it's the other thing that needs a
fix.

Given all this is happening during really early boot, I'd expect no or
very few calls to kasan_poison() until kfence_alloc_pool() is called.
We can probably debug it more by having kasan_poison() do a "if
(!__kfence_pool) dump_stack();" somewhere. Can you try this on the
system where you can repro the problem? I tried this just now on the
latest mainline kernel, and saw 0 calls until kfence_alloc_pool().

Thanks,
-- Marco

> ---
>  mm/kfence/core.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/mm/kfence/core.c b/mm/kfence/core.c
> index d53c91f881a4..bb22b0cf77aa 100644
> --- a/mm/kfence/core.c
> +++ b/mm/kfence/core.c
> @@ -633,13 +633,19 @@ static DECLARE_DELAYED_WORK(kfence_timer, toggle_allocation_gate);
>
>  void __init kfence_alloc_pool(void)
>  {
> +       void *pool;
> +
>         if (!kfence_sample_interval)
>                 return;
>
> -       __kfence_pool = memblock_alloc(KFENCE_POOL_SIZE, PAGE_SIZE);
> -
> -       if (!__kfence_pool)
> +       pool = memblock_alloc(KFENCE_POOL_SIZE, PAGE_SIZE);
> +       if (!pool) {
>                 pr_err("failed to allocate pool\n");
> +               return;
> +       }
> +
> +       kasan_unpoison_range(pool, KFENCE_POOL_SIZE);
> +       __kfence_pool = pool;
>  }
>
>  void __init kfence_init(void)
> --
> 2.31.0.208.g409f899ff0-goog
>