Re: [PATCH] x86: suppress KMSAN reports in arch_within_stack_frames()

[Alexander Potapenko]

On Mon, Nov 21, 2022 at 12:38 PM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> On Mon, Nov 21, 2022 at 11:28:39AM +0100, Alexander Potapenko wrote:
>
> > > > +__no_kmsan_checks
> > > >  static inline int arch_within_stack_frames(const void * const stack,
> > > >                                          const void * const stackend,
> > > >                                          const void *obj, unsigned long len)
> > >
> > > Seems OK; but now I'm confused as to the exact distinction between
> > > __no_sanitize_memory and __no_kmsan_checks.
> > >
> > > The comments there about seem to suggest __no_sanitize_memory ensures no
> > > instrumentation at all, and __no_kmsan_checks some instrumentation but
> > > doesn't actually check anything -- so what's left then?
> >
> > __no_sanitize_memory prohibits all instrumentation whatsoever, whereas
> > __no_kmsan_checks adds instrumentation that suppresses potential false
> > positives around this function.
> >
> > Quoting include/linux/compiler-clang.h:
> >
> > /*
> >  * The __no_kmsan_checks attribute ensures that a function does not produce
> >  * false positive reports by:
> >  *  - initializing all local variables and memory stores in this function;
> >  *  - skipping all shadow checks;
> >  *  - passing initialized arguments to this function's callees.
> >  */
> >
> > Does this answer your question?
>
> So I read that comment; and it didn't click. So you're explicitly
> initializing variables/arguments and explicitly not checking shadow
> state vs, not doing explicit initialization and checking shadow state?
>
> That is, it doesn't do the normal checks and adds explicit
> initialization to avoid triggering discontent in surrounding functions?
>
Correct

In other words, for normal instrumentation:
 - locals are explicitly marked as uninitialized;
 - shadow values are calculated for arithmetic operations based on their inputs;
 - shadow values are checked for branches, pointer dereferences, and
before passing them as function arguments;
 - memory stores update shadow for affected variables.

For __no_kmsan_checks:
 - locals are explicitly marked as initialized;
 - no instrumentation is added for arithmetic operations, branches,
pointer dereferences;
 - all function arguments are marked as initialized;
 - stores always mark memory as initialized.

For __no_sanitize_memory:
 - no instrumentation for locals (they may end up being initialized or
uninitialized - doesn't matter, because their shadow values are never
used);
 - no instrumentation for arithmetic operations, branches, pointer dereferences;
 - no instrumentation for function calls (an instrumented function
will receive garbage shadow values from a non-instrumented one);
 - no instrumentation for stores (initialization done in these
functions is invisible).

In all the cases explicit calls to
kmsan_poison_memory()/kmsan_unpoison_memory()/kmsan_check_memory()
behave the same way and can be used to tell the tool what is going on
in the absence of instrumentation.

-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg