On Mon, Mar 6, 2023 at 12:13 PM Alexander Potapenko <glider@xxxxxxxxxx> 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@xxxxxxxxx> > Cc: Marco Elver <elver@xxxxxxxxxx> > Suggested-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx> > Signed-off-by: Alexander Potapenko <glider@xxxxxxxxxx> > --- > 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@xxxxxxxxx>
