Re: [PATCH -tip 1/2] Kconfig: Bump required compiler version of KASAN and UBSAN
On Wed, 3 Jun 2020 at 15:35, 'Andrey Konovalov' via kasan-dev wrote: > > On Tue, Jun 2, 2020 at 8:44 PM Marco Elver wrote: > > > > Adds config variable CC_HAS_WORKING_NOSANITIZE, which will be true if we > > have a compiler that does not fail builds due to no_sanitize functions. > > This does not yet mean they work as intended, but for automated > > build-tests, this is the minimum requirement. > > > > For example, we require that __always_inline functions used from > > no_sanitize functions do not generate instrumentation. On GCC <= 7 this > > fails to build entirely, therefore we make the minimum version GCC 8. > > > > For KCSAN this is a non-functional change, however, we should add it in > > case this variable changes in future. > > > > Link: > > https://lkml.kernel.org/r/20200602175859.gc2...@hirez.programming.kicks-ass.net > > Suggested-by: Peter Zijlstra > > Signed-off-by: Marco Elver > > Acked-by: Andrey Konovalov I've sent a v2 of this, which limits the compiler-bump to KASAN only. It appears no_sanitize_undefined (for UBSAN) is not broken GCC <= 7, and in general the no_sanitize attributes seem to behave differently from sanitizer to sanitizer as we discovered for UBSAN. https://lkml.kernel.org/r/20200604055811.247298-1-el...@google.com
Re: [PATCH -tip 1/2] Kconfig: Bump required compiler version of KASAN and UBSAN
On Tue, Jun 2, 2020 at 8:44 PM Marco Elver wrote:
>
> Adds config variable CC_HAS_WORKING_NOSANITIZE, which will be true if we
> have a compiler that does not fail builds due to no_sanitize functions.
> This does not yet mean they work as intended, but for automated
> build-tests, this is the minimum requirement.
>
> For example, we require that __always_inline functions used from
> no_sanitize functions do not generate instrumentation. On GCC <= 7 this
> fails to build entirely, therefore we make the minimum version GCC 8.
>
> For KCSAN this is a non-functional change, however, we should add it in
> case this variable changes in future.
>
> Link:
> https://lkml.kernel.org/r/20200602175859.gc2...@hirez.programming.kicks-ass.net
> Suggested-by: Peter Zijlstra
> Signed-off-by: Marco Elver
Acked-by: Andrey Konovalov
> ---
> Apply after:
> https://lkml.kernel.org/r/20200602173103.931412...@infradead.org
> ---
> init/Kconfig | 3 +++
> lib/Kconfig.kasan | 1 +
> lib/Kconfig.kcsan | 1 +
> lib/Kconfig.ubsan | 1 +
> 4 files changed, 6 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 0f72eb4ffc87..3e8565bc8376 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -39,6 +39,9 @@ config TOOLS_SUPPORT_RELR
> config CC_HAS_ASM_INLINE
> def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC)
> -x c - -c -o /dev/null)
>
> +config CC_HAS_WORKING_NOSANITIZE
> + def_bool !CC_IS_GCC || GCC_VERSION >= 8
> +
> config CONSTRUCTORS
> bool
> depends on !UML
> diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
> index 81f5464ea9e1..15e6c4b26a40 100644
> --- a/lib/Kconfig.kasan
> +++ b/lib/Kconfig.kasan
> @@ -20,6 +20,7 @@ config KASAN
> depends on (HAVE_ARCH_KASAN && CC_HAS_KASAN_GENERIC) || \
>(HAVE_ARCH_KASAN_SW_TAGS && CC_HAS_KASAN_SW_TAGS)
> depends on (SLUB && SYSFS) || (SLAB && !DEBUG_SLAB)
> + depends on CC_HAS_WORKING_NOSANITIZE
> help
> Enables KASAN (KernelAddressSANitizer) - runtime memory debugger,
> designed to find out-of-bounds accesses and use-after-free bugs.
> diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> index 5ee88e5119c2..2ab4a7f511c9 100644
> --- a/lib/Kconfig.kcsan
> +++ b/lib/Kconfig.kcsan
> @@ -5,6 +5,7 @@ config HAVE_ARCH_KCSAN
>
> config HAVE_KCSAN_COMPILER
> def_bool CC_IS_CLANG && $(cc-option,-fsanitize=thread -mllvm
> -tsan-distinguish-volatile=1)
> + depends on CC_HAS_WORKING_NOSANITIZE
> help
> For the list of compilers that support KCSAN, please see
> .
> diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
> index a5ba2fd51823..f725d126af7d 100644
> --- a/lib/Kconfig.ubsan
> +++ b/lib/Kconfig.ubsan
> @@ -4,6 +4,7 @@ config ARCH_HAS_UBSAN_SANITIZE_ALL
>
> menuconfig UBSAN
> bool "Undefined behaviour sanity checker"
> + depends on CC_HAS_WORKING_NOSANITIZE
> help
> This option enables the Undefined Behaviour sanity checker.
> Compile-time instrumentation is used to detect various undefined
> --
> 2.27.0.rc2.251.g90737beb825-goog
>
Re: [PATCH -tip 1/2] Kconfig: Bump required compiler version of KASAN and UBSAN
On Tue, Jun 2, 2020 at 12:38 PM Peter Zijlstra wrote:
>
> On Tue, Jun 02, 2020 at 09:25:47PM +0200, Marco Elver wrote:
> > On Tue, 2 Jun 2020 at 21:19, Peter Zijlstra wrote:
>
> > > Currently x86 only, but I know other arch maintainers are planning to
> > > have a hard look at their code based on our findings.
> >
> > I've already spotted a bunch of 'noinstr' outside arch/x86 e.g. in
> > kernel/{locking,rcu}, and a bunch of these functions use atomic_*, all
> > of which are __always_inline. The noinstr uses outside arch/x86 would
> > break builds on all architecture with GCC <= 7 when using sanitizers.
> > At least that's what led me to conclude we need this for all
> > architectures.
>
> True; but !x86 could, probably, get away with not fully respecting
> noinstr at this time. But that'd make a mess of things again, so my
> preference is as you did, unilaterally raise the min version for *SAN.
Fair, thought I'd ask. (I prefer people use newer
hopefully-less-buggier-but-maybe-not-really-suprise-they're-actually-worse
tools anyways)
Reviewed-by: Nick Desaulniers
---
Thanks,
~Nick Desaulniers
Re: [PATCH -tip 1/2] Kconfig: Bump required compiler version of KASAN and UBSAN
On Tue, Jun 02, 2020 at 09:25:47PM +0200, Marco Elver wrote:
> On Tue, 2 Jun 2020 at 21:19, Peter Zijlstra wrote:
> > Currently x86 only, but I know other arch maintainers are planning to
> > have a hard look at their code based on our findings.
>
> I've already spotted a bunch of 'noinstr' outside arch/x86 e.g. in
> kernel/{locking,rcu}, and a bunch of these functions use atomic_*, all
> of which are __always_inline. The noinstr uses outside arch/x86 would
> break builds on all architecture with GCC <= 7 when using sanitizers.
> At least that's what led me to conclude we need this for all
> architectures.
True; but !x86 could, probably, get away with not fully respecting
noinstr at this time. But that'd make a mess of things again, so my
preference is as you did, unilaterally raise the min version for *SAN.
That said; noinstr's __no_sanitize combined with atomic_t might be
'interesting', because the regular atomic things have explicit
annotations in them. That should give validation warnings for the right
.config, I'll have to go try -- so far I've made sure to never enable
the *SAN stuff.
Re: [PATCH -tip 1/2] Kconfig: Bump required compiler version of KASAN and UBSAN
On Tue, 2 Jun 2020 at 21:19, Peter Zijlstra wrote:
>
> On Tue, Jun 02, 2020 at 11:57:15AM -0700, Nick Desaulniers wrote:
> > On Tue, Jun 2, 2020 at 11:44 AM 'Marco Elver' via Clang Built Linux
> > wrote:
> > >
> > > Adds config variable CC_HAS_WORKING_NOSANITIZE, which will be true if we
> > > have a compiler that does not fail builds due to no_sanitize functions.
> > > This does not yet mean they work as intended, but for automated
> > > build-tests, this is the minimum requirement.
> > >
> > > For example, we require that __always_inline functions used from
> > > no_sanitize functions do not generate instrumentation. On GCC <= 7 this
> > > fails to build entirely, therefore we make the minimum version GCC 8.
> > >
> > > For KCSAN this is a non-functional change, however, we should add it in
> > > case this variable changes in future.
> > >
> > > Link:
> > > https://lkml.kernel.org/r/20200602175859.gc2...@hirez.programming.kicks-ass.net
> > > Suggested-by: Peter Zijlstra
> > > Signed-off-by: Marco Elver
> >
> > Is this a problem only for x86? If so, that's quite a jump in minimal
> > compiler versions for a feature that I don't think is currently
> > problematic for other architectures? (Based on
> > https://lore.kernel.org/lkml/20200529171104.gd706...@hirez.programming.kicks-ass.net/
> > )
>
> Currently x86 only, but I know other arch maintainers are planning to
> have a hard look at their code based on our findings.
I've already spotted a bunch of 'noinstr' outside arch/x86 e.g. in
kernel/{locking,rcu}, and a bunch of these functions use atomic_*, all
of which are __always_inline. The noinstr uses outside arch/x86 would
break builds on all architecture with GCC <= 7 when using sanitizers.
At least that's what led me to conclude we need this for all
architectures.
Thanks,
-- Marco
Re: [PATCH -tip 1/2] Kconfig: Bump required compiler version of KASAN and UBSAN
On Tue, Jun 02, 2020 at 11:57:15AM -0700, Nick Desaulniers wrote: > On Tue, Jun 2, 2020 at 11:44 AM 'Marco Elver' via Clang Built Linux > wrote: > > > > Adds config variable CC_HAS_WORKING_NOSANITIZE, which will be true if we > > have a compiler that does not fail builds due to no_sanitize functions. > > This does not yet mean they work as intended, but for automated > > build-tests, this is the minimum requirement. > > > > For example, we require that __always_inline functions used from > > no_sanitize functions do not generate instrumentation. On GCC <= 7 this > > fails to build entirely, therefore we make the minimum version GCC 8. > > > > For KCSAN this is a non-functional change, however, we should add it in > > case this variable changes in future. > > > > Link: > > https://lkml.kernel.org/r/20200602175859.gc2...@hirez.programming.kicks-ass.net > > Suggested-by: Peter Zijlstra > > Signed-off-by: Marco Elver > > Is this a problem only for x86? If so, that's quite a jump in minimal > compiler versions for a feature that I don't think is currently > problematic for other architectures? (Based on > https://lore.kernel.org/lkml/20200529171104.gd706...@hirez.programming.kicks-ass.net/ > ) Currently x86 only, but I know other arch maintainers are planning to have a hard look at their code based on our findings.
Re: [PATCH -tip 1/2] Kconfig: Bump required compiler version of KASAN and UBSAN
On Tue, Jun 2, 2020 at 9:07 PM Marco Elver wrote: > > On Tue, 2 Jun 2020 at 20:53, Andrey Konovalov wrote: > > > > On Tue, Jun 2, 2020 at 8:44 PM Marco Elver wrote: > > > > > > Adds config variable CC_HAS_WORKING_NOSANITIZE, which will be true if we > > > have a compiler that does not fail builds due to no_sanitize functions. > > > This does not yet mean they work as intended, but for automated > > > build-tests, this is the minimum requirement. > > > > > > For example, we require that __always_inline functions used from > > > no_sanitize functions do not generate instrumentation. On GCC <= 7 this > > > fails to build entirely, therefore we make the minimum version GCC 8. > > > > Could you also update KASAN docs to mention this requirement? As a > > separate patch or in v2, up to you. > > I can do a v2 tomorrow. But all this is once again tangled up with > KCSAN, so I was hoping to keep changes minimal. ;-) OK, we can do a separate patch after all this is merged.
Re: [PATCH -tip 1/2] Kconfig: Bump required compiler version of KASAN and UBSAN
On Tue, 2 Jun 2020 at 20:53, Andrey Konovalov wrote: > > On Tue, Jun 2, 2020 at 8:44 PM Marco Elver wrote: > > > > Adds config variable CC_HAS_WORKING_NOSANITIZE, which will be true if we > > have a compiler that does not fail builds due to no_sanitize functions. > > This does not yet mean they work as intended, but for automated > > build-tests, this is the minimum requirement. > > > > For example, we require that __always_inline functions used from > > no_sanitize functions do not generate instrumentation. On GCC <= 7 this > > fails to build entirely, therefore we make the minimum version GCC 8. > > Could you also update KASAN docs to mention this requirement? As a > separate patch or in v2, up to you. I can do a v2 tomorrow. But all this is once again tangled up with KCSAN, so I was hoping to keep changes minimal. ;-) Thanks, -- Marco
Re: [PATCH -tip 1/2] Kconfig: Bump required compiler version of KASAN and UBSAN
On Tue, 2 Jun 2020 at 20:57, Nick Desaulniers wrote:
>
> On Tue, Jun 2, 2020 at 11:44 AM 'Marco Elver' via Clang Built Linux
> wrote:
> >
> > Adds config variable CC_HAS_WORKING_NOSANITIZE, which will be true if we
> > have a compiler that does not fail builds due to no_sanitize functions.
> > This does not yet mean they work as intended, but for automated
> > build-tests, this is the minimum requirement.
> >
> > For example, we require that __always_inline functions used from
> > no_sanitize functions do not generate instrumentation. On GCC <= 7 this
> > fails to build entirely, therefore we make the minimum version GCC 8.
> >
> > For KCSAN this is a non-functional change, however, we should add it in
> > case this variable changes in future.
> >
> > Link:
> > https://lkml.kernel.org/r/20200602175859.gc2...@hirez.programming.kicks-ass.net
> > Suggested-by: Peter Zijlstra
> > Signed-off-by: Marco Elver
>
> Is this a problem only for x86? If so, that's quite a jump in minimal
> compiler versions for a feature that I don't think is currently
> problematic for other architectures? (Based on
> https://lore.kernel.org/lkml/20200529171104.gd706...@hirez.programming.kicks-ass.net/
> )
__always_inline void foo(void) {}
__no_sanitize_address void bar(void) { foo(); }
where __no_sanitize_address is implied by 'noinstr' now, and 'noinstr'
is no longer just x86.
Therefore, it's broken on *all* architectures. The compiler will just
break the build with an error. I don't think we can fix that.
Thanks,
-- Marco
Re: [PATCH -tip 1/2] Kconfig: Bump required compiler version of KASAN and UBSAN
On Tue, Jun 2, 2020 at 11:44 AM 'Marco Elver' via Clang Built Linux
wrote:
>
> Adds config variable CC_HAS_WORKING_NOSANITIZE, which will be true if we
> have a compiler that does not fail builds due to no_sanitize functions.
> This does not yet mean they work as intended, but for automated
> build-tests, this is the minimum requirement.
>
> For example, we require that __always_inline functions used from
> no_sanitize functions do not generate instrumentation. On GCC <= 7 this
> fails to build entirely, therefore we make the minimum version GCC 8.
>
> For KCSAN this is a non-functional change, however, we should add it in
> case this variable changes in future.
>
> Link:
> https://lkml.kernel.org/r/20200602175859.gc2...@hirez.programming.kicks-ass.net
> Suggested-by: Peter Zijlstra
> Signed-off-by: Marco Elver
Is this a problem only for x86? If so, that's quite a jump in minimal
compiler versions for a feature that I don't think is currently
problematic for other architectures? (Based on
https://lore.kernel.org/lkml/20200529171104.gd706...@hirez.programming.kicks-ass.net/
)
> ---
> Apply after:
> https://lkml.kernel.org/r/20200602173103.931412...@infradead.org
> ---
> init/Kconfig | 3 +++
> lib/Kconfig.kasan | 1 +
> lib/Kconfig.kcsan | 1 +
> lib/Kconfig.ubsan | 1 +
> 4 files changed, 6 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 0f72eb4ffc87..3e8565bc8376 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -39,6 +39,9 @@ config TOOLS_SUPPORT_RELR
> config CC_HAS_ASM_INLINE
> def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC)
> -x c - -c -o /dev/null)
>
> +config CC_HAS_WORKING_NOSANITIZE
> + def_bool !CC_IS_GCC || GCC_VERSION >= 8
> +
> config CONSTRUCTORS
> bool
> depends on !UML
> diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
> index 81f5464ea9e1..15e6c4b26a40 100644
> --- a/lib/Kconfig.kasan
> +++ b/lib/Kconfig.kasan
> @@ -20,6 +20,7 @@ config KASAN
> depends on (HAVE_ARCH_KASAN && CC_HAS_KASAN_GENERIC) || \
>(HAVE_ARCH_KASAN_SW_TAGS && CC_HAS_KASAN_SW_TAGS)
> depends on (SLUB && SYSFS) || (SLAB && !DEBUG_SLAB)
> + depends on CC_HAS_WORKING_NOSANITIZE
> help
> Enables KASAN (KernelAddressSANitizer) - runtime memory debugger,
> designed to find out-of-bounds accesses and use-after-free bugs.
> diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> index 5ee88e5119c2..2ab4a7f511c9 100644
> --- a/lib/Kconfig.kcsan
> +++ b/lib/Kconfig.kcsan
> @@ -5,6 +5,7 @@ config HAVE_ARCH_KCSAN
>
> config HAVE_KCSAN_COMPILER
> def_bool CC_IS_CLANG && $(cc-option,-fsanitize=thread -mllvm
> -tsan-distinguish-volatile=1)
> + depends on CC_HAS_WORKING_NOSANITIZE
> help
> For the list of compilers that support KCSAN, please see
> .
> diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
> index a5ba2fd51823..f725d126af7d 100644
> --- a/lib/Kconfig.ubsan
> +++ b/lib/Kconfig.ubsan
> @@ -4,6 +4,7 @@ config ARCH_HAS_UBSAN_SANITIZE_ALL
>
> menuconfig UBSAN
> bool "Undefined behaviour sanity checker"
> + depends on CC_HAS_WORKING_NOSANITIZE
> help
> This option enables the Undefined Behaviour sanity checker.
> Compile-time instrumentation is used to detect various undefined
> --
--
Thanks,
~Nick Desaulniers
Re: [PATCH -tip 1/2] Kconfig: Bump required compiler version of KASAN and UBSAN
On Tue, Jun 2, 2020 at 8:44 PM Marco Elver wrote:
>
> Adds config variable CC_HAS_WORKING_NOSANITIZE, which will be true if we
> have a compiler that does not fail builds due to no_sanitize functions.
> This does not yet mean they work as intended, but for automated
> build-tests, this is the minimum requirement.
>
> For example, we require that __always_inline functions used from
> no_sanitize functions do not generate instrumentation. On GCC <= 7 this
> fails to build entirely, therefore we make the minimum version GCC 8.
Could you also update KASAN docs to mention this requirement? As a
separate patch or in v2, up to you.
>
> For KCSAN this is a non-functional change, however, we should add it in
> case this variable changes in future.
>
> Link:
> https://lkml.kernel.org/r/20200602175859.gc2...@hirez.programming.kicks-ass.net
> Suggested-by: Peter Zijlstra
> Signed-off-by: Marco Elver
> ---
> Apply after:
> https://lkml.kernel.org/r/20200602173103.931412...@infradead.org
> ---
> init/Kconfig | 3 +++
> lib/Kconfig.kasan | 1 +
> lib/Kconfig.kcsan | 1 +
> lib/Kconfig.ubsan | 1 +
> 4 files changed, 6 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 0f72eb4ffc87..3e8565bc8376 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -39,6 +39,9 @@ config TOOLS_SUPPORT_RELR
> config CC_HAS_ASM_INLINE
> def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC)
> -x c - -c -o /dev/null)
>
> +config CC_HAS_WORKING_NOSANITIZE
> + def_bool !CC_IS_GCC || GCC_VERSION >= 8
> +
> config CONSTRUCTORS
> bool
> depends on !UML
> diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
> index 81f5464ea9e1..15e6c4b26a40 100644
> --- a/lib/Kconfig.kasan
> +++ b/lib/Kconfig.kasan
> @@ -20,6 +20,7 @@ config KASAN
> depends on (HAVE_ARCH_KASAN && CC_HAS_KASAN_GENERIC) || \
>(HAVE_ARCH_KASAN_SW_TAGS && CC_HAS_KASAN_SW_TAGS)
> depends on (SLUB && SYSFS) || (SLAB && !DEBUG_SLAB)
> + depends on CC_HAS_WORKING_NOSANITIZE
> help
> Enables KASAN (KernelAddressSANitizer) - runtime memory debugger,
> designed to find out-of-bounds accesses and use-after-free bugs.
> diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> index 5ee88e5119c2..2ab4a7f511c9 100644
> --- a/lib/Kconfig.kcsan
> +++ b/lib/Kconfig.kcsan
> @@ -5,6 +5,7 @@ config HAVE_ARCH_KCSAN
>
> config HAVE_KCSAN_COMPILER
> def_bool CC_IS_CLANG && $(cc-option,-fsanitize=thread -mllvm
> -tsan-distinguish-volatile=1)
> + depends on CC_HAS_WORKING_NOSANITIZE
> help
> For the list of compilers that support KCSAN, please see
> .
> diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
> index a5ba2fd51823..f725d126af7d 100644
> --- a/lib/Kconfig.ubsan
> +++ b/lib/Kconfig.ubsan
> @@ -4,6 +4,7 @@ config ARCH_HAS_UBSAN_SANITIZE_ALL
>
> menuconfig UBSAN
> bool "Undefined behaviour sanity checker"
> + depends on CC_HAS_WORKING_NOSANITIZE
> help
> This option enables the Undefined Behaviour sanity checker.
> Compile-time instrumentation is used to detect various undefined
> --
> 2.27.0.rc2.251.g90737beb825-goog
>
