MIME-Version: 1.0
References: <20230306111322.205724-1-glider@google.com>
In-Reply-To: <20230306111322.205724-1-glider@google.com>
From:   Andrey Konovalov <andreyknvl@gmail.com>
Date:   Sat, 11 Mar 2023 00:50:09 +0100
Message-ID: <CA+fCnZfENShgduZuu1xzrmCnNFv+ovHtcavGXKjYumsGA1kX5w@mail.gmail.com>
Subject: Re: [PATCH 1/2] lib/stackdepot: kmsan: mark API outputs as initialized
To:     Alexander Potapenko <glider@google.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        akpm@linux-foundation.org, elver@google.com, dvyukov@google.com,
        kasan-dev@googlegroups.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Mar 6, 2023 at 12:13=E2=80=AFPM Alexander Potapenko <glider@google.=
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@gmail.com>
> Cc: Marco Elver <elver@google.com>
> Suggested-by: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Alexander Potapenko <glider@google.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 =3D 0;
>         memcpy(stack->entries, entries, flex_array_size(stack, entries, s=
ize));
>         pool_offset +=3D required_size;
> +       /*
> +        * Let KMSAN know the stored stack record is initialized. This sh=
all
> +        * prevent false positive reports if instrumented code accesses i=
t.
> +        */
> +       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 =3D NULL;
> +       /*
> +        * Let KMSAN know *entries is initialized. This shall prevent fal=
se
> +        * 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@gmail.com>