Re: [PATCH RFC v3 22/36] kmsan: mm: call KMSAN hooks from SLUB code

[Alexander Potapenko <glider@google.com>]

On Mon, Dec 2, 2019 at 4:37 PM Marco Elver <elver@google.com> wrote:
>
> On Fri, 22 Nov 2019 at 12:27, <glider@google.com> wrote:
> >
> > In order to report uninitialized memory coming from heap allocations
> > KMSAN has to poison them unless they're created with __GFP_ZERO.
> >
> > It's handy that we need KMSAN hooks in the places where
> > init_on_alloc/init_on_free initialization is performed.
> >
> > Signed-off-by: Alexander Potapenko <glider@google.com>
> > To: Alexander Potapenko <glider@google.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Vegard Nossum <vegard.nossum@oracle.com>
> > Cc: Dmitry Vyukov <dvyukov@google.com>
> > Cc: linux-mm@kvack.org
> > ---
> > v3:
> >  - reverted unrelated whitespace changes
> >
> > Change-Id: I51103b7981d3aabed747d0c85cbdc85568665871
> > ---
> >  mm/slub.c | 34 +++++++++++++++++++++++++++++-----
> >  1 file changed, 29 insertions(+), 5 deletions(-)
> >
> > diff --git a/mm/slub.c b/mm/slub.c
> > index b25c807a111f..b5d2be1ac755 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -21,6 +21,8 @@
> >  #include <linux/proc_fs.h>
> >  #include <linux/seq_file.h>
> >  #include <linux/kasan.h>
> > +#include <linux/kmsan.h>
> > +#include <linux/kmsan-checks.h> /* KMSAN_INIT_VALUE */
> >  #include <linux/cpu.h>
> >  #include <linux/cpuset.h>
> >  #include <linux/mempolicy.h>
> > @@ -285,17 +287,27 @@ static void prefetch_freepointer(const struct kmem_cache *s, void *object)
> >         prefetch(object + s->offset);
> >  }
> >
> > +/*
> > + * When running under KMSAN, get_freepointer_safe() may return an uninitialized
> > + * pointer value in the case the current thread loses the race for the next
> > + * memory chunk in the freelist. In that case this_cpu_cmpxchg_double() in
> > + * slab_alloc_node() will fail, so the uninitialized value won't be used, but
> > + * KMSAN will still check all arguments of cmpxchg because of imperfect
> > + * handling of inline assembly.
> > + * To work around this problem, use KMSAN_INIT_VALUE() to force initialize the
> > + * return value of get_freepointer_safe().
> > + */
>
> Isn't this a general problem with cmpxchg? I.e. does other code using
> it have the same problem?
>
> Would it be better to just use KMSAN_INIT_VALUE in cmpxchg, rather
> than having the one-off workaround here?
I don't think so. It's normally still an error to pass an
uninitialized arg to cmpxchg, as in other cases it may be copied to
the result.
> Thanks,
> -- Marco



-- 
Alexander Potapenko
Software Engineer

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

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg