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/[email protected]
Suggested-by: Peter Zijlstra <[email protected]>
Signed-off-by: Marco Elver <[email protected]>
---
Apply after:
https://lkml.kernel.org/r/[email protected]
---
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 >= 80000
+
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
<file:Documentation/dev-tools/kcsan.rst>.
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
Adds the portable definitions for __no_sanitize_address, and
__no_sanitize_undefined, and subsequently changes noinstr to use the
attributes to disable instrumentation via KASAN or UBSAN.
Link: https://lore.kernel.org/lkml/[email protected]/
Reported-by: [email protected]
Signed-off-by: Marco Elver <[email protected]>
---
Note: __no_sanitize_coverage (for KCOV) isn't possible right now,
because neither GCC nor Clang support such an attribute. This means
going and changing the compilers again (for Clang it's fine, for GCC,
it'll take a while).
However, it looks like that KCOV_INSTRUMENT := n is currently in all the
right places. Short-term, this should be reasonable.
---
include/linux/compiler-clang.h | 8 ++++++++
include/linux/compiler-gcc.h | 6 ++++++
include/linux/compiler_types.h | 3 ++-
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index 2cb42d8bdedc..c0e4b193b311 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -33,6 +33,14 @@
#define __no_sanitize_thread
#endif
+#if __has_feature(undefined_behavior_sanitizer)
+/* GCC does not have __SANITIZE_UNDEFINED__ */
+#define __no_sanitize_undefined \
+ __attribute__((no_sanitize("undefined")))
+#else
+#define __no_sanitize_undefined
+#endif
+
/*
* Not all versions of clang implement the the type-generic versions
* of the builtin overflow checkers. Fortunately, clang implements
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 7dd4e0349ef3..1c74464c80c6 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -150,6 +150,12 @@
#define __no_sanitize_thread
#endif
+#if __has_attribute(__no_sanitize_undefined__)
+#define __no_sanitize_undefined __attribute__((no_sanitize_undefined))
+#else
+#define __no_sanitize_undefined
+#endif
+
#if GCC_VERSION >= 50100
#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
#endif
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 02becd21d456..89b8c1ae18a1 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -198,7 +198,8 @@ struct ftrace_likely_data {
/* Section for code which can't be instrumented at all */
#define noinstr \
- noinline notrace __attribute((__section__(".noinstr.text"))) __no_kcsan
+ noinline notrace __attribute((__section__(".noinstr.text"))) \
+ __no_kcsan __no_sanitize_address __no_sanitize_undefined
#endif /* __KERNEL__ */
--
2.27.0.rc2.251.g90737beb825-goog
On Tue, Jun 2, 2020 at 11:44 AM 'Marco Elver' via Clang Built Linux
<[email protected]> wrote:
>
> Adds the portable definitions for __no_sanitize_address, and
> __no_sanitize_undefined, and subsequently changes noinstr to use the
> attributes to disable instrumentation via KASAN or UBSAN.
>
> Link: https://lore.kernel.org/lkml/[email protected]/
> Reported-by: [email protected]
> Signed-off-by: Marco Elver <[email protected]>
Currently most of our compiler attribute detection is done in
include/linux/compiler_attributes.h; I think this should be handled
there. +Miguel Ojeda
> ---
>
> Note: __no_sanitize_coverage (for KCOV) isn't possible right now,
> because neither GCC nor Clang support such an attribute. This means
> going and changing the compilers again (for Clang it's fine, for GCC,
> it'll take a while).
>
> However, it looks like that KCOV_INSTRUMENT := n is currently in all the
> right places. Short-term, this should be reasonable.
> ---
> include/linux/compiler-clang.h | 8 ++++++++
> include/linux/compiler-gcc.h | 6 ++++++
> include/linux/compiler_types.h | 3 ++-
> 3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index 2cb42d8bdedc..c0e4b193b311 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -33,6 +33,14 @@
> #define __no_sanitize_thread
> #endif
>
> +#if __has_feature(undefined_behavior_sanitizer)
> +/* GCC does not have __SANITIZE_UNDEFINED__ */
> +#define __no_sanitize_undefined \
> + __attribute__((no_sanitize("undefined")))
> +#else
> +#define __no_sanitize_undefined
> +#endif
> +
> /*
> * Not all versions of clang implement the the type-generic versions
> * of the builtin overflow checkers. Fortunately, clang implements
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 7dd4e0349ef3..1c74464c80c6 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -150,6 +150,12 @@
> #define __no_sanitize_thread
> #endif
>
> +#if __has_attribute(__no_sanitize_undefined__)
> +#define __no_sanitize_undefined __attribute__((no_sanitize_undefined))
> +#else
> +#define __no_sanitize_undefined
> +#endif
> +
> #if GCC_VERSION >= 50100
> #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
> #endif
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 02becd21d456..89b8c1ae18a1 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -198,7 +198,8 @@ struct ftrace_likely_data {
>
> /* Section for code which can't be instrumented at all */
> #define noinstr \
> - noinline notrace __attribute((__section__(".noinstr.text"))) __no_kcsan
> + noinline notrace __attribute((__section__(".noinstr.text"))) \
> + __no_kcsan __no_sanitize_address __no_sanitize_undefined
>
> #endif /* __KERNEL__ */
>
> --
--
Thanks,
~Nick Desaulniers
On Tue, Jun 2, 2020 at 8:44 PM Marco Elver <[email protected]> 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/[email protected]
> Suggested-by: Peter Zijlstra <[email protected]>
> Signed-off-by: Marco Elver <[email protected]>
> ---
> Apply after:
> https://lkml.kernel.org/r/[email protected]
> ---
> 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 >= 80000
> +
> 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
> <file:Documentation/dev-tools/kcsan.rst>.
> 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
>
On Tue, Jun 2, 2020 at 11:44 AM 'Marco Elver' via Clang Built Linux
<[email protected]> 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/[email protected]
> Suggested-by: Peter Zijlstra <[email protected]>
> Signed-off-by: Marco Elver <[email protected]>
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/[email protected]/
)
> ---
> Apply after:
> https://lkml.kernel.org/r/[email protected]
> ---
> 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 >= 80000
> +
> 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
> <file:Documentation/dev-tools/kcsan.rst>.
> 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
On Tue, 2 Jun 2020 at 20:49, 'Nick Desaulniers' via kasan-dev
<[email protected]> wrote:
>
> On Tue, Jun 2, 2020 at 11:44 AM 'Marco Elver' via Clang Built Linux
> <[email protected]> wrote:
> >
> > Adds the portable definitions for __no_sanitize_address, and
> > __no_sanitize_undefined, and subsequently changes noinstr to use the
> > attributes to disable instrumentation via KASAN or UBSAN.
> >
> > Link: https://lore.kernel.org/lkml/[email protected]/
> > Reported-by: [email protected]
> > Signed-off-by: Marco Elver <[email protected]>
>
> Currently most of our compiler attribute detection is done in
> include/linux/compiler_attributes.h; I think this should be handled
> there. +Miguel Ojeda
GCC and Clang define these very differently, and the way to query for
them is different too. All we want is a portable __no_sanitize, and
compiler-{gcc,clang}.h is the right place for that. Similar to why we
define the other __no_sanitize above the places they were added.
Thanks,
-- Marco
On Tue, 2 Jun 2020 at 20:57, Nick Desaulniers <[email protected]> wrote:
>
> On Tue, Jun 2, 2020 at 11:44 AM 'Marco Elver' via Clang Built Linux
> <[email protected]> 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/[email protected]
> > Suggested-by: Peter Zijlstra <[email protected]>
> > Signed-off-by: Marco Elver <[email protected]>
>
> 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/[email protected]/
> )
__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
On Tue, 2 Jun 2020 at 20:53, Andrey Konovalov <[email protected]> wrote:
>
> On Tue, Jun 2, 2020 at 8:44 PM Marco Elver <[email protected]> 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
On Tue, Jun 2, 2020 at 9:07 PM Marco Elver <[email protected]> wrote:
>
> On Tue, 2 Jun 2020 at 20:53, Andrey Konovalov <[email protected]> wrote:
> >
> > On Tue, Jun 2, 2020 at 8:44 PM Marco Elver <[email protected]> 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.
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
> <[email protected]> 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/[email protected]
> > Suggested-by: Peter Zijlstra <[email protected]>
> > Signed-off-by: Marco Elver <[email protected]>
>
> 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/[email protected]/
> )
Currently x86 only, but I know other arch maintainers are planning to
have a hard look at their code based on our findings.
On Tue, 2 Jun 2020 at 21:19, Peter Zijlstra <[email protected]> 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
> > <[email protected]> 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/[email protected]
> > > Suggested-by: Peter Zijlstra <[email protected]>
> > > Signed-off-by: Marco Elver <[email protected]>
> >
> > 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/[email protected]/
> > )
>
> 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
On Tue, Jun 02, 2020 at 09:25:47PM +0200, Marco Elver wrote:
> On Tue, 2 Jun 2020 at 21:19, Peter Zijlstra <[email protected]> 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.
On Tue, Jun 2, 2020 at 12:38 PM Peter Zijlstra <[email protected]> wrote:
>
> On Tue, Jun 02, 2020 at 09:25:47PM +0200, Marco Elver wrote:
> > On Tue, 2 Jun 2020 at 21:19, Peter Zijlstra <[email protected]> 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 <[email protected]>
---
Thanks,
~Nick Desaulniers
On Tue, Jun 02, 2020 at 09:38:53PM +0200, Peter Zijlstra wrote:
> 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.
---
Subject: rcu: Fixup noinstr warnings
A KCSAN build revealed we have explicit annoations through atomic_t
usage, switch to arch_atomic_*() for the respective functions.
vmlinux.o: warning: objtool: rcu_nmi_exit()+0x4d: call to __kcsan_check_access() leaves .noinstr.text section
vmlinux.o: warning: objtool: rcu_dynticks_eqs_enter()+0x25: call to __kcsan_check_access() leaves .noinstr.text section
vmlinux.o: warning: objtool: rcu_nmi_enter()+0x4f: call to __kcsan_check_access() leaves .noinstr.text section
vmlinux.o: warning: objtool: rcu_dynticks_eqs_exit()+0x2a: call to __kcsan_check_access() leaves .noinstr.text section
vmlinux.o: warning: objtool: __rcu_is_watching()+0x25: call to __kcsan_check_access() leaves .noinstr.text section
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
kernel/rcu/tree.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index c716eadc7617..162656b80db9 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -250,7 +250,7 @@ static noinstr void rcu_dynticks_eqs_enter(void)
* next idle sojourn.
*/
rcu_dynticks_task_trace_enter(); // Before ->dynticks update!
- seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, &rdp->dynticks);
+ seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, &rdp->dynticks);
// RCU is no longer watching. Better be in extended quiescent state!
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
(seq & RCU_DYNTICK_CTRL_CTR));
@@ -274,13 +274,13 @@ static noinstr void rcu_dynticks_eqs_exit(void)
* and we also must force ordering with the next RCU read-side
* critical section.
*/
- seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, &rdp->dynticks);
+ seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, &rdp->dynticks);
// RCU is now watching. Better not be in an extended quiescent state!
rcu_dynticks_task_trace_exit(); // After ->dynticks update!
WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
!(seq & RCU_DYNTICK_CTRL_CTR));
if (seq & RCU_DYNTICK_CTRL_MASK) {
- atomic_andnot(RCU_DYNTICK_CTRL_MASK, &rdp->dynticks);
+ arch_atomic_andnot(RCU_DYNTICK_CTRL_MASK, &rdp->dynticks);
smp_mb__after_atomic(); /* _exit after clearing mask. */
}
}
@@ -313,7 +313,7 @@ static __always_inline bool rcu_dynticks_curr_cpu_in_eqs(void)
{
struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
- return !(atomic_read(&rdp->dynticks) & RCU_DYNTICK_CTRL_CTR);
+ return !(arch_atomic_read(&rdp->dynticks) & RCU_DYNTICK_CTRL_CTR);
}
/*
@@ -692,6 +692,7 @@ noinstr void rcu_nmi_exit(void)
{
struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
+ instrumentation_begin();
/*
* Check for ->dynticks_nmi_nesting underflow and bad ->dynticks.
* (We are exiting an NMI handler, so RCU better be paying attention
@@ -705,7 +706,6 @@ noinstr void rcu_nmi_exit(void)
* leave it in non-RCU-idle state.
*/
if (rdp->dynticks_nmi_nesting != 1) {
- instrumentation_begin();
trace_rcu_dyntick(TPS("--="), rdp->dynticks_nmi_nesting, rdp->dynticks_nmi_nesting - 2,
atomic_read(&rdp->dynticks));
WRITE_ONCE(rdp->dynticks_nmi_nesting, /* No store tearing. */
@@ -714,7 +714,6 @@ noinstr void rcu_nmi_exit(void)
return;
}
- instrumentation_begin();
/* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */
trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nmi_nesting, 0, atomic_read(&rdp->dynticks));
WRITE_ONCE(rdp->dynticks_nmi_nesting, 0); /* Avoid store tearing. */
On Wed, Jun 03, 2020 at 10:48:18AM +0200, Peter Zijlstra wrote:
> On Tue, Jun 02, 2020 at 09:38:53PM +0200, Peter Zijlstra wrote:
>
> > 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.
>
> ---
> Subject: rcu: Fixup noinstr warnings
>
> A KCSAN build revealed we have explicit annoations through atomic_t
> usage, switch to arch_atomic_*() for the respective functions.
>
> vmlinux.o: warning: objtool: rcu_nmi_exit()+0x4d: call to __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: rcu_dynticks_eqs_enter()+0x25: call to __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: rcu_nmi_enter()+0x4f: call to __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: rcu_dynticks_eqs_exit()+0x2a: call to __kcsan_check_access() leaves .noinstr.text section
> vmlinux.o: warning: objtool: __rcu_is_watching()+0x25: call to __kcsan_check_access() leaves .noinstr.text section
>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
This one does not apply cleanly onto the -rcu tree's "dev" branch, so
I am guessing that it is intended to be carried in -tip with yours and
Thomas's patch series.
If you do instead want this in -rcu, please let me know.
Thanx, Paul
> ---
> kernel/rcu/tree.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index c716eadc7617..162656b80db9 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -250,7 +250,7 @@ static noinstr void rcu_dynticks_eqs_enter(void)
> * next idle sojourn.
> */
> rcu_dynticks_task_trace_enter(); // Before ->dynticks update!
> - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, &rdp->dynticks);
> + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, &rdp->dynticks);
> // RCU is no longer watching. Better be in extended quiescent state!
> WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> (seq & RCU_DYNTICK_CTRL_CTR));
> @@ -274,13 +274,13 @@ static noinstr void rcu_dynticks_eqs_exit(void)
> * and we also must force ordering with the next RCU read-side
> * critical section.
> */
> - seq = atomic_add_return(RCU_DYNTICK_CTRL_CTR, &rdp->dynticks);
> + seq = arch_atomic_add_return(RCU_DYNTICK_CTRL_CTR, &rdp->dynticks);
> // RCU is now watching. Better not be in an extended quiescent state!
> rcu_dynticks_task_trace_exit(); // After ->dynticks update!
> WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
> !(seq & RCU_DYNTICK_CTRL_CTR));
> if (seq & RCU_DYNTICK_CTRL_MASK) {
> - atomic_andnot(RCU_DYNTICK_CTRL_MASK, &rdp->dynticks);
> + arch_atomic_andnot(RCU_DYNTICK_CTRL_MASK, &rdp->dynticks);
> smp_mb__after_atomic(); /* _exit after clearing mask. */
> }
> }
> @@ -313,7 +313,7 @@ static __always_inline bool rcu_dynticks_curr_cpu_in_eqs(void)
> {
> struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
>
> - return !(atomic_read(&rdp->dynticks) & RCU_DYNTICK_CTRL_CTR);
> + return !(arch_atomic_read(&rdp->dynticks) & RCU_DYNTICK_CTRL_CTR);
> }
>
> /*
> @@ -692,6 +692,7 @@ noinstr void rcu_nmi_exit(void)
> {
> struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
>
> + instrumentation_begin();
> /*
> * Check for ->dynticks_nmi_nesting underflow and bad ->dynticks.
> * (We are exiting an NMI handler, so RCU better be paying attention
> @@ -705,7 +706,6 @@ noinstr void rcu_nmi_exit(void)
> * leave it in non-RCU-idle state.
> */
> if (rdp->dynticks_nmi_nesting != 1) {
> - instrumentation_begin();
> trace_rcu_dyntick(TPS("--="), rdp->dynticks_nmi_nesting, rdp->dynticks_nmi_nesting - 2,
> atomic_read(&rdp->dynticks));
> WRITE_ONCE(rdp->dynticks_nmi_nesting, /* No store tearing. */
> @@ -714,7 +714,6 @@ noinstr void rcu_nmi_exit(void)
> return;
> }
>
> - instrumentation_begin();
> /* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */
> trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nmi_nesting, 0, atomic_read(&rdp->dynticks));
> WRITE_ONCE(rdp->dynticks_nmi_nesting, 0); /* Avoid store tearing. */
On Wed, Jun 03, 2020 at 02:59:32AM -0700, Paul E. McKenney wrote:
> On Wed, Jun 03, 2020 at 10:48:18AM +0200, Peter Zijlstra wrote:
> > On Tue, Jun 02, 2020 at 09:38:53PM +0200, Peter Zijlstra wrote:
> >
> > > 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.
> >
> > ---
> > Subject: rcu: Fixup noinstr warnings
> >
> > A KCSAN build revealed we have explicit annoations through atomic_t
> > usage, switch to arch_atomic_*() for the respective functions.
> >
> > vmlinux.o: warning: objtool: rcu_nmi_exit()+0x4d: call to __kcsan_check_access() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: rcu_dynticks_eqs_enter()+0x25: call to __kcsan_check_access() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: rcu_nmi_enter()+0x4f: call to __kcsan_check_access() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: rcu_dynticks_eqs_exit()+0x2a: call to __kcsan_check_access() leaves .noinstr.text section
> > vmlinux.o: warning: objtool: __rcu_is_watching()+0x25: call to __kcsan_check_access() leaves .noinstr.text section
> >
> > Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
>
> This one does not apply cleanly onto the -rcu tree's "dev" branch, so
> I am guessing that it is intended to be carried in -tip with yours and
> Thomas's patch series.
Right, I've not played patch tetris yet so see how it should all fit
together. I also didn't know you feel about loosing the instrumentation
in these functions.
One option would be do add explicit: instrument_atomic_write() calls
before instrument_end() / after instrument_begin() in
the respective callers that have that.
Anyway, I'll shortly be posting a pile of patches resulting from various
KCSAN and KASAN builds. The good news is that GCC-KASAN seems to behave
quite well with Marco's patches, the bad news is that GCC-KASAN is
retarded wrt inline and needs a bunch of kicks.
That is, it out-of-lines:
static inline bool foo(..)
{
return false;
}
just because..
On Tue, Jun 2, 2020 at 8:44 PM Marco Elver <[email protected]> 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/[email protected]
> Suggested-by: Peter Zijlstra <[email protected]>
> Signed-off-by: Marco Elver <[email protected]>
Acked-by: Andrey Konovalov <[email protected]>
> ---
> Apply after:
> https://lkml.kernel.org/r/[email protected]
> ---
> 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 >= 80000
> +
> 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
> <file:Documentation/dev-tools/kcsan.rst>.
> 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
>
On Tue, Jun 2, 2020 at 8:44 PM Marco Elver <[email protected]> wrote:
>
> Adds the portable definitions for __no_sanitize_address, and
> __no_sanitize_undefined, and subsequently changes noinstr to use the
> attributes to disable instrumentation via KASAN or UBSAN.
>
> Link: https://lore.kernel.org/lkml/[email protected]/
> Reported-by: [email protected]
> Signed-off-by: Marco Elver <[email protected]>
Acked-by: Andrey Konovalov <[email protected]>
> ---
>
> Note: __no_sanitize_coverage (for KCOV) isn't possible right now,
> because neither GCC nor Clang support such an attribute. This means
> going and changing the compilers again (for Clang it's fine, for GCC,
> it'll take a while).
>
> However, it looks like that KCOV_INSTRUMENT := n is currently in all the
> right places. Short-term, this should be reasonable.
> ---
> include/linux/compiler-clang.h | 8 ++++++++
> include/linux/compiler-gcc.h | 6 ++++++
> include/linux/compiler_types.h | 3 ++-
> 3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index 2cb42d8bdedc..c0e4b193b311 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -33,6 +33,14 @@
> #define __no_sanitize_thread
> #endif
>
> +#if __has_feature(undefined_behavior_sanitizer)
> +/* GCC does not have __SANITIZE_UNDEFINED__ */
> +#define __no_sanitize_undefined \
> + __attribute__((no_sanitize("undefined")))
> +#else
> +#define __no_sanitize_undefined
> +#endif
> +
> /*
> * Not all versions of clang implement the the type-generic versions
> * of the builtin overflow checkers. Fortunately, clang implements
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 7dd4e0349ef3..1c74464c80c6 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -150,6 +150,12 @@
> #define __no_sanitize_thread
> #endif
>
> +#if __has_attribute(__no_sanitize_undefined__)
> +#define __no_sanitize_undefined __attribute__((no_sanitize_undefined))
> +#else
> +#define __no_sanitize_undefined
> +#endif
> +
> #if GCC_VERSION >= 50100
> #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
> #endif
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 02becd21d456..89b8c1ae18a1 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -198,7 +198,8 @@ struct ftrace_likely_data {
>
> /* Section for code which can't be instrumented at all */
> #define noinstr \
> - noinline notrace __attribute((__section__(".noinstr.text"))) __no_kcsan
> + noinline notrace __attribute((__section__(".noinstr.text"))) \
> + __no_kcsan __no_sanitize_address __no_sanitize_undefined
>
> #endif /* __KERNEL__ */
>
> --
> 2.27.0.rc2.251.g90737beb825-goog
>
On Tue, Jun 2, 2020 at 8:49 PM Nick Desaulniers <[email protected]> wrote:
>
> Currently most of our compiler attribute detection is done in
> include/linux/compiler_attributes.h; I think this should be handled
> there. +Miguel Ojeda
Thanks a lot for the CC Nick! Marco is right, since this attribute is
different per-compiler, we don't want them in `compiler_attributes.h`
(for the moment -- we'll see if they end up with the same
syntax/behavior in the future).
Acked-by: Miguel Ojeda <[email protected]>
Cheers,
Miguel
On Wed, Jun 03, 2020 at 12:52:06PM +0200, Peter Zijlstra wrote:
> On Wed, Jun 03, 2020 at 02:59:32AM -0700, Paul E. McKenney wrote:
> > On Wed, Jun 03, 2020 at 10:48:18AM +0200, Peter Zijlstra wrote:
> > > On Tue, Jun 02, 2020 at 09:38:53PM +0200, Peter Zijlstra wrote:
> > >
> > > > 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.
> > >
> > > ---
> > > Subject: rcu: Fixup noinstr warnings
> > >
> > > A KCSAN build revealed we have explicit annoations through atomic_t
> > > usage, switch to arch_atomic_*() for the respective functions.
> > >
> > > vmlinux.o: warning: objtool: rcu_nmi_exit()+0x4d: call to __kcsan_check_access() leaves .noinstr.text section
> > > vmlinux.o: warning: objtool: rcu_dynticks_eqs_enter()+0x25: call to __kcsan_check_access() leaves .noinstr.text section
> > > vmlinux.o: warning: objtool: rcu_nmi_enter()+0x4f: call to __kcsan_check_access() leaves .noinstr.text section
> > > vmlinux.o: warning: objtool: rcu_dynticks_eqs_exit()+0x2a: call to __kcsan_check_access() leaves .noinstr.text section
> > > vmlinux.o: warning: objtool: __rcu_is_watching()+0x25: call to __kcsan_check_access() leaves .noinstr.text section
> > >
> > > Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> >
> > This one does not apply cleanly onto the -rcu tree's "dev" branch, so
> > I am guessing that it is intended to be carried in -tip with yours and
> > Thomas's patch series.
>
> Right, I've not played patch tetris yet so see how it should all fit
> together. I also didn't know you feel about loosing the instrumentation
> in these functions.
It would be very good for KCSAN to be able to detect misuse of ->dynticks!
> One option would be do add explicit: instrument_atomic_write() calls
> before instrument_end() / after instrument_begin() in
> the respective callers that have that.
One good thing: The atomic_andnot() goes away in -rcu patches slated
for v5.9. However, the others remain.
So if today's -next is any guide, this instrument_atomic_write()
would be added (for one example) in the functions that invoke
rcu_dynticks_eqs_enter(), since it is noinstr. Rather annoying, and
will require careful commenting. But there are only two such calls and
they are both in the same file and it is very low-level code, so this
should be doable.
I should also add some commentary to the RCU requirements document
say why all of this is happening.
> Anyway, I'll shortly be posting a pile of patches resulting from various
> KCSAN and KASAN builds. The good news is that GCC-KASAN seems to behave
> quite well with Marco's patches, the bad news is that GCC-KASAN is
> retarded wrt inline and needs a bunch of kicks.
>
> That is, it out-of-lines:
>
> static inline bool foo(..)
> {
> return false;
> }
>
> just because..
Compilers!!! :-/
Thanx, Paul
On Wed, 3 Jun 2020 at 15:35, 'Andrey Konovalov' via kasan-dev
<[email protected]> wrote:
>
> On Tue, Jun 2, 2020 at 8:44 PM Marco Elver <[email protected]> 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/[email protected]
> > Suggested-by: Peter Zijlstra <[email protected]>
> > Signed-off-by: Marco Elver <[email protected]>
>
> Acked-by: Andrey Konovalov <[email protected]>
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/[email protected]