LKML Archive on lore.kernel.org
 help / Atom feed
* [PATCH] kasan: fix kasan_check_read/write definitions
@ 2018-12-11 13:34 Arnd Bergmann
  2018-12-11 13:44 ` Dmitry Vyukov
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Arnd Bergmann @ 2018-12-11 13:34 UTC (permalink / raw)
  To: Andrew Morton, Andrey Ryabinin
  Cc: Anders Roxell, Ard Biesheuvel, Will Deacon, Mark Rutland,
	Arnd Bergmann, Alexander Potapenko, Dmitry Vyukov,
	Andrey Konovalov, Stephen Rothwell, kasan-dev, linux-kernel,
	linux-mm

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@linaro.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] kasan: fix kasan_check_read/write definitions
  2018-12-11 13:34 [PATCH] kasan: fix kasan_check_read/write definitions Arnd Bergmann
@ 2018-12-11 13:44 ` Dmitry Vyukov
  2018-12-11 22:25   ` Alexander Potapenko
  2019-01-08  2:26 ` Nathan Chancellor
  2019-01-11 18:46 ` Andrey Ryabinin
  2 siblings, 1 reply; 8+ messages in thread
From: Dmitry Vyukov @ 2018-12-11 13:44 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrew Morton, Andrey Ryabinin, anders.roxell, Ard Biesheuvel,
	Will Deacon, Mark Rutland, Alexander Potapenko, Andrey Konovalov,
	Stephen Rothwell, kasan-dev, LKML, Linux-MM

On Tue, Dec 11, 2018 at 2:35 PM Arnd Bergmann <arnd@arndb.de> 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.


Alexander, I think you are doing a similar thing for similar reasons
in KMSAN patch (see KMSAN_CHECK_ATOMIC_PARAMS):
https://github.com/google/kmsan/commit/17ebbfe19624c84adf79b0e5a74fd258c49ff12b
Namely, non-KMSAN-instrumented files must not get KMSAN callbacks from
atomics too.

Arnd patch does it the other way around: non-instrumented files need
to opt-in instead of opt-out.
Let's settle on a common way to do this, so that we can use it
consistently across all tools.



> Fixes: b1864b828644 ("locking/atomics: build atomic headers as required")
> Reported-by: Anders Roxell <anders.roxell@linaro.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  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
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] kasan: fix kasan_check_read/write definitions
  2018-12-11 13:44 ` Dmitry Vyukov
@ 2018-12-11 22:25   ` Alexander Potapenko
  2018-12-12 10:00     ` Dmitry Vyukov
  0 siblings, 1 reply; 8+ messages in thread
From: Alexander Potapenko @ 2018-12-11 22:25 UTC (permalink / raw)
  To: Dmitriy Vyukov
  Cc: Arnd Bergmann, Andrew Morton, Andrey Ryabinin, anders.roxell,
	Ard Biesheuvel, Will Deacon, Mark Rutland, Andrey Konovalov,
	Stephen Rothwell, kasan-dev, LKML, Linux Memory Management List

On Tue, Dec 11, 2018 at 2:45 PM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Tue, Dec 11, 2018 at 2:35 PM Arnd Bergmann <arnd@arndb.de> 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.
>
>
> Alexander, I think you are doing a similar thing for similar reasons
> in KMSAN patch (see KMSAN_CHECK_ATOMIC_PARAMS):
> https://github.com/google/kmsan/commit/17ebbfe19624c84adf79b0e5a74fd258c49ff12b
> Namely, non-KMSAN-instrumented files must not get KMSAN callbacks from
> atomics too.
I'll need to double-check, but it occurs to me that we won't need
additional hooks for atomics in KMSAN - the compiler instrumentation
should suffice.

> Arnd patch does it the other way around: non-instrumented files need
> to opt-in instead of opt-out.
Shouldn't we put __SANITIZE_ADDRESS__ somewhere into mm/kasan/kasan.h then?
> Let's settle on a common way to do this, so that we can use it
> consistently across all tools.
>
>
>
> > Fixes: b1864b828644 ("locking/atomics: build atomic headers as required")
> > Reported-by: Anders Roxell <anders.roxell@linaro.org>
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  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
> >



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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] kasan: fix kasan_check_read/write definitions
  2018-12-11 22:25   ` Alexander Potapenko
@ 2018-12-12 10:00     ` Dmitry Vyukov
  0 siblings, 0 replies; 8+ messages in thread
From: Dmitry Vyukov @ 2018-12-12 10:00 UTC (permalink / raw)
  To: Alexander Potapenko
  Cc: Arnd Bergmann, Andrew Morton, Andrey Ryabinin, anders.roxell,
	Ard Biesheuvel, Will Deacon, Mark Rutland, Andrey Konovalov,
	Stephen Rothwell, kasan-dev, LKML, Linux-MM

On Tue, Dec 11, 2018 at 11:25 PM Alexander Potapenko <glider@google.com> 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.
> >
> >
> > Alexander, I think you are doing a similar thing for similar reasons
> > in KMSAN patch (see KMSAN_CHECK_ATOMIC_PARAMS):
> > https://github.com/google/kmsan/commit/17ebbfe19624c84adf79b0e5a74fd258c49ff12b
> > Namely, non-KMSAN-instrumented files must not get KMSAN callbacks from
> > atomics too.
> I'll need to double-check, but it occurs to me that we won't need
> additional hooks for atomics in KMSAN - the compiler instrumentation
> should suffice.

Compiler asm instrumentation will only insert conservative
initialization, but not checks of arguments, right?
I mean these checks are optional in the sense that it's only false
negatives, but since we already have them and they don't seem to lead
to false positives, why do we want to remove them?


> > Arnd patch does it the other way around: non-instrumented files need
> > to opt-in instead of opt-out.
> Shouldn't we put __SANITIZE_ADDRESS__ somewhere into mm/kasan/kasan.h then?
> > Let's settle on a common way to do this, so that we can use it
> > consistently across all tools.
> >
> >
> >
> > > Fixes: b1864b828644 ("locking/atomics: build atomic headers as required")
> > > Reported-by: Anders Roxell <anders.roxell@linaro.org>
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > >  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
> > >
>
>
>
> --
> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] kasan: fix kasan_check_read/write definitions
  2018-12-11 13:34 [PATCH] kasan: fix kasan_check_read/write definitions Arnd Bergmann
  2018-12-11 13:44 ` Dmitry Vyukov
@ 2019-01-08  2:26 ` Nathan Chancellor
  2019-01-08  4:51   ` Dmitry Vyukov
  2019-01-11 18:46 ` Andrey Ryabinin
  2 siblings, 1 reply; 8+ messages in thread
From: Nathan Chancellor @ 2019-01-08  2:26 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrew Morton, Andrey Ryabinin, Anders Roxell, Ard Biesheuvel,
	Will Deacon, Mark Rutland, Alexander Potapenko, Dmitry Vyukov,
	Andrey Konovalov, Stephen Rothwell, kasan-dev, linux-kernel,
	linux-mm

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@linaro.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  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.

Thanks,
Nathan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] kasan: fix kasan_check_read/write definitions
  2019-01-08  2:26 ` Nathan Chancellor
@ 2019-01-08  4:51   ` Dmitry Vyukov
  2019-01-08  9:48     ` Alexander Potapenko
  0 siblings, 1 reply; 8+ messages in thread
From: Dmitry Vyukov @ 2019-01-08  4:51 UTC (permalink / raw)
  To: Alexander Potapenko
  Cc: Arnd Bergmann, Andrew Morton, Andrey Ryabinin, Anders Roxell,
	Ard Biesheuvel, Will Deacon, Mark Rutland, Andrey Konovalov,
	Stephen Rothwell, kasan-dev, LKML, Linux-MM, Nathan Chancellor

On Tue, Jan 8, 2019 at 3:27 AM Nathan Chancellor
<natechancellor@gmail.com> 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@linaro.org>
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] kasan: fix kasan_check_read/write definitions
  2019-01-08  4:51   ` Dmitry Vyukov
@ 2019-01-08  9:48     ` Alexander Potapenko
  0 siblings, 0 replies; 8+ messages in thread
From: Alexander Potapenko @ 2019-01-08  9:48 UTC (permalink / raw)
  To: Dmitry Vyukov, Arnd Bergmann
  Cc: Andrew Morton, Andrey Ryabinin, Anders Roxell, Ard Biesheuvel,
	Will Deacon, Mark Rutland, Andrey Konovalov, Stephen Rothwell,
	kasan-dev, LKML, Linux-MM, Nathan Chancellor

On Tue, Jan 8, 2019 at 5:51 AM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Tue, Jan 8, 2019 at 3:27 AM Nathan Chancellor
> <natechancellor@gmail.com> 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@linaro.org>
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Alexander Potapenko <glider@google.com>
> > > ---
> > >  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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] kasan: fix kasan_check_read/write definitions
  2018-12-11 13:34 [PATCH] kasan: fix kasan_check_read/write definitions Arnd Bergmann
  2018-12-11 13:44 ` Dmitry Vyukov
  2019-01-08  2:26 ` Nathan Chancellor
@ 2019-01-11 18:46 ` Andrey Ryabinin
  2 siblings, 0 replies; 8+ messages in thread
From: Andrey Ryabinin @ 2019-01-11 18:46 UTC (permalink / raw)
  To: Arnd Bergmann, Andrew Morton
  Cc: Anders Roxell, Ard Biesheuvel, Will Deacon, Mark Rutland,
	Alexander Potapenko, Dmitry Vyukov, Andrey Konovalov,
	Stephen Rothwell, kasan-dev, linux-kernel, linux-mm

On 12/11/18 4:34 PM, 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@linaro.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---

Acked-by: Andrey Ryabinin <aryabinin@virtuozzo.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-11 13:34 [PATCH] kasan: fix kasan_check_read/write definitions Arnd Bergmann
2018-12-11 13:44 ` Dmitry Vyukov
2018-12-11 22:25   ` Alexander Potapenko
2018-12-12 10:00     ` Dmitry Vyukov
2019-01-08  2:26 ` Nathan Chancellor
2019-01-08  4:51   ` Dmitry Vyukov
2019-01-08  9:48     ` Alexander Potapenko
2019-01-11 18:46 ` Andrey Ryabinin

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox