All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kasan: Always respect CONFIG_KASAN_STACK
@ 2021-09-22 20:55 Nathan Chancellor
  2021-09-23 10:07   ` Marco Elver
  2021-10-03 18:04   ` Andrey Konovalov
  0 siblings, 2 replies; 26+ messages in thread
From: Nathan Chancellor @ 2021-09-22 20:55 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov,
	Nick Desaulniers, Arnd Bergmann, kasan-dev, linux-kernel, llvm,
	Nathan Chancellor

Currently, the asan-stack parameter is only passed along if
CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
be defined in Kconfig so that the value can be checked. In RISC-V's
case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
is disabled, resulting in large stack warnings with allmodconfig:

drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
error: stack frame size (14400) exceeds limit (2048) in function
'lb035q02_connect' [-Werror,-Wframe-larger-than]
static int lb035q02_connect(struct omap_dss_device *dssdev)
           ^
1 error generated.

Ensure that the value of CONFIG_KASAN_STACK is always passed along to
the compiler so that these warnings do not happen when
CONFIG_KASAN_STACK is disabled.

Link: https://github.com/ClangBuiltLinux/linux/issues/1453
References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 scripts/Makefile.kasan | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
index 801c415bac59..b9e94c5e7097 100644
--- a/scripts/Makefile.kasan
+++ b/scripts/Makefile.kasan
@@ -33,10 +33,11 @@ else
 	CFLAGS_KASAN := $(CFLAGS_KASAN_SHADOW) \
 	 $(call cc-param,asan-globals=1) \
 	 $(call cc-param,asan-instrumentation-with-call-threshold=$(call_threshold)) \
-	 $(call cc-param,asan-stack=$(stack_enable)) \
 	 $(call cc-param,asan-instrument-allocas=1)
 endif
 
+CFLAGS_KASAN += $(call cc-param,asan-stack=$(stack_enable))
+
 endif # CONFIG_KASAN_GENERIC
 
 ifdef CONFIG_KASAN_SW_TAGS

base-commit: 4057525736b159bd456732d11270af2cc49ec21f
-- 
2.33.0.514.g99c99ed825


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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
  2021-09-22 20:55 [PATCH] kasan: Always respect CONFIG_KASAN_STACK Nathan Chancellor
  2021-09-23 10:07   ` Marco Elver
@ 2021-09-23 10:07   ` Marco Elver
  1 sibling, 0 replies; 26+ messages in thread
From: Marco Elver @ 2021-09-23 10:07 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov,
	Dmitry Vyukov, Nick Desaulniers, Arnd Bergmann, kasan-dev,
	linux-kernel, llvm, linux-riscv, Paul Walmsley, Palmer Dabbelt,
	Albert Ou

On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor <nathan@kernel.org> wrote:
> Currently, the asan-stack parameter is only passed along if
> CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
> be defined in Kconfig so that the value can be checked. In RISC-V's
> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
> asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
> is disabled, resulting in large stack warnings with allmodconfig:
>
> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> error: stack frame size (14400) exceeds limit (2048) in function
> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> static int lb035q02_connect(struct omap_dss_device *dssdev)
>            ^
> 1 error generated.
>
> Ensure that the value of CONFIG_KASAN_STACK is always passed along to
> the compiler so that these warnings do not happen when
> CONFIG_KASAN_STACK is disabled.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Reviewed-by: Marco Elver <elver@google.com>

[ Which tree are you planning to take it through? ]

Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
comment (copied from arm64). Did RISC-V just forget to copy over the
Kconfig option?


> ---
>  scripts/Makefile.kasan | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> index 801c415bac59..b9e94c5e7097 100644
> --- a/scripts/Makefile.kasan
> +++ b/scripts/Makefile.kasan
> @@ -33,10 +33,11 @@ else
>         CFLAGS_KASAN := $(CFLAGS_KASAN_SHADOW) \
>          $(call cc-param,asan-globals=1) \
>          $(call cc-param,asan-instrumentation-with-call-threshold=$(call_threshold)) \
> -        $(call cc-param,asan-stack=$(stack_enable)) \
>          $(call cc-param,asan-instrument-allocas=1)
>  endif
>
> +CFLAGS_KASAN += $(call cc-param,asan-stack=$(stack_enable))
> +
>  endif # CONFIG_KASAN_GENERIC
>
>  ifdef CONFIG_KASAN_SW_TAGS
>
> base-commit: 4057525736b159bd456732d11270af2cc49ec21f
> --
> 2.33.0.514.g99c99ed825
>
>

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
@ 2021-09-23 10:07   ` Marco Elver
  0 siblings, 0 replies; 26+ messages in thread
From: Marco Elver @ 2021-09-23 10:07 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov,
	Dmitry Vyukov, Nick Desaulniers, Arnd Bergmann, kasan-dev,
	linux-kernel, llvm, linux-riscv, Paul Walmsley, Palmer Dabbelt,
	Albert Ou

On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor <nathan@kernel.org> wrote:
> Currently, the asan-stack parameter is only passed along if
> CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
> be defined in Kconfig so that the value can be checked. In RISC-V's
> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
> asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
> is disabled, resulting in large stack warnings with allmodconfig:
>
> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> error: stack frame size (14400) exceeds limit (2048) in function
> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> static int lb035q02_connect(struct omap_dss_device *dssdev)
>            ^
> 1 error generated.
>
> Ensure that the value of CONFIG_KASAN_STACK is always passed along to
> the compiler so that these warnings do not happen when
> CONFIG_KASAN_STACK is disabled.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Reviewed-by: Marco Elver <elver@google.com>

[ Which tree are you planning to take it through? ]

Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
comment (copied from arm64). Did RISC-V just forget to copy over the
Kconfig option?


> ---
>  scripts/Makefile.kasan | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> index 801c415bac59..b9e94c5e7097 100644
> --- a/scripts/Makefile.kasan
> +++ b/scripts/Makefile.kasan
> @@ -33,10 +33,11 @@ else
>         CFLAGS_KASAN := $(CFLAGS_KASAN_SHADOW) \
>          $(call cc-param,asan-globals=1) \
>          $(call cc-param,asan-instrumentation-with-call-threshold=$(call_threshold)) \
> -        $(call cc-param,asan-stack=$(stack_enable)) \
>          $(call cc-param,asan-instrument-allocas=1)
>  endif
>
> +CFLAGS_KASAN += $(call cc-param,asan-stack=$(stack_enable))
> +
>  endif # CONFIG_KASAN_GENERIC
>
>  ifdef CONFIG_KASAN_SW_TAGS
>
> base-commit: 4057525736b159bd456732d11270af2cc49ec21f
> --
> 2.33.0.514.g99c99ed825
>
>

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
@ 2021-09-23 10:07   ` Marco Elver
  0 siblings, 0 replies; 26+ messages in thread
From: Marco Elver @ 2021-09-23 10:07 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov,
	Dmitry Vyukov, Nick Desaulniers, Arnd Bergmann, kasan-dev,
	linux-kernel, llvm, linux-riscv, Paul Walmsley, Palmer Dabbelt,
	Albert Ou

On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor <nathan@kernel.org> wrote:
> Currently, the asan-stack parameter is only passed along if
> CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
> be defined in Kconfig so that the value can be checked. In RISC-V's
> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
> asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
> is disabled, resulting in large stack warnings with allmodconfig:
>
> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> error: stack frame size (14400) exceeds limit (2048) in function
> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> static int lb035q02_connect(struct omap_dss_device *dssdev)
>            ^
> 1 error generated.
>
> Ensure that the value of CONFIG_KASAN_STACK is always passed along to
> the compiler so that these warnings do not happen when
> CONFIG_KASAN_STACK is disabled.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Reviewed-by: Marco Elver <elver@google.com>

[ Which tree are you planning to take it through? ]

Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
comment (copied from arm64). Did RISC-V just forget to copy over the
Kconfig option?


> ---
>  scripts/Makefile.kasan | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> index 801c415bac59..b9e94c5e7097 100644
> --- a/scripts/Makefile.kasan
> +++ b/scripts/Makefile.kasan
> @@ -33,10 +33,11 @@ else
>         CFLAGS_KASAN := $(CFLAGS_KASAN_SHADOW) \
>          $(call cc-param,asan-globals=1) \
>          $(call cc-param,asan-instrumentation-with-call-threshold=$(call_threshold)) \
> -        $(call cc-param,asan-stack=$(stack_enable)) \
>          $(call cc-param,asan-instrument-allocas=1)
>  endif
>
> +CFLAGS_KASAN += $(call cc-param,asan-stack=$(stack_enable))
> +
>  endif # CONFIG_KASAN_GENERIC
>
>  ifdef CONFIG_KASAN_SW_TAGS
>
> base-commit: 4057525736b159bd456732d11270af2cc49ec21f
> --
> 2.33.0.514.g99c99ed825
>
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
  2021-09-23 10:07   ` Marco Elver
@ 2021-09-23 14:59     ` Nathan Chancellor
  -1 siblings, 0 replies; 26+ messages in thread
From: Nathan Chancellor @ 2021-09-23 14:59 UTC (permalink / raw)
  To: Marco Elver, Andrew Morton
  Cc: Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov,
	Dmitry Vyukov, Nick Desaulniers, Arnd Bergmann, kasan-dev,
	linux-kernel, llvm, linux-riscv, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, linux-mm

On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor <nathan@kernel.org> wrote:
> > Currently, the asan-stack parameter is only passed along if
> > CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
> > be defined in Kconfig so that the value can be checked. In RISC-V's
> > case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
> > asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
> > is disabled, resulting in large stack warnings with allmodconfig:
> >
> > drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> > error: stack frame size (14400) exceeds limit (2048) in function
> > 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> > static int lb035q02_connect(struct omap_dss_device *dssdev)
> >            ^
> > 1 error generated.
> >
> > Ensure that the value of CONFIG_KASAN_STACK is always passed along to
> > the compiler so that these warnings do not happen when
> > CONFIG_KASAN_STACK is disabled.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> > References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> 
> Reviewed-by: Marco Elver <elver@google.com>

Thanks!

> [ Which tree are you planning to take it through? ]

Gah, I was intending for it to go through -mm, then I cc'd neither
Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
grab it from LKML?

> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
> comment (copied from arm64). Did RISC-V just forget to copy over the
> Kconfig option?

I do see it defined in that file as well but you are right that they did
not copy the Kconfig logic, even though it was present in the tree when
RISC-V KASAN was implemented. Perhaps they should so that they get
access to the other flags in the "else" branch?

> > ---
> >  scripts/Makefile.kasan | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> > index 801c415bac59..b9e94c5e7097 100644
> > --- a/scripts/Makefile.kasan
> > +++ b/scripts/Makefile.kasan
> > @@ -33,10 +33,11 @@ else
> >         CFLAGS_KASAN := $(CFLAGS_KASAN_SHADOW) \
> >          $(call cc-param,asan-globals=1) \
> >          $(call cc-param,asan-instrumentation-with-call-threshold=$(call_threshold)) \
> > -        $(call cc-param,asan-stack=$(stack_enable)) \
> >          $(call cc-param,asan-instrument-allocas=1)
> >  endif
> >
> > +CFLAGS_KASAN += $(call cc-param,asan-stack=$(stack_enable))
> > +
> >  endif # CONFIG_KASAN_GENERIC
> >
> >  ifdef CONFIG_KASAN_SW_TAGS
> >
> > base-commit: 4057525736b159bd456732d11270af2cc49ec21f
> > --
> > 2.33.0.514.g99c99ed825
> >
> >

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
@ 2021-09-23 14:59     ` Nathan Chancellor
  0 siblings, 0 replies; 26+ messages in thread
From: Nathan Chancellor @ 2021-09-23 14:59 UTC (permalink / raw)
  To: Marco Elver, Andrew Morton
  Cc: Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov,
	Dmitry Vyukov, Nick Desaulniers, Arnd Bergmann, kasan-dev,
	linux-kernel, llvm, linux-riscv, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, linux-mm

On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor <nathan@kernel.org> wrote:
> > Currently, the asan-stack parameter is only passed along if
> > CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
> > be defined in Kconfig so that the value can be checked. In RISC-V's
> > case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
> > asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
> > is disabled, resulting in large stack warnings with allmodconfig:
> >
> > drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> > error: stack frame size (14400) exceeds limit (2048) in function
> > 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> > static int lb035q02_connect(struct omap_dss_device *dssdev)
> >            ^
> > 1 error generated.
> >
> > Ensure that the value of CONFIG_KASAN_STACK is always passed along to
> > the compiler so that these warnings do not happen when
> > CONFIG_KASAN_STACK is disabled.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> > References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> 
> Reviewed-by: Marco Elver <elver@google.com>

Thanks!

> [ Which tree are you planning to take it through? ]

Gah, I was intending for it to go through -mm, then I cc'd neither
Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
grab it from LKML?

> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
> comment (copied from arm64). Did RISC-V just forget to copy over the
> Kconfig option?

I do see it defined in that file as well but you are right that they did
not copy the Kconfig logic, even though it was present in the tree when
RISC-V KASAN was implemented. Perhaps they should so that they get
access to the other flags in the "else" branch?

> > ---
> >  scripts/Makefile.kasan | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> > index 801c415bac59..b9e94c5e7097 100644
> > --- a/scripts/Makefile.kasan
> > +++ b/scripts/Makefile.kasan
> > @@ -33,10 +33,11 @@ else
> >         CFLAGS_KASAN := $(CFLAGS_KASAN_SHADOW) \
> >          $(call cc-param,asan-globals=1) \
> >          $(call cc-param,asan-instrumentation-with-call-threshold=$(call_threshold)) \
> > -        $(call cc-param,asan-stack=$(stack_enable)) \
> >          $(call cc-param,asan-instrument-allocas=1)
> >  endif
> >
> > +CFLAGS_KASAN += $(call cc-param,asan-stack=$(stack_enable))
> > +
> >  endif # CONFIG_KASAN_GENERIC
> >
> >  ifdef CONFIG_KASAN_SW_TAGS
> >
> > base-commit: 4057525736b159bd456732d11270af2cc49ec21f
> > --
> > 2.33.0.514.g99c99ed825
> >
> >

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
  2021-09-22 20:55 [PATCH] kasan: Always respect CONFIG_KASAN_STACK Nathan Chancellor
@ 2021-10-03 18:04   ` Andrey Konovalov
  2021-10-03 18:04   ` Andrey Konovalov
  1 sibling, 0 replies; 26+ messages in thread
From: Andrey Konovalov @ 2021-10-03 18:04 UTC (permalink / raw)
  To: Nathan Chancellor, Andrey Ryabinin, Marco Elver
  Cc: Alexander Potapenko, Dmitry Vyukov, Nick Desaulniers,
	Arnd Bergmann, kasan-dev, LKML, llvm

On Wed, Sep 22, 2021 at 10:55 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> Currently, the asan-stack parameter is only passed along if
> CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
> be defined in Kconfig so that the value can be checked. In RISC-V's
> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
> asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
> is disabled, resulting in large stack warnings with allmodconfig:
>
> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> error: stack frame size (14400) exceeds limit (2048) in function
> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> static int lb035q02_connect(struct omap_dss_device *dssdev)
>            ^
> 1 error generated.
>
> Ensure that the value of CONFIG_KASAN_STACK is always passed along to
> the compiler so that these warnings do not happen when
> CONFIG_KASAN_STACK is disabled.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  scripts/Makefile.kasan | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> index 801c415bac59..b9e94c5e7097 100644
> --- a/scripts/Makefile.kasan
> +++ b/scripts/Makefile.kasan
> @@ -33,10 +33,11 @@ else
>         CFLAGS_KASAN := $(CFLAGS_KASAN_SHADOW) \
>          $(call cc-param,asan-globals=1) \
>          $(call cc-param,asan-instrumentation-with-call-threshold=$(call_threshold)) \
> -        $(call cc-param,asan-stack=$(stack_enable)) \
>          $(call cc-param,asan-instrument-allocas=1)
>  endif

This part of code always looked weird to me.

Shouldn't we be able to pull all these options out of the else section?

Then, the code structure would make sense: first, try applying
KASAN_SHADOW_OFFSET; if failed, use CFLAGS_KASAN_MINIMAL; and then try
applying all these options one by one.

> +CFLAGS_KASAN += $(call cc-param,asan-stack=$(stack_enable))
> +
>  endif # CONFIG_KASAN_GENERIC
>
>  ifdef CONFIG_KASAN_SW_TAGS
>
> base-commit: 4057525736b159bd456732d11270af2cc49ec21f
> --
> 2.33.0.514.g99c99ed825
>

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
@ 2021-10-03 18:04   ` Andrey Konovalov
  0 siblings, 0 replies; 26+ messages in thread
From: Andrey Konovalov @ 2021-10-03 18:04 UTC (permalink / raw)
  To: Nathan Chancellor, Andrey Ryabinin, Marco Elver
  Cc: Alexander Potapenko, Dmitry Vyukov, Nick Desaulniers,
	Arnd Bergmann, kasan-dev, LKML, llvm

On Wed, Sep 22, 2021 at 10:55 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> Currently, the asan-stack parameter is only passed along if
> CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
> be defined in Kconfig so that the value can be checked. In RISC-V's
> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
> asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
> is disabled, resulting in large stack warnings with allmodconfig:
>
> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> error: stack frame size (14400) exceeds limit (2048) in function
> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> static int lb035q02_connect(struct omap_dss_device *dssdev)
>            ^
> 1 error generated.
>
> Ensure that the value of CONFIG_KASAN_STACK is always passed along to
> the compiler so that these warnings do not happen when
> CONFIG_KASAN_STACK is disabled.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  scripts/Makefile.kasan | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> index 801c415bac59..b9e94c5e7097 100644
> --- a/scripts/Makefile.kasan
> +++ b/scripts/Makefile.kasan
> @@ -33,10 +33,11 @@ else
>         CFLAGS_KASAN := $(CFLAGS_KASAN_SHADOW) \
>          $(call cc-param,asan-globals=1) \
>          $(call cc-param,asan-instrumentation-with-call-threshold=$(call_threshold)) \
> -        $(call cc-param,asan-stack=$(stack_enable)) \
>          $(call cc-param,asan-instrument-allocas=1)
>  endif

This part of code always looked weird to me.

Shouldn't we be able to pull all these options out of the else section?

Then, the code structure would make sense: first, try applying
KASAN_SHADOW_OFFSET; if failed, use CFLAGS_KASAN_MINIMAL; and then try
applying all these options one by one.

> +CFLAGS_KASAN += $(call cc-param,asan-stack=$(stack_enable))
> +
>  endif # CONFIG_KASAN_GENERIC
>
>  ifdef CONFIG_KASAN_SW_TAGS
>
> base-commit: 4057525736b159bd456732d11270af2cc49ec21f
> --
> 2.33.0.514.g99c99ed825
>

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
  2021-10-03 18:04   ` Andrey Konovalov
  (?)
@ 2021-10-06  2:43   ` Nathan Chancellor
  2021-10-06 11:57     ` Andrey Konovalov
  -1 siblings, 1 reply; 26+ messages in thread
From: Nathan Chancellor @ 2021-10-06  2:43 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Andrey Ryabinin, Marco Elver, Alexander Potapenko, Dmitry Vyukov,
	Nick Desaulniers, Arnd Bergmann, kasan-dev, LKML, llvm

On Sun, Oct 03, 2021 at 08:04:53PM +0200, Andrey Konovalov wrote:
> On Wed, Sep 22, 2021 at 10:55 PM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > Currently, the asan-stack parameter is only passed along if
> > CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
> > be defined in Kconfig so that the value can be checked. In RISC-V's
> > case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
> > asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
> > is disabled, resulting in large stack warnings with allmodconfig:
> >
> > drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> > error: stack frame size (14400) exceeds limit (2048) in function
> > 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> > static int lb035q02_connect(struct omap_dss_device *dssdev)
> >            ^
> > 1 error generated.
> >
> > Ensure that the value of CONFIG_KASAN_STACK is always passed along to
> > the compiler so that these warnings do not happen when
> > CONFIG_KASAN_STACK is disabled.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> > References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> >  scripts/Makefile.kasan | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> > index 801c415bac59..b9e94c5e7097 100644
> > --- a/scripts/Makefile.kasan
> > +++ b/scripts/Makefile.kasan
> > @@ -33,10 +33,11 @@ else
> >         CFLAGS_KASAN := $(CFLAGS_KASAN_SHADOW) \
> >          $(call cc-param,asan-globals=1) \
> >          $(call cc-param,asan-instrumentation-with-call-threshold=$(call_threshold)) \
> > -        $(call cc-param,asan-stack=$(stack_enable)) \
> >          $(call cc-param,asan-instrument-allocas=1)
> >  endif
> 
> This part of code always looked weird to me.
> 
> Shouldn't we be able to pull all these options out of the else section?
> 
> Then, the code structure would make sense: first, try applying
> KASAN_SHADOW_OFFSET; if failed, use CFLAGS_KASAN_MINIMAL; and then try
> applying all these options one by one.

Prior to commit 1a69e7ce8391 ("kasan/Makefile: support LLVM style asan
parameters"), all the flags were run under one cc-option, meaning that
if $(KASAN_SHADOW_OFFSET) was not set, the whole call would fail.
However, after that commit, it is possible to do this but I was not sure
if that was intentional so I went for the minimal fix.

Cheers,
Nathan

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
  2021-10-06  2:43   ` Nathan Chancellor
@ 2021-10-06 11:57     ` Andrey Konovalov
  0 siblings, 0 replies; 26+ messages in thread
From: Andrey Konovalov @ 2021-10-06 11:57 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Andrey Ryabinin, Marco Elver, Alexander Potapenko, Dmitry Vyukov,
	Nick Desaulniers, Arnd Bergmann, kasan-dev, LKML, llvm

On Wed, Oct 6, 2021 at 4:43 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> > This part of code always looked weird to me.
> >
> > Shouldn't we be able to pull all these options out of the else section?
> >
> > Then, the code structure would make sense: first, try applying
> > KASAN_SHADOW_OFFSET; if failed, use CFLAGS_KASAN_MINIMAL; and then try
> > applying all these options one by one.
>
> Prior to commit 1a69e7ce8391 ("kasan/Makefile: support LLVM style asan
> parameters"), all the flags were run under one cc-option, meaning that
> if $(KASAN_SHADOW_OFFSET) was not set, the whole call would fail.
> However, after that commit, it is possible to do this but I was not sure
> if that was intentional so I went for the minimal fix.

Ack. Filed https://bugzilla.kernel.org/show_bug.cgi?id=214629 for the rest.

Thanks!

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
  2021-09-23 14:59     ` Nathan Chancellor
@ 2021-10-08 18:46       ` Palmer Dabbelt
  -1 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2021-10-08 18:46 UTC (permalink / raw)
  To: nathan
  Cc: elver, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov,
	ndesaulniers, Arnd Bergmann, kasan-dev, linux-kernel, llvm,
	linux-riscv, Paul Walmsley, aou, linux-mm

On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
> On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
>> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor <nathan@kernel.org> wrote:
>> > Currently, the asan-stack parameter is only passed along if
>> > CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
>> > be defined in Kconfig so that the value can be checked. In RISC-V's
>> > case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
>> > asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
>> > is disabled, resulting in large stack warnings with allmodconfig:
>> >
>> > drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
>> > error: stack frame size (14400) exceeds limit (2048) in function
>> > 'lb035q02_connect' [-Werror,-Wframe-larger-than]
>> > static int lb035q02_connect(struct omap_dss_device *dssdev)
>> >            ^
>> > 1 error generated.
>> >
>> > Ensure that the value of CONFIG_KASAN_STACK is always passed along to
>> > the compiler so that these warnings do not happen when
>> > CONFIG_KASAN_STACK is disabled.
>> >
>> > Link: https://github.com/ClangBuiltLinux/linux/issues/1453
>> > References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
>> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>>
>> Reviewed-by: Marco Elver <elver@google.com>
>
> Thanks!
>
>> [ Which tree are you planning to take it through? ]
>
> Gah, I was intending for it to go through -mm, then I cc'd neither
> Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
> grab it from LKML?

Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>

(assuming you still want it through somewhere else)

>> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
>> comment (copied from arm64). Did RISC-V just forget to copy over the
>> Kconfig option?
>
> I do see it defined in that file as well but you are right that they did
> not copy the Kconfig logic, even though it was present in the tree when
> RISC-V KASAN was implemented. Perhaps they should so that they get
> access to the other flags in the "else" branch?

Ya, looks like we just screwed this up.  I'm seeing some warnings like

    cc1: warning: ‘-fsanitize=kernel-address’ with stack protection is not supported without ‘-fasan-shadow-offset=’ for this target

which is how I ended up here, I'm assuming that's what you're talking 
about here?  LMK if you were planning on sending along a fix or if you 
want me to go figure it out.

>
>> > ---
>> >  scripts/Makefile.kasan | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
>> > index 801c415bac59..b9e94c5e7097 100644
>> > --- a/scripts/Makefile.kasan
>> > +++ b/scripts/Makefile.kasan
>> > @@ -33,10 +33,11 @@ else
>> >         CFLAGS_KASAN := $(CFLAGS_KASAN_SHADOW) \
>> >          $(call cc-param,asan-globals=1) \
>> >          $(call cc-param,asan-instrumentation-with-call-threshold=$(call_threshold)) \
>> > -        $(call cc-param,asan-stack=$(stack_enable)) \
>> >          $(call cc-param,asan-instrument-allocas=1)
>> >  endif
>> >
>> > +CFLAGS_KASAN += $(call cc-param,asan-stack=$(stack_enable))
>> > +
>> >  endif # CONFIG_KASAN_GENERIC
>> >
>> >  ifdef CONFIG_KASAN_SW_TAGS
>> >
>> > base-commit: 4057525736b159bd456732d11270af2cc49ec21f
>> > --
>> > 2.33.0.514.g99c99ed825
>> >
>> >

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
@ 2021-10-08 18:46       ` Palmer Dabbelt
  0 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2021-10-08 18:46 UTC (permalink / raw)
  To: nathan
  Cc: elver, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov,
	ndesaulniers, Arnd Bergmann, kasan-dev, linux-kernel, llvm,
	linux-riscv, Paul Walmsley, aou, linux-mm

On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
> On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
>> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor <nathan@kernel.org> wrote:
>> > Currently, the asan-stack parameter is only passed along if
>> > CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
>> > be defined in Kconfig so that the value can be checked. In RISC-V's
>> > case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
>> > asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
>> > is disabled, resulting in large stack warnings with allmodconfig:
>> >
>> > drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
>> > error: stack frame size (14400) exceeds limit (2048) in function
>> > 'lb035q02_connect' [-Werror,-Wframe-larger-than]
>> > static int lb035q02_connect(struct omap_dss_device *dssdev)
>> >            ^
>> > 1 error generated.
>> >
>> > Ensure that the value of CONFIG_KASAN_STACK is always passed along to
>> > the compiler so that these warnings do not happen when
>> > CONFIG_KASAN_STACK is disabled.
>> >
>> > Link: https://github.com/ClangBuiltLinux/linux/issues/1453
>> > References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
>> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>>
>> Reviewed-by: Marco Elver <elver@google.com>
>
> Thanks!
>
>> [ Which tree are you planning to take it through? ]
>
> Gah, I was intending for it to go through -mm, then I cc'd neither
> Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
> grab it from LKML?

Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>

(assuming you still want it through somewhere else)

>> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
>> comment (copied from arm64). Did RISC-V just forget to copy over the
>> Kconfig option?
>
> I do see it defined in that file as well but you are right that they did
> not copy the Kconfig logic, even though it was present in the tree when
> RISC-V KASAN was implemented. Perhaps they should so that they get
> access to the other flags in the "else" branch?

Ya, looks like we just screwed this up.  I'm seeing some warnings like

    cc1: warning: ‘-fsanitize=kernel-address’ with stack protection is not supported without ‘-fasan-shadow-offset=’ for this target

which is how I ended up here, I'm assuming that's what you're talking 
about here?  LMK if you were planning on sending along a fix or if you 
want me to go figure it out.

>
>> > ---
>> >  scripts/Makefile.kasan | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
>> > index 801c415bac59..b9e94c5e7097 100644
>> > --- a/scripts/Makefile.kasan
>> > +++ b/scripts/Makefile.kasan
>> > @@ -33,10 +33,11 @@ else
>> >         CFLAGS_KASAN := $(CFLAGS_KASAN_SHADOW) \
>> >          $(call cc-param,asan-globals=1) \
>> >          $(call cc-param,asan-instrumentation-with-call-threshold=$(call_threshold)) \
>> > -        $(call cc-param,asan-stack=$(stack_enable)) \
>> >          $(call cc-param,asan-instrument-allocas=1)
>> >  endif
>> >
>> > +CFLAGS_KASAN += $(call cc-param,asan-stack=$(stack_enable))
>> > +
>> >  endif # CONFIG_KASAN_GENERIC
>> >
>> >  ifdef CONFIG_KASAN_SW_TAGS
>> >
>> > base-commit: 4057525736b159bd456732d11270af2cc49ec21f
>> > --
>> > 2.33.0.514.g99c99ed825
>> >
>> >

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
  2021-10-08 18:46       ` Palmer Dabbelt
@ 2021-10-14 16:55         ` Nathan Chancellor
  -1 siblings, 0 replies; 26+ messages in thread
From: Nathan Chancellor @ 2021-10-14 16:55 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: elver, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov,
	ndesaulniers, Arnd Bergmann, kasan-dev, linux-kernel, llvm,
	linux-riscv, Paul Walmsley, aou, linux-mm

On Fri, Oct 08, 2021 at 11:46:55AM -0700, Palmer Dabbelt wrote:
> On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
> > On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
> > > On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor <nathan@kernel.org> wrote:
> > > > Currently, the asan-stack parameter is only passed along if
> > > > CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
> > > > be defined in Kconfig so that the value can be checked. In RISC-V's
> > > > case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
> > > > asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
> > > > is disabled, resulting in large stack warnings with allmodconfig:
> > > >
> > > > drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> > > > error: stack frame size (14400) exceeds limit (2048) in function
> > > > 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> > > > static int lb035q02_connect(struct omap_dss_device *dssdev)
> > > >            ^
> > > > 1 error generated.
> > > >
> > > > Ensure that the value of CONFIG_KASAN_STACK is always passed along to
> > > > the compiler so that these warnings do not happen when
> > > > CONFIG_KASAN_STACK is disabled.
> > > >
> > > > Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> > > > References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
> > > > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > > 
> > > Reviewed-by: Marco Elver <elver@google.com>
> > 
> > Thanks!
> > 
> > > [ Which tree are you planning to take it through? ]
> > 
> > Gah, I was intending for it to go through -mm, then I cc'd neither
> > Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
> > grab it from LKML?
> 
> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
> 
> (assuming you still want it through somewhere else)

Thanks, it is now in mainline as commit 19532869feb9 ("kasan: always
respect CONFIG_KASAN_STACK").

> > > Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
> > > comment (copied from arm64). Did RISC-V just forget to copy over the
> > > Kconfig option?
> > 
> > I do see it defined in that file as well but you are right that they did
> > not copy the Kconfig logic, even though it was present in the tree when
> > RISC-V KASAN was implemented. Perhaps they should so that they get
> > access to the other flags in the "else" branch?
> 
> Ya, looks like we just screwed this up.  I'm seeing some warnings like
> 
>    cc1: warning: ‘-fsanitize=kernel-address’ with stack protection is not supported without ‘-fasan-shadow-offset=’ for this target

Hmmm, I thought I did a GCC build with this change but I must not have
:/ 

> which is how I ended up here, I'm assuming that's what you're talking about
> here?  LMK if you were planning on sending along a fix or if you want me to
> go figure it out.

I took a look at moving the logic into Kconfig like arm64 before sending
this change and I did not really understand it well enough to do so. I
think it would be best if you were able to do that so that nothing gets
messed up.

Cheers,
Nathan

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
@ 2021-10-14 16:55         ` Nathan Chancellor
  0 siblings, 0 replies; 26+ messages in thread
From: Nathan Chancellor @ 2021-10-14 16:55 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: elver, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov,
	ndesaulniers, Arnd Bergmann, kasan-dev, linux-kernel, llvm,
	linux-riscv, Paul Walmsley, aou, linux-mm

On Fri, Oct 08, 2021 at 11:46:55AM -0700, Palmer Dabbelt wrote:
> On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
> > On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
> > > On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor <nathan@kernel.org> wrote:
> > > > Currently, the asan-stack parameter is only passed along if
> > > > CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
> > > > be defined in Kconfig so that the value can be checked. In RISC-V's
> > > > case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
> > > > asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
> > > > is disabled, resulting in large stack warnings with allmodconfig:
> > > >
> > > > drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> > > > error: stack frame size (14400) exceeds limit (2048) in function
> > > > 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> > > > static int lb035q02_connect(struct omap_dss_device *dssdev)
> > > >            ^
> > > > 1 error generated.
> > > >
> > > > Ensure that the value of CONFIG_KASAN_STACK is always passed along to
> > > > the compiler so that these warnings do not happen when
> > > > CONFIG_KASAN_STACK is disabled.
> > > >
> > > > Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> > > > References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
> > > > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > > 
> > > Reviewed-by: Marco Elver <elver@google.com>
> > 
> > Thanks!
> > 
> > > [ Which tree are you planning to take it through? ]
> > 
> > Gah, I was intending for it to go through -mm, then I cc'd neither
> > Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
> > grab it from LKML?
> 
> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
> 
> (assuming you still want it through somewhere else)

Thanks, it is now in mainline as commit 19532869feb9 ("kasan: always
respect CONFIG_KASAN_STACK").

> > > Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
> > > comment (copied from arm64). Did RISC-V just forget to copy over the
> > > Kconfig option?
> > 
> > I do see it defined in that file as well but you are right that they did
> > not copy the Kconfig logic, even though it was present in the tree when
> > RISC-V KASAN was implemented. Perhaps they should so that they get
> > access to the other flags in the "else" branch?
> 
> Ya, looks like we just screwed this up.  I'm seeing some warnings like
> 
>    cc1: warning: ‘-fsanitize=kernel-address’ with stack protection is not supported without ‘-fasan-shadow-offset=’ for this target

Hmmm, I thought I did a GCC build with this change but I must not have
:/ 

> which is how I ended up here, I'm assuming that's what you're talking about
> here?  LMK if you were planning on sending along a fix or if you want me to
> go figure it out.

I took a look at moving the logic into Kconfig like arm64 before sending
this change and I did not really understand it well enough to do so. I
think it would be best if you were able to do that so that nothing gets
messed up.

Cheers,
Nathan

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
  2021-10-14 16:55         ` Nathan Chancellor
@ 2021-10-14 18:31           ` Alex Ghiti
  -1 siblings, 0 replies; 26+ messages in thread
From: Alex Ghiti @ 2021-10-14 18:31 UTC (permalink / raw)
  To: Nathan Chancellor, Palmer Dabbelt
  Cc: elver, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov,
	ndesaulniers, Arnd Bergmann, kasan-dev, linux-kernel, llvm,
	linux-riscv, Paul Walmsley, aou, linux-mm

Hi Nathan,

Le 14/10/2021 à 18:55, Nathan Chancellor a écrit :
> On Fri, Oct 08, 2021 at 11:46:55AM -0700, Palmer Dabbelt wrote:
>> On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
>>> On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
>>>> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor <nathan@kernel.org> wrote:
>>>>> Currently, the asan-stack parameter is only passed along if
>>>>> CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
>>>>> be defined in Kconfig so that the value can be checked. In RISC-V's
>>>>> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
>>>>> asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
>>>>> is disabled, resulting in large stack warnings with allmodconfig:
>>>>>
>>>>> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
>>>>> error: stack frame size (14400) exceeds limit (2048) in function
>>>>> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
>>>>> static int lb035q02_connect(struct omap_dss_device *dssdev)
>>>>>             ^
>>>>> 1 error generated.
>>>>>
>>>>> Ensure that the value of CONFIG_KASAN_STACK is always passed along to
>>>>> the compiler so that these warnings do not happen when
>>>>> CONFIG_KASAN_STACK is disabled.
>>>>>
>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
>>>>> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
>>>>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>>>>
>>>> Reviewed-by: Marco Elver <elver@google.com>
>>>
>>> Thanks!
>>>
>>>> [ Which tree are you planning to take it through? ]
>>>
>>> Gah, I was intending for it to go through -mm, then I cc'd neither
>>> Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
>>> grab it from LKML?
>>
>> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
>>
>> (assuming you still want it through somewhere else)
> 
> Thanks, it is now in mainline as commit 19532869feb9 ("kasan: always
> respect CONFIG_KASAN_STACK").
> 
>>>> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
>>>> comment (copied from arm64). Did RISC-V just forget to copy over the
>>>> Kconfig option?
>>>
>>> I do see it defined in that file as well but you are right that they did
>>> not copy the Kconfig logic, even though it was present in the tree when
>>> RISC-V KASAN was implemented. Perhaps they should so that they get
>>> access to the other flags in the "else" branch?
>>
>> Ya, looks like we just screwed this up.  I'm seeing some warnings like
>>
>>     cc1: warning: ‘-fsanitize=kernel-address’ with stack protection is not supported without ‘-fasan-shadow-offset=’ for this target
> 
> Hmmm, I thought I did a GCC build with this change but I must not have
> :/
> 
>> which is how I ended up here, I'm assuming that's what you're talking about
>> here?  LMK if you were planning on sending along a fix or if you want me to
>> go figure it out.
> 
> I took a look at moving the logic into Kconfig like arm64 before sending
> this change and I did not really understand it well enough to do so. I
> think it would be best if you were able to do that so that nothing gets
> messed up.
> 

I'll do it tomorrow, I'm the last one who touched kasan on riscv :)

Thanks,

Alex

> Cheers,
> Nathan
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
> 

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
@ 2021-10-14 18:31           ` Alex Ghiti
  0 siblings, 0 replies; 26+ messages in thread
From: Alex Ghiti @ 2021-10-14 18:31 UTC (permalink / raw)
  To: Nathan Chancellor, Palmer Dabbelt
  Cc: elver, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov,
	ndesaulniers, Arnd Bergmann, kasan-dev, linux-kernel, llvm,
	linux-riscv, Paul Walmsley, aou, linux-mm

Hi Nathan,

Le 14/10/2021 à 18:55, Nathan Chancellor a écrit :
> On Fri, Oct 08, 2021 at 11:46:55AM -0700, Palmer Dabbelt wrote:
>> On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
>>> On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
>>>> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor <nathan@kernel.org> wrote:
>>>>> Currently, the asan-stack parameter is only passed along if
>>>>> CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
>>>>> be defined in Kconfig so that the value can be checked. In RISC-V's
>>>>> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
>>>>> asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
>>>>> is disabled, resulting in large stack warnings with allmodconfig:
>>>>>
>>>>> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
>>>>> error: stack frame size (14400) exceeds limit (2048) in function
>>>>> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
>>>>> static int lb035q02_connect(struct omap_dss_device *dssdev)
>>>>>             ^
>>>>> 1 error generated.
>>>>>
>>>>> Ensure that the value of CONFIG_KASAN_STACK is always passed along to
>>>>> the compiler so that these warnings do not happen when
>>>>> CONFIG_KASAN_STACK is disabled.
>>>>>
>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
>>>>> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
>>>>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>>>>
>>>> Reviewed-by: Marco Elver <elver@google.com>
>>>
>>> Thanks!
>>>
>>>> [ Which tree are you planning to take it through? ]
>>>
>>> Gah, I was intending for it to go through -mm, then I cc'd neither
>>> Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
>>> grab it from LKML?
>>
>> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
>>
>> (assuming you still want it through somewhere else)
> 
> Thanks, it is now in mainline as commit 19532869feb9 ("kasan: always
> respect CONFIG_KASAN_STACK").
> 
>>>> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
>>>> comment (copied from arm64). Did RISC-V just forget to copy over the
>>>> Kconfig option?
>>>
>>> I do see it defined in that file as well but you are right that they did
>>> not copy the Kconfig logic, even though it was present in the tree when
>>> RISC-V KASAN was implemented. Perhaps they should so that they get
>>> access to the other flags in the "else" branch?
>>
>> Ya, looks like we just screwed this up.  I'm seeing some warnings like
>>
>>     cc1: warning: ‘-fsanitize=kernel-address’ with stack protection is not supported without ‘-fasan-shadow-offset=’ for this target
> 
> Hmmm, I thought I did a GCC build with this change but I must not have
> :/
> 
>> which is how I ended up here, I'm assuming that's what you're talking about
>> here?  LMK if you were planning on sending along a fix or if you want me to
>> go figure it out.
> 
> I took a look at moving the logic into Kconfig like arm64 before sending
> this change and I did not really understand it well enough to do so. I
> think it would be best if you were able to do that so that nothing gets
> messed up.
> 

I'll do it tomorrow, I'm the last one who touched kasan on riscv :)

Thanks,

Alex

> Cheers,
> Nathan
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
  2021-10-14 18:31           ` Alex Ghiti
@ 2021-10-15 13:04             ` Alexandre ghiti
  -1 siblings, 0 replies; 26+ messages in thread
From: Alexandre ghiti @ 2021-10-15 13:04 UTC (permalink / raw)
  To: Nathan Chancellor, Palmer Dabbelt
  Cc: elver, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov,
	ndesaulniers, Arnd Bergmann, kasan-dev, linux-kernel, llvm,
	linux-riscv, Paul Walmsley, aou, linux-mm

On 10/14/21 8:31 PM, Alex Ghiti wrote:
> Hi Nathan,
>
> Le 14/10/2021 à 18:55, Nathan Chancellor a écrit :
>> On Fri, Oct 08, 2021 at 11:46:55AM -0700, Palmer Dabbelt wrote:
>>> On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
>>>> On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
>>>>> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor
>>>>> <nathan@kernel.org> wrote:
>>>>>> Currently, the asan-stack parameter is only passed along if
>>>>>> CFLAGS_KASAN_SHADOW is not empty, which requires
>>>>>> KASAN_SHADOW_OFFSET to
>>>>>> be defined in Kconfig so that the value can be checked. In RISC-V's
>>>>>> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means
>>>>>> that
>>>>>> asan-stack does not get disabled with clang even when
>>>>>> CONFIG_KASAN_STACK
>>>>>> is disabled, resulting in large stack warnings with allmodconfig:
>>>>>>
>>>>>> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
>>>>>>
>>>>>> error: stack frame size (14400) exceeds limit (2048) in function
>>>>>> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
>>>>>> static int lb035q02_connect(struct omap_dss_device *dssdev)
>>>>>>             ^
>>>>>> 1 error generated.
>>>>>>
>>>>>> Ensure that the value of CONFIG_KASAN_STACK is always passed
>>>>>> along to
>>>>>> the compiler so that these warnings do not happen when
>>>>>> CONFIG_KASAN_STACK is disabled.
>>>>>>
>>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
>>>>>> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8
>>>>>> and earlier")
>>>>>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>>>>>
>>>>> Reviewed-by: Marco Elver <elver@google.com>
>>>>
>>>> Thanks!
>>>>
>>>>> [ Which tree are you planning to take it through? ]
>>>>
>>>> Gah, I was intending for it to go through -mm, then I cc'd neither
>>>> Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
>>>> grab it from LKML?
>>>
>>> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
>>>
>>> (assuming you still want it through somewhere else)
>>
>> Thanks, it is now in mainline as commit 19532869feb9 ("kasan: always
>> respect CONFIG_KASAN_STACK").
>>
>>>>> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
>>>>> comment (copied from arm64). Did RISC-V just forget to copy over the
>>>>> Kconfig option?
>>>>
>>>> I do see it defined in that file as well but you are right that
>>>> they did
>>>> not copy the Kconfig logic, even though it was present in the tree
>>>> when
>>>> RISC-V KASAN was implemented. Perhaps they should so that they get
>>>> access to the other flags in the "else" branch?
>>>
>>> Ya, looks like we just screwed this up.  I'm seeing some warnings like
>>>
>>>     cc1: warning: ‘-fsanitize=kernel-address’ with stack protection
>>> is not supported without ‘-fasan-shadow-offset=’ for this target
>>
>> Hmmm, I thought I did a GCC build with this change but I must not have
>> :/
>>
>>> which is how I ended up here, I'm assuming that's what you're
>>> talking about
>>> here?  LMK if you were planning on sending along a fix or if you
>>> want me to
>>> go figure it out.
>>
>> I took a look at moving the logic into Kconfig like arm64 before sending
>> this change and I did not really understand it well enough to do so. I
>> think it would be best if you were able to do that so that nothing gets
>> messed up.
>>
>
> I'll do it tomorrow, I'm the last one who touched kasan on riscv :)
>

Adding KASAN_SHADOW_OFFSET config makes kasan kernel fails to boot.
It receives a *write* fault at the beginning of a memblock_alloc
function while populating the kernel shadow memory: the trap address is
in the kasan shadow virtual address range and this corresponds to a
kernel address in init_stack. The question is: how do I populate the
stack shadow mapping without using memblock API? It's weird, I don't
find anything on other architectures.

And just a short note: I have realized this will break with the sv48
patchset as we decide at runtime the address space width and the kasan
shadow start address is different between sv39 and sv48. I will have to
do like x86 and move the kasan shadow start at the end of the address
space so that it is the same for both sv39 and sv48.

Thanks,

Alex


> Thanks,
>
> Alex
>
>> Cheers,
>> Nathan
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
@ 2021-10-15 13:04             ` Alexandre ghiti
  0 siblings, 0 replies; 26+ messages in thread
From: Alexandre ghiti @ 2021-10-15 13:04 UTC (permalink / raw)
  To: Nathan Chancellor, Palmer Dabbelt
  Cc: elver, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov,
	ndesaulniers, Arnd Bergmann, kasan-dev, linux-kernel, llvm,
	linux-riscv, Paul Walmsley, aou, linux-mm

On 10/14/21 8:31 PM, Alex Ghiti wrote:
> Hi Nathan,
>
> Le 14/10/2021 à 18:55, Nathan Chancellor a écrit :
>> On Fri, Oct 08, 2021 at 11:46:55AM -0700, Palmer Dabbelt wrote:
>>> On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
>>>> On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
>>>>> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor
>>>>> <nathan@kernel.org> wrote:
>>>>>> Currently, the asan-stack parameter is only passed along if
>>>>>> CFLAGS_KASAN_SHADOW is not empty, which requires
>>>>>> KASAN_SHADOW_OFFSET to
>>>>>> be defined in Kconfig so that the value can be checked. In RISC-V's
>>>>>> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means
>>>>>> that
>>>>>> asan-stack does not get disabled with clang even when
>>>>>> CONFIG_KASAN_STACK
>>>>>> is disabled, resulting in large stack warnings with allmodconfig:
>>>>>>
>>>>>> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
>>>>>>
>>>>>> error: stack frame size (14400) exceeds limit (2048) in function
>>>>>> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
>>>>>> static int lb035q02_connect(struct omap_dss_device *dssdev)
>>>>>>             ^
>>>>>> 1 error generated.
>>>>>>
>>>>>> Ensure that the value of CONFIG_KASAN_STACK is always passed
>>>>>> along to
>>>>>> the compiler so that these warnings do not happen when
>>>>>> CONFIG_KASAN_STACK is disabled.
>>>>>>
>>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
>>>>>> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8
>>>>>> and earlier")
>>>>>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>>>>>
>>>>> Reviewed-by: Marco Elver <elver@google.com>
>>>>
>>>> Thanks!
>>>>
>>>>> [ Which tree are you planning to take it through? ]
>>>>
>>>> Gah, I was intending for it to go through -mm, then I cc'd neither
>>>> Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
>>>> grab it from LKML?
>>>
>>> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
>>>
>>> (assuming you still want it through somewhere else)
>>
>> Thanks, it is now in mainline as commit 19532869feb9 ("kasan: always
>> respect CONFIG_KASAN_STACK").
>>
>>>>> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
>>>>> comment (copied from arm64). Did RISC-V just forget to copy over the
>>>>> Kconfig option?
>>>>
>>>> I do see it defined in that file as well but you are right that
>>>> they did
>>>> not copy the Kconfig logic, even though it was present in the tree
>>>> when
>>>> RISC-V KASAN was implemented. Perhaps they should so that they get
>>>> access to the other flags in the "else" branch?
>>>
>>> Ya, looks like we just screwed this up.  I'm seeing some warnings like
>>>
>>>     cc1: warning: ‘-fsanitize=kernel-address’ with stack protection
>>> is not supported without ‘-fasan-shadow-offset=’ for this target
>>
>> Hmmm, I thought I did a GCC build with this change but I must not have
>> :/
>>
>>> which is how I ended up here, I'm assuming that's what you're
>>> talking about
>>> here?  LMK if you were planning on sending along a fix or if you
>>> want me to
>>> go figure it out.
>>
>> I took a look at moving the logic into Kconfig like arm64 before sending
>> this change and I did not really understand it well enough to do so. I
>> think it would be best if you were able to do that so that nothing gets
>> messed up.
>>
>
> I'll do it tomorrow, I'm the last one who touched kasan on riscv :)
>

Adding KASAN_SHADOW_OFFSET config makes kasan kernel fails to boot.
It receives a *write* fault at the beginning of a memblock_alloc
function while populating the kernel shadow memory: the trap address is
in the kasan shadow virtual address range and this corresponds to a
kernel address in init_stack. The question is: how do I populate the
stack shadow mapping without using memblock API? It's weird, I don't
find anything on other architectures.

And just a short note: I have realized this will break with the sv48
patchset as we decide at runtime the address space width and the kasan
shadow start address is different between sv39 and sv48. I will have to
do like x86 and move the kasan shadow start at the end of the address
space so that it is the same for both sv39 and sv48.

Thanks,

Alex


> Thanks,
>
> Alex
>
>> Cheers,
>> Nathan
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
  2021-10-14 18:31           ` Alex Ghiti
@ 2021-10-16  1:11             ` Palmer Dabbelt
  -1 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2021-10-16  1:11 UTC (permalink / raw)
  To: alex
  Cc: nathan, elver, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov,
	ndesaulniers, Arnd Bergmann, kasan-dev, linux-kernel, llvm,
	linux-riscv, Paul Walmsley, aou, linux-mm

On Thu, 14 Oct 2021 11:31:00 PDT (-0700), alex@ghiti.fr wrote:
> Hi Nathan,
>
> Le 14/10/2021 à 18:55, Nathan Chancellor a écrit :
>> On Fri, Oct 08, 2021 at 11:46:55AM -0700, Palmer Dabbelt wrote:
>>> On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
>>>> On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
>>>>> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor <nathan@kernel.org> wrote:
>>>>>> Currently, the asan-stack parameter is only passed along if
>>>>>> CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
>>>>>> be defined in Kconfig so that the value can be checked. In RISC-V's
>>>>>> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
>>>>>> asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
>>>>>> is disabled, resulting in large stack warnings with allmodconfig:
>>>>>>
>>>>>> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
>>>>>> error: stack frame size (14400) exceeds limit (2048) in function
>>>>>> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
>>>>>> static int lb035q02_connect(struct omap_dss_device *dssdev)
>>>>>>             ^
>>>>>> 1 error generated.
>>>>>>
>>>>>> Ensure that the value of CONFIG_KASAN_STACK is always passed along to
>>>>>> the compiler so that these warnings do not happen when
>>>>>> CONFIG_KASAN_STACK is disabled.
>>>>>>
>>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
>>>>>> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
>>>>>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>>>>>
>>>>> Reviewed-by: Marco Elver <elver@google.com>
>>>>
>>>> Thanks!
>>>>
>>>>> [ Which tree are you planning to take it through? ]
>>>>
>>>> Gah, I was intending for it to go through -mm, then I cc'd neither
>>>> Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
>>>> grab it from LKML?
>>>
>>> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
>>>
>>> (assuming you still want it through somewhere else)
>>
>> Thanks, it is now in mainline as commit 19532869feb9 ("kasan: always
>> respect CONFIG_KASAN_STACK").
>>
>>>>> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
>>>>> comment (copied from arm64). Did RISC-V just forget to copy over the
>>>>> Kconfig option?
>>>>
>>>> I do see it defined in that file as well but you are right that they did
>>>> not copy the Kconfig logic, even though it was present in the tree when
>>>> RISC-V KASAN was implemented. Perhaps they should so that they get
>>>> access to the other flags in the "else" branch?
>>>
>>> Ya, looks like we just screwed this up.  I'm seeing some warnings like
>>>
>>>     cc1: warning: ‘-fsanitize=kernel-address’ with stack protection is not supported without ‘-fasan-shadow-offset=’ for this target
>>
>> Hmmm, I thought I did a GCC build with this change but I must not have
>> :/
>>
>>> which is how I ended up here, I'm assuming that's what you're talking about
>>> here?  LMK if you were planning on sending along a fix or if you want me to
>>> go figure it out.
>>
>> I took a look at moving the logic into Kconfig like arm64 before sending
>> this change and I did not really understand it well enough to do so. I
>> think it would be best if you were able to do that so that nothing gets
>> messed up.
>>
>
> I'll do it tomorrow, I'm the last one who touched kasan on riscv :)

Any luck?  I tried what I think is the simple way to do it last week, 
(merging with Linus' tree is turning these warnings into build 
failures) but it's hanging on boot.  Not sure what's going on

    diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
    index c3f3fd583e04..d3998b4a45f1 100644
    --- a/arch/riscv/Kconfig
    +++ b/arch/riscv/Kconfig
    @@ -212,6 +212,12 @@ config PGTABLE_LEVELS
     config LOCKDEP_SUPPORT
            def_bool y
    
    +config KASAN_SHADOW_OFFSET
    +       hex
    +       depends on KASAN_GENERIC
    +       default 0xdfffffc800000000  if 64BIT
    +       default 0xffffffff          if 32BIT
    +
     source "arch/riscv/Kconfig.socs"
     source "arch/riscv/Kconfig.erratas"
    
    diff --git a/arch/riscv/include/asm/kasan.h b/arch/riscv/include/asm/kasan.h
    index a2b3d9cdbc86..b00f503ec124 100644
    --- a/arch/riscv/include/asm/kasan.h
    +++ b/arch/riscv/include/asm/kasan.h
    @@ -30,8 +30,7 @@
     #define KASAN_SHADOW_SIZE      (UL(1) << ((CONFIG_VA_BITS - 1) - KASAN_SHADOW_SCALE_SHIFT))
     #define KASAN_SHADOW_START     KERN_VIRT_START
     #define KASAN_SHADOW_END       (KASAN_SHADOW_START + KASAN_SHADOW_SIZE)
    -#define KASAN_SHADOW_OFFSET    (KASAN_SHADOW_END - (1ULL << \
    -                                       (64 - KASAN_SHADOW_SCALE_SHIFT)))
    +#define KASAN_SHADOW_OFFSET    _AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
    
     void kasan_init(void);
     asmlinkage void kasan_early_init(void);

>
> Thanks,
>
> Alex
>
>> Cheers,
>> Nathan
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
@ 2021-10-16  1:11             ` Palmer Dabbelt
  0 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2021-10-16  1:11 UTC (permalink / raw)
  To: alex
  Cc: nathan, elver, akpm, ryabinin.a.a, glider, andreyknvl, dvyukov,
	ndesaulniers, Arnd Bergmann, kasan-dev, linux-kernel, llvm,
	linux-riscv, Paul Walmsley, aou, linux-mm

On Thu, 14 Oct 2021 11:31:00 PDT (-0700), alex@ghiti.fr wrote:
> Hi Nathan,
>
> Le 14/10/2021 à 18:55, Nathan Chancellor a écrit :
>> On Fri, Oct 08, 2021 at 11:46:55AM -0700, Palmer Dabbelt wrote:
>>> On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
>>>> On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
>>>>> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor <nathan@kernel.org> wrote:
>>>>>> Currently, the asan-stack parameter is only passed along if
>>>>>> CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
>>>>>> be defined in Kconfig so that the value can be checked. In RISC-V's
>>>>>> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
>>>>>> asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
>>>>>> is disabled, resulting in large stack warnings with allmodconfig:
>>>>>>
>>>>>> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
>>>>>> error: stack frame size (14400) exceeds limit (2048) in function
>>>>>> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
>>>>>> static int lb035q02_connect(struct omap_dss_device *dssdev)
>>>>>>             ^
>>>>>> 1 error generated.
>>>>>>
>>>>>> Ensure that the value of CONFIG_KASAN_STACK is always passed along to
>>>>>> the compiler so that these warnings do not happen when
>>>>>> CONFIG_KASAN_STACK is disabled.
>>>>>>
>>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
>>>>>> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
>>>>>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>>>>>
>>>>> Reviewed-by: Marco Elver <elver@google.com>
>>>>
>>>> Thanks!
>>>>
>>>>> [ Which tree are you planning to take it through? ]
>>>>
>>>> Gah, I was intending for it to go through -mm, then I cc'd neither
>>>> Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
>>>> grab it from LKML?
>>>
>>> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
>>>
>>> (assuming you still want it through somewhere else)
>>
>> Thanks, it is now in mainline as commit 19532869feb9 ("kasan: always
>> respect CONFIG_KASAN_STACK").
>>
>>>>> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
>>>>> comment (copied from arm64). Did RISC-V just forget to copy over the
>>>>> Kconfig option?
>>>>
>>>> I do see it defined in that file as well but you are right that they did
>>>> not copy the Kconfig logic, even though it was present in the tree when
>>>> RISC-V KASAN was implemented. Perhaps they should so that they get
>>>> access to the other flags in the "else" branch?
>>>
>>> Ya, looks like we just screwed this up.  I'm seeing some warnings like
>>>
>>>     cc1: warning: ‘-fsanitize=kernel-address’ with stack protection is not supported without ‘-fasan-shadow-offset=’ for this target
>>
>> Hmmm, I thought I did a GCC build with this change but I must not have
>> :/
>>
>>> which is how I ended up here, I'm assuming that's what you're talking about
>>> here?  LMK if you were planning on sending along a fix or if you want me to
>>> go figure it out.
>>
>> I took a look at moving the logic into Kconfig like arm64 before sending
>> this change and I did not really understand it well enough to do so. I
>> think it would be best if you were able to do that so that nothing gets
>> messed up.
>>
>
> I'll do it tomorrow, I'm the last one who touched kasan on riscv :)

Any luck?  I tried what I think is the simple way to do it last week, 
(merging with Linus' tree is turning these warnings into build 
failures) but it's hanging on boot.  Not sure what's going on

    diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
    index c3f3fd583e04..d3998b4a45f1 100644
    --- a/arch/riscv/Kconfig
    +++ b/arch/riscv/Kconfig
    @@ -212,6 +212,12 @@ config PGTABLE_LEVELS
     config LOCKDEP_SUPPORT
            def_bool y
    
    +config KASAN_SHADOW_OFFSET
    +       hex
    +       depends on KASAN_GENERIC
    +       default 0xdfffffc800000000  if 64BIT
    +       default 0xffffffff          if 32BIT
    +
     source "arch/riscv/Kconfig.socs"
     source "arch/riscv/Kconfig.erratas"
    
    diff --git a/arch/riscv/include/asm/kasan.h b/arch/riscv/include/asm/kasan.h
    index a2b3d9cdbc86..b00f503ec124 100644
    --- a/arch/riscv/include/asm/kasan.h
    +++ b/arch/riscv/include/asm/kasan.h
    @@ -30,8 +30,7 @@
     #define KASAN_SHADOW_SIZE      (UL(1) << ((CONFIG_VA_BITS - 1) - KASAN_SHADOW_SCALE_SHIFT))
     #define KASAN_SHADOW_START     KERN_VIRT_START
     #define KASAN_SHADOW_END       (KASAN_SHADOW_START + KASAN_SHADOW_SIZE)
    -#define KASAN_SHADOW_OFFSET    (KASAN_SHADOW_END - (1ULL << \
    -                                       (64 - KASAN_SHADOW_SCALE_SHIFT)))
    +#define KASAN_SHADOW_OFFSET    _AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
    
     void kasan_init(void);
     asmlinkage void kasan_early_init(void);

>
> Thanks,
>
> Alex
>
>> Cheers,
>> Nathan
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
  2021-10-15 13:04             ` Alexandre ghiti
@ 2021-10-26  4:39               ` Alexandre Ghiti
  -1 siblings, 0 replies; 26+ messages in thread
From: Alexandre Ghiti @ 2021-10-26  4:39 UTC (permalink / raw)
  To: Alexandre ghiti
  Cc: Nathan Chancellor, Palmer Dabbelt, elver, akpm, ryabinin.a.a,
	glider, andreyknvl, dvyukov, ndesaulniers, Arnd Bergmann,
	kasan-dev, linux-kernel, llvm, linux-riscv, Paul Walmsley, aou,
	linux-mm

Hi,

On Fri, Oct 15, 2021 at 3:08 PM Alexandre ghiti <alex@ghiti.fr> wrote:
>
> On 10/14/21 8:31 PM, Alex Ghiti wrote:
> > Hi Nathan,
> >
> > Le 14/10/2021 à 18:55, Nathan Chancellor a écrit :
> >> On Fri, Oct 08, 2021 at 11:46:55AM -0700, Palmer Dabbelt wrote:
> >>> On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
> >>>> On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
> >>>>> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor
> >>>>> <nathan@kernel.org> wrote:
> >>>>>> Currently, the asan-stack parameter is only passed along if
> >>>>>> CFLAGS_KASAN_SHADOW is not empty, which requires
> >>>>>> KASAN_SHADOW_OFFSET to
> >>>>>> be defined in Kconfig so that the value can be checked. In RISC-V's
> >>>>>> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means
> >>>>>> that
> >>>>>> asan-stack does not get disabled with clang even when
> >>>>>> CONFIG_KASAN_STACK
> >>>>>> is disabled, resulting in large stack warnings with allmodconfig:
> >>>>>>
> >>>>>> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> >>>>>>
> >>>>>> error: stack frame size (14400) exceeds limit (2048) in function
> >>>>>> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> >>>>>> static int lb035q02_connect(struct omap_dss_device *dssdev)
> >>>>>>             ^
> >>>>>> 1 error generated.
> >>>>>>
> >>>>>> Ensure that the value of CONFIG_KASAN_STACK is always passed
> >>>>>> along to
> >>>>>> the compiler so that these warnings do not happen when
> >>>>>> CONFIG_KASAN_STACK is disabled.
> >>>>>>
> >>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> >>>>>> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8
> >>>>>> and earlier")
> >>>>>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> >>>>>
> >>>>> Reviewed-by: Marco Elver <elver@google.com>
> >>>>
> >>>> Thanks!
> >>>>
> >>>>> [ Which tree are you planning to take it through? ]
> >>>>
> >>>> Gah, I was intending for it to go through -mm, then I cc'd neither
> >>>> Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
> >>>> grab it from LKML?
> >>>
> >>> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
> >>>
> >>> (assuming you still want it through somewhere else)
> >>
> >> Thanks, it is now in mainline as commit 19532869feb9 ("kasan: always
> >> respect CONFIG_KASAN_STACK").
> >>
> >>>>> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
> >>>>> comment (copied from arm64). Did RISC-V just forget to copy over the
> >>>>> Kconfig option?
> >>>>
> >>>> I do see it defined in that file as well but you are right that
> >>>> they did
> >>>> not copy the Kconfig logic, even though it was present in the tree
> >>>> when
> >>>> RISC-V KASAN was implemented. Perhaps they should so that they get
> >>>> access to the other flags in the "else" branch?
> >>>
> >>> Ya, looks like we just screwed this up.  I'm seeing some warnings like
> >>>
> >>>     cc1: warning: ‘-fsanitize=kernel-address’ with stack protection
> >>> is not supported without ‘-fasan-shadow-offset=’ for this target
> >>
> >> Hmmm, I thought I did a GCC build with this change but I must not have
> >> :/
> >>
> >>> which is how I ended up here, I'm assuming that's what you're
> >>> talking about
> >>> here?  LMK if you were planning on sending along a fix or if you
> >>> want me to
> >>> go figure it out.
> >>
> >> I took a look at moving the logic into Kconfig like arm64 before sending
> >> this change and I did not really understand it well enough to do so. I
> >> think it would be best if you were able to do that so that nothing gets
> >> messed up.
> >>
> >
> > I'll do it tomorrow, I'm the last one who touched kasan on riscv :)
> >
>
> Adding KASAN_SHADOW_OFFSET config makes kasan kernel fails to boot.
> It receives a *write* fault at the beginning of a memblock_alloc
> function while populating the kernel shadow memory: the trap address is
> in the kasan shadow virtual address range and this corresponds to a
> kernel address in init_stack. The question is: how do I populate the
> stack shadow mapping without using memblock API? It's weird, I don't
> find anything on other architectures.

@kasan: Any idea what we are doing wrong in riscv to encounter the
above situation?

Thanks,

Alex

>
> And just a short note: I have realized this will break with the sv48
> patchset as we decide at runtime the address space width and the kasan
> shadow start address is different between sv39 and sv48. I will have to
> do like x86 and move the kasan shadow start at the end of the address
> space so that it is the same for both sv39 and sv48.
>
> Thanks,
>
> Alex
>
>
> > Thanks,
> >
> > Alex
> >
> >> Cheers,
> >> Nathan
> >>
> >> _______________________________________________
> >> linux-riscv mailing list
> >> linux-riscv@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-riscv
> >>
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
@ 2021-10-26  4:39               ` Alexandre Ghiti
  0 siblings, 0 replies; 26+ messages in thread
From: Alexandre Ghiti @ 2021-10-26  4:39 UTC (permalink / raw)
  To: Alexandre ghiti
  Cc: Nathan Chancellor, Palmer Dabbelt, elver, akpm, ryabinin.a.a,
	glider, andreyknvl, dvyukov, ndesaulniers, Arnd Bergmann,
	kasan-dev, linux-kernel, llvm, linux-riscv, Paul Walmsley, aou,
	linux-mm

Hi,

On Fri, Oct 15, 2021 at 3:08 PM Alexandre ghiti <alex@ghiti.fr> wrote:
>
> On 10/14/21 8:31 PM, Alex Ghiti wrote:
> > Hi Nathan,
> >
> > Le 14/10/2021 à 18:55, Nathan Chancellor a écrit :
> >> On Fri, Oct 08, 2021 at 11:46:55AM -0700, Palmer Dabbelt wrote:
> >>> On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
> >>>> On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
> >>>>> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor
> >>>>> <nathan@kernel.org> wrote:
> >>>>>> Currently, the asan-stack parameter is only passed along if
> >>>>>> CFLAGS_KASAN_SHADOW is not empty, which requires
> >>>>>> KASAN_SHADOW_OFFSET to
> >>>>>> be defined in Kconfig so that the value can be checked. In RISC-V's
> >>>>>> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means
> >>>>>> that
> >>>>>> asan-stack does not get disabled with clang even when
> >>>>>> CONFIG_KASAN_STACK
> >>>>>> is disabled, resulting in large stack warnings with allmodconfig:
> >>>>>>
> >>>>>> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> >>>>>>
> >>>>>> error: stack frame size (14400) exceeds limit (2048) in function
> >>>>>> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> >>>>>> static int lb035q02_connect(struct omap_dss_device *dssdev)
> >>>>>>             ^
> >>>>>> 1 error generated.
> >>>>>>
> >>>>>> Ensure that the value of CONFIG_KASAN_STACK is always passed
> >>>>>> along to
> >>>>>> the compiler so that these warnings do not happen when
> >>>>>> CONFIG_KASAN_STACK is disabled.
> >>>>>>
> >>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> >>>>>> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8
> >>>>>> and earlier")
> >>>>>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> >>>>>
> >>>>> Reviewed-by: Marco Elver <elver@google.com>
> >>>>
> >>>> Thanks!
> >>>>
> >>>>> [ Which tree are you planning to take it through? ]
> >>>>
> >>>> Gah, I was intending for it to go through -mm, then I cc'd neither
> >>>> Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
> >>>> grab it from LKML?
> >>>
> >>> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
> >>>
> >>> (assuming you still want it through somewhere else)
> >>
> >> Thanks, it is now in mainline as commit 19532869feb9 ("kasan: always
> >> respect CONFIG_KASAN_STACK").
> >>
> >>>>> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
> >>>>> comment (copied from arm64). Did RISC-V just forget to copy over the
> >>>>> Kconfig option?
> >>>>
> >>>> I do see it defined in that file as well but you are right that
> >>>> they did
> >>>> not copy the Kconfig logic, even though it was present in the tree
> >>>> when
> >>>> RISC-V KASAN was implemented. Perhaps they should so that they get
> >>>> access to the other flags in the "else" branch?
> >>>
> >>> Ya, looks like we just screwed this up.  I'm seeing some warnings like
> >>>
> >>>     cc1: warning: ‘-fsanitize=kernel-address’ with stack protection
> >>> is not supported without ‘-fasan-shadow-offset=’ for this target
> >>
> >> Hmmm, I thought I did a GCC build with this change but I must not have
> >> :/
> >>
> >>> which is how I ended up here, I'm assuming that's what you're
> >>> talking about
> >>> here?  LMK if you were planning on sending along a fix or if you
> >>> want me to
> >>> go figure it out.
> >>
> >> I took a look at moving the logic into Kconfig like arm64 before sending
> >> this change and I did not really understand it well enough to do so. I
> >> think it would be best if you were able to do that so that nothing gets
> >> messed up.
> >>
> >
> > I'll do it tomorrow, I'm the last one who touched kasan on riscv :)
> >
>
> Adding KASAN_SHADOW_OFFSET config makes kasan kernel fails to boot.
> It receives a *write* fault at the beginning of a memblock_alloc
> function while populating the kernel shadow memory: the trap address is
> in the kasan shadow virtual address range and this corresponds to a
> kernel address in init_stack. The question is: how do I populate the
> stack shadow mapping without using memblock API? It's weird, I don't
> find anything on other architectures.

@kasan: Any idea what we are doing wrong in riscv to encounter the
above situation?

Thanks,

Alex

>
> And just a short note: I have realized this will break with the sv48
> patchset as we decide at runtime the address space width and the kasan
> shadow start address is different between sv39 and sv48. I will have to
> do like x86 and move the kasan shadow start at the end of the address
> space so that it is the same for both sv39 and sv48.
>
> Thanks,
>
> Alex
>
>
> > Thanks,
> >
> > Alex
> >
> >> Cheers,
> >> Nathan
> >>
> >> _______________________________________________
> >> linux-riscv mailing list
> >> linux-riscv@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-riscv
> >>
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
  2021-10-26  4:39               ` Alexandre Ghiti
@ 2021-10-26  4:48                 ` Dmitry Vyukov
  -1 siblings, 0 replies; 26+ messages in thread
From: Dmitry Vyukov @ 2021-10-26  4:48 UTC (permalink / raw)
  To: Alexandre Ghiti
  Cc: Alexandre ghiti, Nathan Chancellor, Palmer Dabbelt, elver, akpm,
	ryabinin.a.a, glider, andreyknvl, ndesaulniers, Arnd Bergmann,
	kasan-dev, linux-kernel, llvm, linux-riscv, Paul Walmsley, aou,
	linux-mm

On Tue, 26 Oct 2021 at 06:39, Alexandre Ghiti
<alexandre.ghiti@canonical.com> wrote:
>
> Hi,
>
> On Fri, Oct 15, 2021 at 3:08 PM Alexandre ghiti <alex@ghiti.fr> wrote:
> >
> > On 10/14/21 8:31 PM, Alex Ghiti wrote:
> > > Hi Nathan,
> > >
> > > Le 14/10/2021 à 18:55, Nathan Chancellor a écrit :
> > >> On Fri, Oct 08, 2021 at 11:46:55AM -0700, Palmer Dabbelt wrote:
> > >>> On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
> > >>>> On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
> > >>>>> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor
> > >>>>> <nathan@kernel.org> wrote:
> > >>>>>> Currently, the asan-stack parameter is only passed along if
> > >>>>>> CFLAGS_KASAN_SHADOW is not empty, which requires
> > >>>>>> KASAN_SHADOW_OFFSET to
> > >>>>>> be defined in Kconfig so that the value can be checked. In RISC-V's
> > >>>>>> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means
> > >>>>>> that
> > >>>>>> asan-stack does not get disabled with clang even when
> > >>>>>> CONFIG_KASAN_STACK
> > >>>>>> is disabled, resulting in large stack warnings with allmodconfig:
> > >>>>>>
> > >>>>>> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> > >>>>>>
> > >>>>>> error: stack frame size (14400) exceeds limit (2048) in function
> > >>>>>> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> > >>>>>> static int lb035q02_connect(struct omap_dss_device *dssdev)
> > >>>>>>             ^
> > >>>>>> 1 error generated.
> > >>>>>>
> > >>>>>> Ensure that the value of CONFIG_KASAN_STACK is always passed
> > >>>>>> along to
> > >>>>>> the compiler so that these warnings do not happen when
> > >>>>>> CONFIG_KASAN_STACK is disabled.
> > >>>>>>
> > >>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> > >>>>>> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8
> > >>>>>> and earlier")
> > >>>>>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > >>>>>
> > >>>>> Reviewed-by: Marco Elver <elver@google.com>
> > >>>>
> > >>>> Thanks!
> > >>>>
> > >>>>> [ Which tree are you planning to take it through? ]
> > >>>>
> > >>>> Gah, I was intending for it to go through -mm, then I cc'd neither
> > >>>> Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
> > >>>> grab it from LKML?
> > >>>
> > >>> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
> > >>>
> > >>> (assuming you still want it through somewhere else)
> > >>
> > >> Thanks, it is now in mainline as commit 19532869feb9 ("kasan: always
> > >> respect CONFIG_KASAN_STACK").
> > >>
> > >>>>> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
> > >>>>> comment (copied from arm64). Did RISC-V just forget to copy over the
> > >>>>> Kconfig option?
> > >>>>
> > >>>> I do see it defined in that file as well but you are right that
> > >>>> they did
> > >>>> not copy the Kconfig logic, even though it was present in the tree
> > >>>> when
> > >>>> RISC-V KASAN was implemented. Perhaps they should so that they get
> > >>>> access to the other flags in the "else" branch?
> > >>>
> > >>> Ya, looks like we just screwed this up.  I'm seeing some warnings like
> > >>>
> > >>>     cc1: warning: ‘-fsanitize=kernel-address’ with stack protection
> > >>> is not supported without ‘-fasan-shadow-offset=’ for this target
> > >>
> > >> Hmmm, I thought I did a GCC build with this change but I must not have
> > >> :/
> > >>
> > >>> which is how I ended up here, I'm assuming that's what you're
> > >>> talking about
> > >>> here?  LMK if you were planning on sending along a fix or if you
> > >>> want me to
> > >>> go figure it out.
> > >>
> > >> I took a look at moving the logic into Kconfig like arm64 before sending
> > >> this change and I did not really understand it well enough to do so. I
> > >> think it would be best if you were able to do that so that nothing gets
> > >> messed up.
> > >>
> > >
> > > I'll do it tomorrow, I'm the last one who touched kasan on riscv :)
> > >
> >
> > Adding KASAN_SHADOW_OFFSET config makes kasan kernel fails to boot.
> > It receives a *write* fault at the beginning of a memblock_alloc
> > function while populating the kernel shadow memory: the trap address is
> > in the kasan shadow virtual address range and this corresponds to a
> > kernel address in init_stack. The question is: how do I populate the
> > stack shadow mapping without using memblock API? It's weird, I don't
> > find anything on other architectures.
>
> @kasan: Any idea what we are doing wrong in riscv to encounter the
> above situation?

Hi Alex, Palmer,

The patch changes the definition of the KASAN_SHADOW_OFFSET const.
Does it's value change as a result or not? Have you tried to print it
before/after?
If value does not change, then this is more mysterious. If it changes,
then there lots of possible explanations (points to unmapped region,
overlaps with something), but we need to know values before/after to
answer this.


> Thanks,
>
> Alex
>
> >
> > And just a short note: I have realized this will break with the sv48
> > patchset as we decide at runtime the address space width and the kasan
> > shadow start address is different between sv39 and sv48. I will have to
> > do like x86 and move the kasan shadow start at the end of the address
> > space so that it is the same for both sv39 and sv48.
> >
> > Thanks,
> >
> > Alex
> >
> >
> > > Thanks,
> > >
> > > Alex
> > >
> > >> Cheers,
> > >> Nathan
> > >>
> > >> _______________________________________________
> > >> linux-riscv mailing list
> > >> linux-riscv@lists.infradead.org
> > >> http://lists.infradead.org/mailman/listinfo/linux-riscv
> > >>
> > >
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
@ 2021-10-26  4:48                 ` Dmitry Vyukov
  0 siblings, 0 replies; 26+ messages in thread
From: Dmitry Vyukov @ 2021-10-26  4:48 UTC (permalink / raw)
  To: Alexandre Ghiti
  Cc: Alexandre ghiti, Nathan Chancellor, Palmer Dabbelt, elver, akpm,
	ryabinin.a.a, glider, andreyknvl, ndesaulniers, Arnd Bergmann,
	kasan-dev, linux-kernel, llvm, linux-riscv, Paul Walmsley, aou,
	linux-mm

On Tue, 26 Oct 2021 at 06:39, Alexandre Ghiti
<alexandre.ghiti@canonical.com> wrote:
>
> Hi,
>
> On Fri, Oct 15, 2021 at 3:08 PM Alexandre ghiti <alex@ghiti.fr> wrote:
> >
> > On 10/14/21 8:31 PM, Alex Ghiti wrote:
> > > Hi Nathan,
> > >
> > > Le 14/10/2021 à 18:55, Nathan Chancellor a écrit :
> > >> On Fri, Oct 08, 2021 at 11:46:55AM -0700, Palmer Dabbelt wrote:
> > >>> On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
> > >>>> On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
> > >>>>> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor
> > >>>>> <nathan@kernel.org> wrote:
> > >>>>>> Currently, the asan-stack parameter is only passed along if
> > >>>>>> CFLAGS_KASAN_SHADOW is not empty, which requires
> > >>>>>> KASAN_SHADOW_OFFSET to
> > >>>>>> be defined in Kconfig so that the value can be checked. In RISC-V's
> > >>>>>> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means
> > >>>>>> that
> > >>>>>> asan-stack does not get disabled with clang even when
> > >>>>>> CONFIG_KASAN_STACK
> > >>>>>> is disabled, resulting in large stack warnings with allmodconfig:
> > >>>>>>
> > >>>>>> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> > >>>>>>
> > >>>>>> error: stack frame size (14400) exceeds limit (2048) in function
> > >>>>>> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> > >>>>>> static int lb035q02_connect(struct omap_dss_device *dssdev)
> > >>>>>>             ^
> > >>>>>> 1 error generated.
> > >>>>>>
> > >>>>>> Ensure that the value of CONFIG_KASAN_STACK is always passed
> > >>>>>> along to
> > >>>>>> the compiler so that these warnings do not happen when
> > >>>>>> CONFIG_KASAN_STACK is disabled.
> > >>>>>>
> > >>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> > >>>>>> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8
> > >>>>>> and earlier")
> > >>>>>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > >>>>>
> > >>>>> Reviewed-by: Marco Elver <elver@google.com>
> > >>>>
> > >>>> Thanks!
> > >>>>
> > >>>>> [ Which tree are you planning to take it through? ]
> > >>>>
> > >>>> Gah, I was intending for it to go through -mm, then I cc'd neither
> > >>>> Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
> > >>>> grab it from LKML?
> > >>>
> > >>> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
> > >>>
> > >>> (assuming you still want it through somewhere else)
> > >>
> > >> Thanks, it is now in mainline as commit 19532869feb9 ("kasan: always
> > >> respect CONFIG_KASAN_STACK").
> > >>
> > >>>>> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
> > >>>>> comment (copied from arm64). Did RISC-V just forget to copy over the
> > >>>>> Kconfig option?
> > >>>>
> > >>>> I do see it defined in that file as well but you are right that
> > >>>> they did
> > >>>> not copy the Kconfig logic, even though it was present in the tree
> > >>>> when
> > >>>> RISC-V KASAN was implemented. Perhaps they should so that they get
> > >>>> access to the other flags in the "else" branch?
> > >>>
> > >>> Ya, looks like we just screwed this up.  I'm seeing some warnings like
> > >>>
> > >>>     cc1: warning: ‘-fsanitize=kernel-address’ with stack protection
> > >>> is not supported without ‘-fasan-shadow-offset=’ for this target
> > >>
> > >> Hmmm, I thought I did a GCC build with this change but I must not have
> > >> :/
> > >>
> > >>> which is how I ended up here, I'm assuming that's what you're
> > >>> talking about
> > >>> here?  LMK if you were planning on sending along a fix or if you
> > >>> want me to
> > >>> go figure it out.
> > >>
> > >> I took a look at moving the logic into Kconfig like arm64 before sending
> > >> this change and I did not really understand it well enough to do so. I
> > >> think it would be best if you were able to do that so that nothing gets
> > >> messed up.
> > >>
> > >
> > > I'll do it tomorrow, I'm the last one who touched kasan on riscv :)
> > >
> >
> > Adding KASAN_SHADOW_OFFSET config makes kasan kernel fails to boot.
> > It receives a *write* fault at the beginning of a memblock_alloc
> > function while populating the kernel shadow memory: the trap address is
> > in the kasan shadow virtual address range and this corresponds to a
> > kernel address in init_stack. The question is: how do I populate the
> > stack shadow mapping without using memblock API? It's weird, I don't
> > find anything on other architectures.
>
> @kasan: Any idea what we are doing wrong in riscv to encounter the
> above situation?

Hi Alex, Palmer,

The patch changes the definition of the KASAN_SHADOW_OFFSET const.
Does it's value change as a result or not? Have you tried to print it
before/after?
If value does not change, then this is more mysterious. If it changes,
then there lots of possible explanations (points to unmapped region,
overlaps with something), but we need to know values before/after to
answer this.


> Thanks,
>
> Alex
>
> >
> > And just a short note: I have realized this will break with the sv48
> > patchset as we decide at runtime the address space width and the kasan
> > shadow start address is different between sv39 and sv48. I will have to
> > do like x86 and move the kasan shadow start at the end of the address
> > space so that it is the same for both sv39 and sv48.
> >
> > Thanks,
> >
> > Alex
> >
> >
> > > Thanks,
> > >
> > > Alex
> > >
> > >> Cheers,
> > >> Nathan
> > >>
> > >> _______________________________________________
> > >> linux-riscv mailing list
> > >> linux-riscv@lists.infradead.org
> > >> http://lists.infradead.org/mailman/listinfo/linux-riscv
> > >>
> > >
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
  2021-10-26  4:48                 ` Dmitry Vyukov
@ 2021-10-26 11:33                   ` Alexandre Ghiti
  -1 siblings, 0 replies; 26+ messages in thread
From: Alexandre Ghiti @ 2021-10-26 11:33 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Alexandre ghiti, Nathan Chancellor, Palmer Dabbelt, elver, akpm,
	ryabinin.a.a, glider, andreyknvl, ndesaulniers, Arnd Bergmann,
	kasan-dev, linux-kernel, llvm, linux-riscv, Paul Walmsley, aou,
	linux-mm

On Tue, Oct 26, 2021 at 6:48 AM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Tue, 26 Oct 2021 at 06:39, Alexandre Ghiti
> <alexandre.ghiti@canonical.com> wrote:
> >
> > Hi,
> >
> > On Fri, Oct 15, 2021 at 3:08 PM Alexandre ghiti <alex@ghiti.fr> wrote:
> > >
> > > On 10/14/21 8:31 PM, Alex Ghiti wrote:
> > > > Hi Nathan,
> > > >
> > > > Le 14/10/2021 à 18:55, Nathan Chancellor a écrit :
> > > >> On Fri, Oct 08, 2021 at 11:46:55AM -0700, Palmer Dabbelt wrote:
> > > >>> On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
> > > >>>> On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
> > > >>>>> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor
> > > >>>>> <nathan@kernel.org> wrote:
> > > >>>>>> Currently, the asan-stack parameter is only passed along if
> > > >>>>>> CFLAGS_KASAN_SHADOW is not empty, which requires
> > > >>>>>> KASAN_SHADOW_OFFSET to
> > > >>>>>> be defined in Kconfig so that the value can be checked. In RISC-V's
> > > >>>>>> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means
> > > >>>>>> that
> > > >>>>>> asan-stack does not get disabled with clang even when
> > > >>>>>> CONFIG_KASAN_STACK
> > > >>>>>> is disabled, resulting in large stack warnings with allmodconfig:
> > > >>>>>>
> > > >>>>>> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> > > >>>>>>
> > > >>>>>> error: stack frame size (14400) exceeds limit (2048) in function
> > > >>>>>> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> > > >>>>>> static int lb035q02_connect(struct omap_dss_device *dssdev)
> > > >>>>>>             ^
> > > >>>>>> 1 error generated.
> > > >>>>>>
> > > >>>>>> Ensure that the value of CONFIG_KASAN_STACK is always passed
> > > >>>>>> along to
> > > >>>>>> the compiler so that these warnings do not happen when
> > > >>>>>> CONFIG_KASAN_STACK is disabled.
> > > >>>>>>
> > > >>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> > > >>>>>> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8
> > > >>>>>> and earlier")
> > > >>>>>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > > >>>>>
> > > >>>>> Reviewed-by: Marco Elver <elver@google.com>
> > > >>>>
> > > >>>> Thanks!
> > > >>>>
> > > >>>>> [ Which tree are you planning to take it through? ]
> > > >>>>
> > > >>>> Gah, I was intending for it to go through -mm, then I cc'd neither
> > > >>>> Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
> > > >>>> grab it from LKML?
> > > >>>
> > > >>> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
> > > >>>
> > > >>> (assuming you still want it through somewhere else)
> > > >>
> > > >> Thanks, it is now in mainline as commit 19532869feb9 ("kasan: always
> > > >> respect CONFIG_KASAN_STACK").
> > > >>
> > > >>>>> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
> > > >>>>> comment (copied from arm64). Did RISC-V just forget to copy over the
> > > >>>>> Kconfig option?
> > > >>>>
> > > >>>> I do see it defined in that file as well but you are right that
> > > >>>> they did
> > > >>>> not copy the Kconfig logic, even though it was present in the tree
> > > >>>> when
> > > >>>> RISC-V KASAN was implemented. Perhaps they should so that they get
> > > >>>> access to the other flags in the "else" branch?
> > > >>>
> > > >>> Ya, looks like we just screwed this up.  I'm seeing some warnings like
> > > >>>
> > > >>>     cc1: warning: ‘-fsanitize=kernel-address’ with stack protection
> > > >>> is not supported without ‘-fasan-shadow-offset=’ for this target
> > > >>
> > > >> Hmmm, I thought I did a GCC build with this change but I must not have
> > > >> :/
> > > >>
> > > >>> which is how I ended up here, I'm assuming that's what you're
> > > >>> talking about
> > > >>> here?  LMK if you were planning on sending along a fix or if you
> > > >>> want me to
> > > >>> go figure it out.
> > > >>
> > > >> I took a look at moving the logic into Kconfig like arm64 before sending
> > > >> this change and I did not really understand it well enough to do so. I
> > > >> think it would be best if you were able to do that so that nothing gets
> > > >> messed up.
> > > >>
> > > >
> > > > I'll do it tomorrow, I'm the last one who touched kasan on riscv :)
> > > >
> > >
> > > Adding KASAN_SHADOW_OFFSET config makes kasan kernel fails to boot.
> > > It receives a *write* fault at the beginning of a memblock_alloc
> > > function while populating the kernel shadow memory: the trap address is
> > > in the kasan shadow virtual address range and this corresponds to a
> > > kernel address in init_stack. The question is: how do I populate the
> > > stack shadow mapping without using memblock API? It's weird, I don't
> > > find anything on other architectures.
> >
> > @kasan: Any idea what we are doing wrong in riscv to encounter the
> > above situation?
>
> Hi Alex, Palmer,
>
> The patch changes the definition of the KASAN_SHADOW_OFFSET const.
> Does it's value change as a result or not? Have you tried to print it
> before/after?
> If value does not change, then this is more mysterious. If it changes,
> then there lots of possible explanations (points to unmapped region,
> overlaps with something), but we need to know values before/after to

So I debugged a bit more what happened here, and actually the culprit
is the call to kasan_populate_early_shadow at the beginning of
kasan_init which write-protects the access to kasan_early_shadow_page
and hence the write fault later when using memblock. I don't see the
point of this call anyway since we populate swapper_pg_dir in
kasan_early_init and then we write-protect the access to
kasan_early_shadow_page at the end of kasan_init.

But that may not be ideal, so I'm open to a better suggestion than
just removing the call to kasan_populate_early_shadow.

Sorry I did not dig further before asking and thanks for your time,

Alex

> answer this.
>
>
> > Thanks,
> >
> > Alex
> >
> > >
> > > And just a short note: I have realized this will break with the sv48
> > > patchset as we decide at runtime the address space width and the kasan
> > > shadow start address is different between sv39 and sv48. I will have to
> > > do like x86 and move the kasan shadow start at the end of the address
> > > space so that it is the same for both sv39 and sv48.
> > >
> > > Thanks,
> > >
> > > Alex
> > >
> > >
> > > > Thanks,
> > > >
> > > > Alex
> > > >
> > > >> Cheers,
> > > >> Nathan
> > > >>
> > > >> _______________________________________________
> > > >> linux-riscv mailing list
> > > >> linux-riscv@lists.infradead.org
> > > >> http://lists.infradead.org/mailman/listinfo/linux-riscv
> > > >>
> > > >
> > > > _______________________________________________
> > > > linux-riscv mailing list
> > > > linux-riscv@lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > >
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK
@ 2021-10-26 11:33                   ` Alexandre Ghiti
  0 siblings, 0 replies; 26+ messages in thread
From: Alexandre Ghiti @ 2021-10-26 11:33 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Alexandre ghiti, Nathan Chancellor, Palmer Dabbelt, elver, akpm,
	ryabinin.a.a, glider, andreyknvl, ndesaulniers, Arnd Bergmann,
	kasan-dev, linux-kernel, llvm, linux-riscv, Paul Walmsley, aou,
	linux-mm

On Tue, Oct 26, 2021 at 6:48 AM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Tue, 26 Oct 2021 at 06:39, Alexandre Ghiti
> <alexandre.ghiti@canonical.com> wrote:
> >
> > Hi,
> >
> > On Fri, Oct 15, 2021 at 3:08 PM Alexandre ghiti <alex@ghiti.fr> wrote:
> > >
> > > On 10/14/21 8:31 PM, Alex Ghiti wrote:
> > > > Hi Nathan,
> > > >
> > > > Le 14/10/2021 à 18:55, Nathan Chancellor a écrit :
> > > >> On Fri, Oct 08, 2021 at 11:46:55AM -0700, Palmer Dabbelt wrote:
> > > >>> On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@kernel.org wrote:
> > > >>>> On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
> > > >>>>> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor
> > > >>>>> <nathan@kernel.org> wrote:
> > > >>>>>> Currently, the asan-stack parameter is only passed along if
> > > >>>>>> CFLAGS_KASAN_SHADOW is not empty, which requires
> > > >>>>>> KASAN_SHADOW_OFFSET to
> > > >>>>>> be defined in Kconfig so that the value can be checked. In RISC-V's
> > > >>>>>> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means
> > > >>>>>> that
> > > >>>>>> asan-stack does not get disabled with clang even when
> > > >>>>>> CONFIG_KASAN_STACK
> > > >>>>>> is disabled, resulting in large stack warnings with allmodconfig:
> > > >>>>>>
> > > >>>>>> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
> > > >>>>>>
> > > >>>>>> error: stack frame size (14400) exceeds limit (2048) in function
> > > >>>>>> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
> > > >>>>>> static int lb035q02_connect(struct omap_dss_device *dssdev)
> > > >>>>>>             ^
> > > >>>>>> 1 error generated.
> > > >>>>>>
> > > >>>>>> Ensure that the value of CONFIG_KASAN_STACK is always passed
> > > >>>>>> along to
> > > >>>>>> the compiler so that these warnings do not happen when
> > > >>>>>> CONFIG_KASAN_STACK is disabled.
> > > >>>>>>
> > > >>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
> > > >>>>>> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8
> > > >>>>>> and earlier")
> > > >>>>>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > > >>>>>
> > > >>>>> Reviewed-by: Marco Elver <elver@google.com>
> > > >>>>
> > > >>>> Thanks!
> > > >>>>
> > > >>>>> [ Which tree are you planning to take it through? ]
> > > >>>>
> > > >>>> Gah, I was intending for it to go through -mm, then I cc'd neither
> > > >>>> Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
> > > >>>> grab it from LKML?
> > > >>>
> > > >>> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
> > > >>>
> > > >>> (assuming you still want it through somewhere else)
> > > >>
> > > >> Thanks, it is now in mainline as commit 19532869feb9 ("kasan: always
> > > >> respect CONFIG_KASAN_STACK").
> > > >>
> > > >>>>> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
> > > >>>>> comment (copied from arm64). Did RISC-V just forget to copy over the
> > > >>>>> Kconfig option?
> > > >>>>
> > > >>>> I do see it defined in that file as well but you are right that
> > > >>>> they did
> > > >>>> not copy the Kconfig logic, even though it was present in the tree
> > > >>>> when
> > > >>>> RISC-V KASAN was implemented. Perhaps they should so that they get
> > > >>>> access to the other flags in the "else" branch?
> > > >>>
> > > >>> Ya, looks like we just screwed this up.  I'm seeing some warnings like
> > > >>>
> > > >>>     cc1: warning: ‘-fsanitize=kernel-address’ with stack protection
> > > >>> is not supported without ‘-fasan-shadow-offset=’ for this target
> > > >>
> > > >> Hmmm, I thought I did a GCC build with this change but I must not have
> > > >> :/
> > > >>
> > > >>> which is how I ended up here, I'm assuming that's what you're
> > > >>> talking about
> > > >>> here?  LMK if you were planning on sending along a fix or if you
> > > >>> want me to
> > > >>> go figure it out.
> > > >>
> > > >> I took a look at moving the logic into Kconfig like arm64 before sending
> > > >> this change and I did not really understand it well enough to do so. I
> > > >> think it would be best if you were able to do that so that nothing gets
> > > >> messed up.
> > > >>
> > > >
> > > > I'll do it tomorrow, I'm the last one who touched kasan on riscv :)
> > > >
> > >
> > > Adding KASAN_SHADOW_OFFSET config makes kasan kernel fails to boot.
> > > It receives a *write* fault at the beginning of a memblock_alloc
> > > function while populating the kernel shadow memory: the trap address is
> > > in the kasan shadow virtual address range and this corresponds to a
> > > kernel address in init_stack. The question is: how do I populate the
> > > stack shadow mapping without using memblock API? It's weird, I don't
> > > find anything on other architectures.
> >
> > @kasan: Any idea what we are doing wrong in riscv to encounter the
> > above situation?
>
> Hi Alex, Palmer,
>
> The patch changes the definition of the KASAN_SHADOW_OFFSET const.
> Does it's value change as a result or not? Have you tried to print it
> before/after?
> If value does not change, then this is more mysterious. If it changes,
> then there lots of possible explanations (points to unmapped region,
> overlaps with something), but we need to know values before/after to

So I debugged a bit more what happened here, and actually the culprit
is the call to kasan_populate_early_shadow at the beginning of
kasan_init which write-protects the access to kasan_early_shadow_page
and hence the write fault later when using memblock. I don't see the
point of this call anyway since we populate swapper_pg_dir in
kasan_early_init and then we write-protect the access to
kasan_early_shadow_page at the end of kasan_init.

But that may not be ideal, so I'm open to a better suggestion than
just removing the call to kasan_populate_early_shadow.

Sorry I did not dig further before asking and thanks for your time,

Alex

> answer this.
>
>
> > Thanks,
> >
> > Alex
> >
> > >
> > > And just a short note: I have realized this will break with the sv48
> > > patchset as we decide at runtime the address space width and the kasan
> > > shadow start address is different between sv39 and sv48. I will have to
> > > do like x86 and move the kasan shadow start at the end of the address
> > > space so that it is the same for both sv39 and sv48.
> > >
> > > Thanks,
> > >
> > > Alex
> > >
> > >
> > > > Thanks,
> > > >
> > > > Alex
> > > >
> > > >> Cheers,
> > > >> Nathan
> > > >>
> > > >> _______________________________________________
> > > >> linux-riscv mailing list
> > > >> linux-riscv@lists.infradead.org
> > > >> http://lists.infradead.org/mailman/listinfo/linux-riscv
> > > >>
> > > >
> > > > _______________________________________________
> > > > linux-riscv mailing list
> > > > linux-riscv@lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > >
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

end of thread, other threads:[~2021-10-26 11:33 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 20:55 [PATCH] kasan: Always respect CONFIG_KASAN_STACK Nathan Chancellor
2021-09-23 10:07 ` Marco Elver
2021-09-23 10:07   ` Marco Elver
2021-09-23 10:07   ` Marco Elver
2021-09-23 14:59   ` Nathan Chancellor
2021-09-23 14:59     ` Nathan Chancellor
2021-10-08 18:46     ` Palmer Dabbelt
2021-10-08 18:46       ` Palmer Dabbelt
2021-10-14 16:55       ` Nathan Chancellor
2021-10-14 16:55         ` Nathan Chancellor
2021-10-14 18:31         ` Alex Ghiti
2021-10-14 18:31           ` Alex Ghiti
2021-10-15 13:04           ` Alexandre ghiti
2021-10-15 13:04             ` Alexandre ghiti
2021-10-26  4:39             ` Alexandre Ghiti
2021-10-26  4:39               ` Alexandre Ghiti
2021-10-26  4:48               ` Dmitry Vyukov
2021-10-26  4:48                 ` Dmitry Vyukov
2021-10-26 11:33                 ` Alexandre Ghiti
2021-10-26 11:33                   ` Alexandre Ghiti
2021-10-16  1:11           ` Palmer Dabbelt
2021-10-16  1:11             ` Palmer Dabbelt
2021-10-03 18:04 ` Andrey Konovalov
2021-10-03 18:04   ` Andrey Konovalov
2021-10-06  2:43   ` Nathan Chancellor
2021-10-06 11:57     ` Andrey Konovalov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.