linux-kernel - Re: [PATCH 1/2] lib/stackdepot: kmsan: mark API outputs as initialized

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]

Date:   Sat, 11 Mar 2023 00:50:09 +0100
From:   Andrey Konovalov <andreyknvl@...il.com>
To:     Alexander Potapenko <glider@...gle.com>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        akpm@...ux-foundation.org, elver@...gle.com, dvyukov@...gle.com,
        kasan-dev@...glegroups.com
Subject: Re: [PATCH 1/2] lib/stackdepot: kmsan: mark API outputs as initialized

On Mon, Mar 6, 2023 at 12:13 PM Alexander Potapenko <glider@...gle.com> wrote:
>
> KMSAN does not instrument stackdepot and may treat memory allocated by
> it as uninitialized. This is not a problem for KMSAN itself, because its
> functions calling stackdepot API are also not instrumented.
> But other kernel features (e.g. netdev tracker) may access stack depot
> from instrumented code, which will lead to false positives, unless we
> explicitly mark stackdepot outputs as initialized.
>
> Cc: Andrey Konovalov <andreyknvl@...il.com>
> Cc: Marco Elver <elver@...gle.com>
> Suggested-by: Dmitry Vyukov <dvyukov@...gle.com>
> Signed-off-by: Alexander Potapenko <glider@...gle.com>
> ---
>  lib/stackdepot.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> index 036da8e295d19..2f5aa851834eb 100644
> --- a/lib/stackdepot.c
> +++ b/lib/stackdepot.c
> @@ -17,6 +17,7 @@
>  #include <linux/gfp.h>
>  #include <linux/jhash.h>
>  #include <linux/kernel.h>
> +#include <linux/kmsan.h>
>  #include <linux/mm.h>
>  #include <linux/mutex.h>
>  #include <linux/percpu.h>
> @@ -306,6 +307,11 @@ depot_alloc_stack(unsigned long *entries, int size, u32 hash, void **prealloc)
>         stack->handle.extra = 0;
>         memcpy(stack->entries, entries, flex_array_size(stack, entries, size));
>         pool_offset += required_size;
> +       /*
> +        * Let KMSAN know the stored stack record is initialized. This shall
> +        * prevent false positive reports if instrumented code accesses it.
> +        */
> +       kmsan_unpoison_memory(stack, required_size);
>
>         return stack;
>  }
> @@ -465,6 +471,12 @@ unsigned int stack_depot_fetch(depot_stack_handle_t handle,
>         struct stack_record *stack;
>
>         *entries = NULL;
> +       /*
> +        * Let KMSAN know *entries is initialized. This shall prevent false
> +        * positive reports if instrumented code accesses it.
> +        */
> +       kmsan_unpoison_memory(entries, sizeof(*entries));
> +
>         if (!handle)
>                 return 0;
>
> --
> 2.40.0.rc0.216.gc4246ad0f0-goog
>

Reviewed-by: Andrey Konovalov <andreyknvl@...il.com>