Re: [PATCH] kasan: fix kasan_check_read/write definitions

[Alexander Potapenko]

On Tue, Jan 8, 2019 at 5:51 AM Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
>
> On Tue, Jan 8, 2019 at 3:27 AM Nathan Chancellor
> <natechancellor@xxxxxxxxx> wrote:
> >
> > On Tue, Dec 11, 2018 at 02:34:35PM +0100, Arnd Bergmann wrote:
> > > Building little-endian allmodconfig kernels on arm64 started failing
> > > with the generated atomic.h implementation, since we now try to call
> > > kasan helpers from the EFI stub:
> > >
> > > aarch64-linux-gnu-ld: drivers/firmware/efi/libstub/arm-stub.stub.o: in function `atomic_set':
> > > include/generated/atomic-instrumented.h:44: undefined reference to `__efistub_kasan_check_write'
> > >
> > > I suspect that we get similar problems in other files that explicitly
> > > disable KASAN for some reason but call atomic_t based helper functions.
> > >
> > > We can fix this by checking the predefined __SANITIZE_ADDRESS__ macro
> > > that the compiler sets instead of checking CONFIG_KASAN, but this in turn
> > > requires a small hack in mm/kasan/common.c so we do see the extern
> > > declaration there instead of the inline function.
> > >
> > > Fixes: b1864b828644 ("locking/atomics: build atomic headers as required")
> > > Reported-by: Anders Roxell <anders.roxell@xxxxxxxxxx>
> > > Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
Reviewed-by: Alexander Potapenko <glider@xxxxxxxxxx>
> > > ---
> > >  include/linux/kasan-checks.h | 2 +-
> > >  mm/kasan/common.c            | 2 ++
> > >  2 files changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/kasan-checks.h b/include/linux/kasan-checks.h
> > > index d314150658a4..a61dc075e2ce 100644
> > > --- a/include/linux/kasan-checks.h
> > > +++ b/include/linux/kasan-checks.h
> > > @@ -2,7 +2,7 @@
> > >  #ifndef _LINUX_KASAN_CHECKS_H
> > >  #define _LINUX_KASAN_CHECKS_H
> > >
> > > -#ifdef CONFIG_KASAN
> > > +#if defined(__SANITIZE_ADDRESS__) || defined(__KASAN_INTERNAL)
> > >  void kasan_check_read(const volatile void *p, unsigned int size);
> > >  void kasan_check_write(const volatile void *p, unsigned int size);
> > >  #else
> > > diff --git a/mm/kasan/common.c b/mm/kasan/common.c
> > > index 03d5d1374ca7..51a7932c33a3 100644
> > > --- a/mm/kasan/common.c
> > > +++ b/mm/kasan/common.c
> > > @@ -14,6 +14,8 @@
> > >   *
> > >   */
> > >
> > > +#define __KASAN_INTERNAL
> > > +
> > >  #include <linux/export.h>
> > >  #include <linux/interrupt.h>
> > >  #include <linux/init.h>
> > > --
> > > 2.20.0
> > >
> >
> > Hi all,
> >
> > Was there any other movement on this patch? I am noticing this fail as
> > well and I have applied this patch in the meantime; it would be nice for
> > it to be merged so I could drop it from my stack.
>
> Alexander, ping, you wanted to double-check re KMSAN asm
> instrumentation and then decide on a common approach for KASAN and
> KMSAN.

I like Arnd's approach and will do the same for KMSAN.
Arnd, please go ahead submitting your patch.
The only possible issue I'm anticipating is that in the future we may
want to disable the checks in non-KASAN code (e.g. in arch/ or mm/),
so __KASAN_INTERNAL may not be the best name, but that's up to you.

-- 
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