* [PATCH -tip v2 01/11] ubsan, kcsan: don't combine sanitizer with kcov on clang
2020-05-21 11:08 [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11) Marco Elver
@ 2020-05-21 11:08 ` Marco Elver
2020-05-21 11:08 ` [PATCH -tip v2 02/11] kcsan: Avoid inserting __tsan_func_entry/exit if possible Marco Elver
` (10 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Marco Elver @ 2020-05-21 11:08 UTC (permalink / raw)
To: elver
Cc: paulmck, dvyukov, glider, andreyknvl, kasan-dev, linux-kernel,
tglx, mingo, peterz, will, clang-built-linux, bp, Arnd Bergmann
From: Arnd Bergmann <arnd@arndb.de>
Clang does not allow -fsanitize-coverage=trace-{pc,cmp} together
with -fsanitize=bounds or with ubsan:
clang: error: argument unused during compilation: '-fsanitize-coverage=trace-pc' [-Werror,-Wunused-command-line-argument]
clang: error: argument unused during compilation: '-fsanitize-coverage=trace-cmp' [-Werror,-Wunused-command-line-argument]
To avoid the warning, check whether clang can handle this correctly
or disallow ubsan and kcsan when kcov is enabled.
Link: https://bugs.llvm.org/show_bug.cgi?id=45831
Link: https://lore.kernel.org/lkml/20200505142341.1096942-1-arnd@arndb.de
Acked-by: Marco Elver <elver@google.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Marco Elver <elver@google.com>
---
This patch is already in -rcu tree, but since since the series is based
on -tip, to avoid conflict it is required for the subsequent patches.
---
lib/Kconfig.kcsan | 11 +++++++++++
lib/Kconfig.ubsan | 11 +++++++++++
2 files changed, 22 insertions(+)
diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
index ea28245c6c1d..a7276035ca0d 100644
--- a/lib/Kconfig.kcsan
+++ b/lib/Kconfig.kcsan
@@ -3,9 +3,20 @@
config HAVE_ARCH_KCSAN
bool
+config KCSAN_KCOV_BROKEN
+ def_bool KCOV && CC_HAS_SANCOV_TRACE_PC
+ depends on CC_IS_CLANG
+ depends on !$(cc-option,-Werror=unused-command-line-argument -fsanitize=thread -fsanitize-coverage=trace-pc)
+ help
+ Some versions of clang support either KCSAN and KCOV but not the
+ combination of the two.
+ See https://bugs.llvm.org/show_bug.cgi?id=45831 for the status
+ in newer releases.
+
menuconfig KCSAN
bool "KCSAN: dynamic data race detector"
depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN
+ depends on !KCSAN_KCOV_BROKEN
select STACKTRACE
help
The Kernel Concurrency Sanitizer (KCSAN) is a dynamic
diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
index 48469c95d78e..3baea77bf37f 100644
--- a/lib/Kconfig.ubsan
+++ b/lib/Kconfig.ubsan
@@ -26,9 +26,20 @@ config UBSAN_TRAP
the system. For some system builders this is an acceptable
trade-off.
+config UBSAN_KCOV_BROKEN
+ def_bool KCOV && CC_HAS_SANCOV_TRACE_PC
+ depends on CC_IS_CLANG
+ depends on !$(cc-option,-Werror=unused-command-line-argument -fsanitize=bounds -fsanitize-coverage=trace-pc)
+ help
+ Some versions of clang support either UBSAN or KCOV but not the
+ combination of the two.
+ See https://bugs.llvm.org/show_bug.cgi?id=45831 for the status
+ in newer releases.
+
config UBSAN_BOUNDS
bool "Perform array index bounds checking"
default UBSAN
+ depends on !UBSAN_KCOV_BROKEN
help
This option enables detection of directly indexed out of bounds
array accesses, where the array size is known at compile time.
--
2.26.2.761.g0e0b3e54be-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH -tip v2 02/11] kcsan: Avoid inserting __tsan_func_entry/exit if possible
2020-05-21 11:08 [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11) Marco Elver
2020-05-21 11:08 ` [PATCH -tip v2 01/11] ubsan, kcsan: don't combine sanitizer with kcov on clang Marco Elver
@ 2020-05-21 11:08 ` Marco Elver
2020-05-21 11:08 ` [PATCH -tip v2 03/11] kcsan: Support distinguishing volatile accesses Marco Elver
` (9 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Marco Elver @ 2020-05-21 11:08 UTC (permalink / raw)
To: elver
Cc: paulmck, dvyukov, glider, andreyknvl, kasan-dev, linux-kernel,
tglx, mingo, peterz, will, clang-built-linux, bp
To avoid inserting __tsan_func_{entry,exit}, add option if supported by
compiler. Currently only Clang can be told to not emit calls to these
functions. It is safe to not emit these, since KCSAN does not rely on
them.
Note that, if we disable __tsan_func_{entry,exit}(), we need to disable
tail-call optimization in sanitized compilation units, as otherwise we
may skip frames in the stack trace; in particular when the tail called
function is one of the KCSAN's runtime functions, and a report is
generated, might we miss the function where the actual access occurred.
Since __tsan_func_{entry,exit}() insertion effectively disabled
tail-call optimization, there should be no observable change. [This was
caught and confirmed with kcsan-test & UNWINDER_ORC.]
Signed-off-by: Marco Elver <elver@google.com>
---
scripts/Makefile.kcsan | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/scripts/Makefile.kcsan b/scripts/Makefile.kcsan
index caf1111a28ae..20337a7ecf54 100644
--- a/scripts/Makefile.kcsan
+++ b/scripts/Makefile.kcsan
@@ -1,6 +1,15 @@
# SPDX-License-Identifier: GPL-2.0
ifdef CONFIG_KCSAN
-CFLAGS_KCSAN := -fsanitize=thread
+# GCC and Clang accept backend options differently. Do not wrap in cc-option,
+# because Clang accepts "--param" even if it is unused.
+ifdef CONFIG_CC_IS_CLANG
+cc-param = -mllvm -$(1)
+else
+cc-param = --param -$(1)
+endif
+
+CFLAGS_KCSAN := -fsanitize=thread \
+ $(call cc-option,$(call cc-param,tsan-instrument-func-entry-exit=0) -fno-optimize-sibling-calls)
endif # CONFIG_KCSAN
--
2.26.2.761.g0e0b3e54be-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH -tip v2 03/11] kcsan: Support distinguishing volatile accesses
2020-05-21 11:08 [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11) Marco Elver
2020-05-21 11:08 ` [PATCH -tip v2 01/11] ubsan, kcsan: don't combine sanitizer with kcov on clang Marco Elver
2020-05-21 11:08 ` [PATCH -tip v2 02/11] kcsan: Avoid inserting __tsan_func_entry/exit if possible Marco Elver
@ 2020-05-21 11:08 ` Marco Elver
2020-05-21 13:18 ` Will Deacon
2020-05-21 11:08 ` [PATCH -tip v2 04/11] kcsan: Pass option tsan-instrument-read-before-write to Clang Marco Elver
` (8 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Marco Elver @ 2020-05-21 11:08 UTC (permalink / raw)
To: elver
Cc: paulmck, dvyukov, glider, andreyknvl, kasan-dev, linux-kernel,
tglx, mingo, peterz, will, clang-built-linux, bp
In the kernel, volatile is used in various concurrent context, whether
in low-level synchronization primitives or for legacy reasons. If
supported by the compiler, we will assume that aligned volatile accesses
up to sizeof(long long) (matching compiletime_assert_rwonce_type()) are
atomic.
Recent versions Clang [1] (GCC tentative [2]) can instrument volatile
accesses differently. Add the option (required) to enable the
instrumentation, and provide the necessary runtime functions. None of
the updated compilers are widely available yet (Clang 11 will be the
first release to support the feature).
[1] https://github.com/llvm/llvm-project/commit/5a2c31116f412c3b6888be361137efd705e05814
[2] https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544452.html
This patch allows removing any explicit checks in primitives such as
READ_ONCE() and WRITE_ONCE().
Signed-off-by: Marco Elver <elver@google.com>
---
v2:
* Reword Makefile comment.
---
kernel/kcsan/core.c | 43 ++++++++++++++++++++++++++++++++++++++++++
scripts/Makefile.kcsan | 5 ++++-
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c
index a73a66cf79df..15f67949d11e 100644
--- a/kernel/kcsan/core.c
+++ b/kernel/kcsan/core.c
@@ -789,6 +789,49 @@ void __tsan_write_range(void *ptr, size_t size)
}
EXPORT_SYMBOL(__tsan_write_range);
+/*
+ * Use of explicit volatile is generally disallowed [1], however, volatile is
+ * still used in various concurrent context, whether in low-level
+ * synchronization primitives or for legacy reasons.
+ * [1] https://lwn.net/Articles/233479/
+ *
+ * We only consider volatile accesses atomic if they are aligned and would pass
+ * the size-check of compiletime_assert_rwonce_type().
+ */
+#define DEFINE_TSAN_VOLATILE_READ_WRITE(size) \
+ void __tsan_volatile_read##size(void *ptr) \
+ { \
+ const bool is_atomic = size <= sizeof(long long) && \
+ IS_ALIGNED((unsigned long)ptr, size); \
+ if (IS_ENABLED(CONFIG_KCSAN_IGNORE_ATOMICS) && is_atomic) \
+ return; \
+ check_access(ptr, size, is_atomic ? KCSAN_ACCESS_ATOMIC : 0); \
+ } \
+ EXPORT_SYMBOL(__tsan_volatile_read##size); \
+ void __tsan_unaligned_volatile_read##size(void *ptr) \
+ __alias(__tsan_volatile_read##size); \
+ EXPORT_SYMBOL(__tsan_unaligned_volatile_read##size); \
+ void __tsan_volatile_write##size(void *ptr) \
+ { \
+ const bool is_atomic = size <= sizeof(long long) && \
+ IS_ALIGNED((unsigned long)ptr, size); \
+ if (IS_ENABLED(CONFIG_KCSAN_IGNORE_ATOMICS) && is_atomic) \
+ return; \
+ check_access(ptr, size, \
+ KCSAN_ACCESS_WRITE | \
+ (is_atomic ? KCSAN_ACCESS_ATOMIC : 0)); \
+ } \
+ EXPORT_SYMBOL(__tsan_volatile_write##size); \
+ void __tsan_unaligned_volatile_write##size(void *ptr) \
+ __alias(__tsan_volatile_write##size); \
+ EXPORT_SYMBOL(__tsan_unaligned_volatile_write##size)
+
+DEFINE_TSAN_VOLATILE_READ_WRITE(1);
+DEFINE_TSAN_VOLATILE_READ_WRITE(2);
+DEFINE_TSAN_VOLATILE_READ_WRITE(4);
+DEFINE_TSAN_VOLATILE_READ_WRITE(8);
+DEFINE_TSAN_VOLATILE_READ_WRITE(16);
+
/*
* The below are not required by KCSAN, but can still be emitted by the
* compiler.
diff --git a/scripts/Makefile.kcsan b/scripts/Makefile.kcsan
index 20337a7ecf54..75d2942b9437 100644
--- a/scripts/Makefile.kcsan
+++ b/scripts/Makefile.kcsan
@@ -9,7 +9,10 @@ else
cc-param = --param -$(1)
endif
+# Keep most options here optional, to allow enabling more compilers if absence
+# of some options does not break KCSAN nor causes false positive reports.
CFLAGS_KCSAN := -fsanitize=thread \
- $(call cc-option,$(call cc-param,tsan-instrument-func-entry-exit=0) -fno-optimize-sibling-calls)
+ $(call cc-option,$(call cc-param,tsan-instrument-func-entry-exit=0) -fno-optimize-sibling-calls) \
+ $(call cc-param,tsan-distinguish-volatile=1)
endif # CONFIG_KCSAN
--
2.26.2.761.g0e0b3e54be-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH -tip v2 03/11] kcsan: Support distinguishing volatile accesses
2020-05-21 11:08 ` [PATCH -tip v2 03/11] kcsan: Support distinguishing volatile accesses Marco Elver
@ 2020-05-21 13:18 ` Will Deacon
2020-05-21 13:26 ` Marco Elver
0 siblings, 1 reply; 21+ messages in thread
From: Will Deacon @ 2020-05-21 13:18 UTC (permalink / raw)
To: Marco Elver
Cc: paulmck, dvyukov, glider, andreyknvl, kasan-dev, linux-kernel,
tglx, mingo, peterz, clang-built-linux, bp
On Thu, May 21, 2020 at 01:08:46PM +0200, Marco Elver wrote:
> In the kernel, volatile is used in various concurrent context, whether
> in low-level synchronization primitives or for legacy reasons. If
> supported by the compiler, we will assume that aligned volatile accesses
> up to sizeof(long long) (matching compiletime_assert_rwonce_type()) are
> atomic.
>
> Recent versions Clang [1] (GCC tentative [2]) can instrument volatile
> accesses differently. Add the option (required) to enable the
> instrumentation, and provide the necessary runtime functions. None of
> the updated compilers are widely available yet (Clang 11 will be the
> first release to support the feature).
>
> [1] https://github.com/llvm/llvm-project/commit/5a2c31116f412c3b6888be361137efd705e05814
> [2] https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544452.html
>
> This patch allows removing any explicit checks in primitives such as
> READ_ONCE() and WRITE_ONCE().
>
> Signed-off-by: Marco Elver <elver@google.com>
> ---
> v2:
> * Reword Makefile comment.
> ---
> kernel/kcsan/core.c | 43 ++++++++++++++++++++++++++++++++++++++++++
> scripts/Makefile.kcsan | 5 ++++-
> 2 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c
> index a73a66cf79df..15f67949d11e 100644
> --- a/kernel/kcsan/core.c
> +++ b/kernel/kcsan/core.c
> @@ -789,6 +789,49 @@ void __tsan_write_range(void *ptr, size_t size)
> }
> EXPORT_SYMBOL(__tsan_write_range);
>
> +/*
> + * Use of explicit volatile is generally disallowed [1], however, volatile is
> + * still used in various concurrent context, whether in low-level
> + * synchronization primitives or for legacy reasons.
> + * [1] https://lwn.net/Articles/233479/
> + *
> + * We only consider volatile accesses atomic if they are aligned and would pass
> + * the size-check of compiletime_assert_rwonce_type().
> + */
> +#define DEFINE_TSAN_VOLATILE_READ_WRITE(size) \
> + void __tsan_volatile_read##size(void *ptr) \
> + { \
> + const bool is_atomic = size <= sizeof(long long) && \
> + IS_ALIGNED((unsigned long)ptr, size); \
> + if (IS_ENABLED(CONFIG_KCSAN_IGNORE_ATOMICS) && is_atomic) \
> + return; \
> + check_access(ptr, size, is_atomic ? KCSAN_ACCESS_ATOMIC : 0); \
> + } \
> + EXPORT_SYMBOL(__tsan_volatile_read##size); \
> + void __tsan_unaligned_volatile_read##size(void *ptr) \
> + __alias(__tsan_volatile_read##size); \
> + EXPORT_SYMBOL(__tsan_unaligned_volatile_read##size); \
> + void __tsan_volatile_write##size(void *ptr) \
> + { \
> + const bool is_atomic = size <= sizeof(long long) && \
> + IS_ALIGNED((unsigned long)ptr, size); \
> + if (IS_ENABLED(CONFIG_KCSAN_IGNORE_ATOMICS) && is_atomic) \
> + return; \
> + check_access(ptr, size, \
> + KCSAN_ACCESS_WRITE | \
> + (is_atomic ? KCSAN_ACCESS_ATOMIC : 0)); \
> + } \
> + EXPORT_SYMBOL(__tsan_volatile_write##size); \
> + void __tsan_unaligned_volatile_write##size(void *ptr) \
> + __alias(__tsan_volatile_write##size); \
> + EXPORT_SYMBOL(__tsan_unaligned_volatile_write##size)
> +
> +DEFINE_TSAN_VOLATILE_READ_WRITE(1);
> +DEFINE_TSAN_VOLATILE_READ_WRITE(2);
> +DEFINE_TSAN_VOLATILE_READ_WRITE(4);
> +DEFINE_TSAN_VOLATILE_READ_WRITE(8);
> +DEFINE_TSAN_VOLATILE_READ_WRITE(16);
Having a 16-byte case seems a bit weird to me, but I guess clang needs this
for some reason?
Will
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH -tip v2 03/11] kcsan: Support distinguishing volatile accesses
2020-05-21 13:18 ` Will Deacon
@ 2020-05-21 13:26 ` Marco Elver
0 siblings, 0 replies; 21+ messages in thread
From: Marco Elver @ 2020-05-21 13:26 UTC (permalink / raw)
To: Will Deacon
Cc: Paul E. McKenney, Dmitry Vyukov, Alexander Potapenko,
Andrey Konovalov, kasan-dev, LKML, Thomas Gleixner, Ingo Molnar,
Peter Zijlstra, clang-built-linux, Borislav Petkov
On Thu, 21 May 2020 at 15:18, Will Deacon <will@kernel.org> wrote:
>
> On Thu, May 21, 2020 at 01:08:46PM +0200, Marco Elver wrote:
> > In the kernel, volatile is used in various concurrent context, whether
> > in low-level synchronization primitives or for legacy reasons. If
> > supported by the compiler, we will assume that aligned volatile accesses
> > up to sizeof(long long) (matching compiletime_assert_rwonce_type()) are
> > atomic.
> >
> > Recent versions Clang [1] (GCC tentative [2]) can instrument volatile
> > accesses differently. Add the option (required) to enable the
> > instrumentation, and provide the necessary runtime functions. None of
> > the updated compilers are widely available yet (Clang 11 will be the
> > first release to support the feature).
> >
> > [1] https://github.com/llvm/llvm-project/commit/5a2c31116f412c3b6888be361137efd705e05814
> > [2] https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544452.html
> >
> > This patch allows removing any explicit checks in primitives such as
> > READ_ONCE() and WRITE_ONCE().
> >
> > Signed-off-by: Marco Elver <elver@google.com>
> > ---
> > v2:
> > * Reword Makefile comment.
> > ---
> > kernel/kcsan/core.c | 43 ++++++++++++++++++++++++++++++++++++++++++
> > scripts/Makefile.kcsan | 5 ++++-
> > 2 files changed, 47 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c
> > index a73a66cf79df..15f67949d11e 100644
> > --- a/kernel/kcsan/core.c
> > +++ b/kernel/kcsan/core.c
> > @@ -789,6 +789,49 @@ void __tsan_write_range(void *ptr, size_t size)
> > }
> > EXPORT_SYMBOL(__tsan_write_range);
> >
> > +/*
> > + * Use of explicit volatile is generally disallowed [1], however, volatile is
> > + * still used in various concurrent context, whether in low-level
> > + * synchronization primitives or for legacy reasons.
> > + * [1] https://lwn.net/Articles/233479/
> > + *
> > + * We only consider volatile accesses atomic if they are aligned and would pass
> > + * the size-check of compiletime_assert_rwonce_type().
> > + */
> > +#define DEFINE_TSAN_VOLATILE_READ_WRITE(size) \
> > + void __tsan_volatile_read##size(void *ptr) \
> > + { \
> > + const bool is_atomic = size <= sizeof(long long) && \
> > + IS_ALIGNED((unsigned long)ptr, size); \
> > + if (IS_ENABLED(CONFIG_KCSAN_IGNORE_ATOMICS) && is_atomic) \
> > + return; \
> > + check_access(ptr, size, is_atomic ? KCSAN_ACCESS_ATOMIC : 0); \
> > + } \
> > + EXPORT_SYMBOL(__tsan_volatile_read##size); \
> > + void __tsan_unaligned_volatile_read##size(void *ptr) \
> > + __alias(__tsan_volatile_read##size); \
> > + EXPORT_SYMBOL(__tsan_unaligned_volatile_read##size); \
> > + void __tsan_volatile_write##size(void *ptr) \
> > + { \
> > + const bool is_atomic = size <= sizeof(long long) && \
> > + IS_ALIGNED((unsigned long)ptr, size); \
> > + if (IS_ENABLED(CONFIG_KCSAN_IGNORE_ATOMICS) && is_atomic) \
> > + return; \
> > + check_access(ptr, size, \
> > + KCSAN_ACCESS_WRITE | \
> > + (is_atomic ? KCSAN_ACCESS_ATOMIC : 0)); \
> > + } \
> > + EXPORT_SYMBOL(__tsan_volatile_write##size); \
> > + void __tsan_unaligned_volatile_write##size(void *ptr) \
> > + __alias(__tsan_volatile_write##size); \
> > + EXPORT_SYMBOL(__tsan_unaligned_volatile_write##size)
> > +
> > +DEFINE_TSAN_VOLATILE_READ_WRITE(1);
> > +DEFINE_TSAN_VOLATILE_READ_WRITE(2);
> > +DEFINE_TSAN_VOLATILE_READ_WRITE(4);
> > +DEFINE_TSAN_VOLATILE_READ_WRITE(8);
> > +DEFINE_TSAN_VOLATILE_READ_WRITE(16);
>
> Having a 16-byte case seems a bit weird to me, but I guess clang needs this
> for some reason?
Yes, the emitted fixed-size instrumentation is up to 16 bytes, so
we'll need it (for both volatile and non-volatile -- otherwise we'll
get linker errors). It doesn't mean we'll consider 16 byte volatile
accesses as atomic, because of the size check to compute is_atomic
above.
Thanks,
-- Marco
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH -tip v2 04/11] kcsan: Pass option tsan-instrument-read-before-write to Clang
2020-05-21 11:08 [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11) Marco Elver
` (2 preceding siblings ...)
2020-05-21 11:08 ` [PATCH -tip v2 03/11] kcsan: Support distinguishing volatile accesses Marco Elver
@ 2020-05-21 11:08 ` Marco Elver
2020-05-21 11:08 ` [PATCH -tip v2 05/11] kcsan: Remove 'noinline' from __no_kcsan_or_inline Marco Elver
` (7 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Marco Elver @ 2020-05-21 11:08 UTC (permalink / raw)
To: elver
Cc: paulmck, dvyukov, glider, andreyknvl, kasan-dev, linux-kernel,
tglx, mingo, peterz, will, clang-built-linux, bp
Clang (unlike GCC) removes reads before writes with matching addresses
in the same basic block. This is an optimization for TSAN, since writes
will always cause conflict if the preceding read would have.
However, for KCSAN we cannot rely on this option, because we apply
several special rules to writes, in particular when the
KCSAN_ASSUME_PLAIN_WRITES_ATOMIC option is selected. To avoid missing
potential data races, pass the -tsan-instrument-read-before-write option
to Clang if it is available [1].
[1] https://github.com/llvm/llvm-project/commit/151ed6aa38a3ec6c01973b35f684586b6e1c0f7e
Signed-off-by: Marco Elver <elver@google.com>
---
scripts/Makefile.kcsan | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/Makefile.kcsan b/scripts/Makefile.kcsan
index 75d2942b9437..bd4da1af5953 100644
--- a/scripts/Makefile.kcsan
+++ b/scripts/Makefile.kcsan
@@ -13,6 +13,7 @@ endif
# of some options does not break KCSAN nor causes false positive reports.
CFLAGS_KCSAN := -fsanitize=thread \
$(call cc-option,$(call cc-param,tsan-instrument-func-entry-exit=0) -fno-optimize-sibling-calls) \
+ $(call cc-option,$(call cc-param,tsan-instrument-read-before-write=1)) \
$(call cc-param,tsan-distinguish-volatile=1)
endif # CONFIG_KCSAN
--
2.26.2.761.g0e0b3e54be-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH -tip v2 05/11] kcsan: Remove 'noinline' from __no_kcsan_or_inline
2020-05-21 11:08 [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11) Marco Elver
` (3 preceding siblings ...)
2020-05-21 11:08 ` [PATCH -tip v2 04/11] kcsan: Pass option tsan-instrument-read-before-write to Clang Marco Elver
@ 2020-05-21 11:08 ` Marco Elver
2020-05-21 11:08 ` [PATCH -tip v2 06/11] kcsan: Restrict supported compilers Marco Elver
` (6 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Marco Elver @ 2020-05-21 11:08 UTC (permalink / raw)
To: elver
Cc: paulmck, dvyukov, glider, andreyknvl, kasan-dev, linux-kernel,
tglx, mingo, peterz, will, clang-built-linux, bp
Some compilers incorrectly inline small __no_kcsan functions, which then
results in instrumenting the accesses. For this reason, the 'noinline'
attribute was added to __no_kcsan_or_inline. All known versions of GCC
are affected by this. Supported version of Clang are unaffected, and
never inlines a no_sanitize function.
However, the attribute 'noinline' in __no_kcsan_or_inline causes
unexpected code generation in functions that are __no_kcsan and call a
__no_kcsan_or_inline function.
In certain situations it is expected that the __no_kcsan_or_inline
function is actually inlined by the __no_kcsan function, and *no* calls
are emitted. By removing the 'noinline' attribute we give the compiler
the ability to inline and generate the expected code in __no_kcsan
functions.
Link: https://lkml.kernel.org/r/CANpmjNNOpJk0tprXKB_deiNAv_UmmORf1-2uajLhnLWQQ1hvoA@mail.gmail.com
Signed-off-by: Marco Elver <elver@google.com>
---
include/linux/compiler.h | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index e24cc3a2bc3e..17c98b215572 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -276,11 +276,9 @@ do { \
#ifdef __SANITIZE_THREAD__
/*
* Rely on __SANITIZE_THREAD__ instead of CONFIG_KCSAN, to avoid not inlining in
- * compilation units where instrumentation is disabled. The attribute 'noinline'
- * is required for older compilers, where implicit inlining of very small
- * functions renders __no_sanitize_thread ineffective.
+ * compilation units where instrumentation is disabled.
*/
-# define __no_kcsan_or_inline __no_kcsan noinline notrace __maybe_unused
+# define __no_kcsan_or_inline __no_kcsan notrace __maybe_unused
# define __no_sanitize_or_inline __no_kcsan_or_inline
#else
# define __no_kcsan_or_inline __always_inline
--
2.26.2.761.g0e0b3e54be-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH -tip v2 06/11] kcsan: Restrict supported compilers
2020-05-21 11:08 [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11) Marco Elver
` (4 preceding siblings ...)
2020-05-21 11:08 ` [PATCH -tip v2 05/11] kcsan: Remove 'noinline' from __no_kcsan_or_inline Marco Elver
@ 2020-05-21 11:08 ` Marco Elver
2020-05-21 11:08 ` [PATCH -tip v2 07/11] kcsan: Update Documentation to change " Marco Elver
` (5 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Marco Elver @ 2020-05-21 11:08 UTC (permalink / raw)
To: elver
Cc: paulmck, dvyukov, glider, andreyknvl, kasan-dev, linux-kernel,
tglx, mingo, peterz, will, clang-built-linux, bp
The first version of Clang that supports -tsan-distinguish-volatile will
be able to support KCSAN. The first Clang release to do so, will be
Clang 11. This is due to satisfying all the following requirements:
1. Never emit calls to __tsan_func_{entry,exit}.
2. __no_kcsan functions should not call anything, not even
kcsan_{enable,disable}_current(), when using __{READ,WRITE}_ONCE => Requires
leaving them plain!
3. Support atomic_{read,set}*() with KCSAN, which rely on
arch_atomic_{read,set}*() using __{READ,WRITE}_ONCE() => Because of
#2, rely on Clang 11's -tsan-distinguish-volatile support. We will
double-instrument atomic_{read,set}*(), but that's reasonable given
it's still lower cost than the data_race() variant due to avoiding 2
extra calls (kcsan_{en,dis}able_current() calls).
4. __always_inline functions inlined into __no_kcsan functions are never
instrumented.
5. __always_inline functions inlined into instrumented functions are
instrumented.
6. __no_kcsan_or_inline functions may be inlined into __no_kcsan functions =>
Implies leaving 'noinline' off of __no_kcsan_or_inline.
7. Because of #6, __no_kcsan and __no_kcsan_or_inline functions should never be
spuriously inlined into instrumented functions, causing the accesses of the
__no_kcsan function to be instrumented.
Older versions of Clang do not satisfy #3. The latest GCC currently doesn't
support at least #1, #3, and #7.
Link: https://lkml.kernel.org/r/CANpmjNMTsY_8241bS7=XAfqvZHFLrVEkv_uM4aDUWE_kh3Rvbw@mail.gmail.com
Signed-off-by: Marco Elver <elver@google.com>
---
lib/Kconfig.kcsan | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
index a7276035ca0d..3f3b5bca7a8f 100644
--- a/lib/Kconfig.kcsan
+++ b/lib/Kconfig.kcsan
@@ -3,6 +3,12 @@
config HAVE_ARCH_KCSAN
bool
+config HAVE_KCSAN_COMPILER
+ def_bool CC_IS_CLANG && $(cc-option,-fsanitize=thread -mllvm -tsan-distinguish-volatile=1)
+ help
+ For the list of compilers that support KCSAN, please see
+ <file:Documentation/dev-tools/kcsan.rst>.
+
config KCSAN_KCOV_BROKEN
def_bool KCOV && CC_HAS_SANCOV_TRACE_PC
depends on CC_IS_CLANG
@@ -15,7 +21,8 @@ config KCSAN_KCOV_BROKEN
menuconfig KCSAN
bool "KCSAN: dynamic data race detector"
- depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN
+ depends on HAVE_ARCH_KCSAN && HAVE_KCSAN_COMPILER
+ depends on DEBUG_KERNEL && !KASAN
depends on !KCSAN_KCOV_BROKEN
select STACKTRACE
help
--
2.26.2.761.g0e0b3e54be-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH -tip v2 07/11] kcsan: Update Documentation to change supported compilers
2020-05-21 11:08 [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11) Marco Elver
` (5 preceding siblings ...)
2020-05-21 11:08 ` [PATCH -tip v2 06/11] kcsan: Restrict supported compilers Marco Elver
@ 2020-05-21 11:08 ` Marco Elver
2020-05-21 13:33 ` Will Deacon
2020-05-21 11:08 ` [PATCH -tip v2 08/11] READ_ONCE, WRITE_ONCE: Remove data_race() and unnecessary checks Marco Elver
` (4 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Marco Elver @ 2020-05-21 11:08 UTC (permalink / raw)
To: elver
Cc: paulmck, dvyukov, glider, andreyknvl, kasan-dev, linux-kernel,
tglx, mingo, peterz, will, clang-built-linux, bp
Signed-off-by: Marco Elver <elver@google.com>
---
Documentation/dev-tools/kcsan.rst | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/Documentation/dev-tools/kcsan.rst b/Documentation/dev-tools/kcsan.rst
index f4b5766f12cc..ce4bbd918648 100644
--- a/Documentation/dev-tools/kcsan.rst
+++ b/Documentation/dev-tools/kcsan.rst
@@ -8,8 +8,7 @@ approach to detect races. KCSAN's primary purpose is to detect `data races`_.
Usage
-----
-KCSAN is supported in both GCC and Clang. With GCC it requires version 7.3.0 or
-later. With Clang it requires version 7.0.0 or later.
+KCSAN requires Clang version 11 or later.
To enable KCSAN configure the kernel with::
@@ -121,12 +120,6 @@ the below options are available:
static __no_kcsan_or_inline void foo(void) {
...
- Note: Older compiler versions (GCC < 9) also do not always honor the
- ``__no_kcsan`` attribute on regular ``inline`` functions. If false positives
- with these compilers cannot be tolerated, for small functions where
- ``__always_inline`` would be appropriate, ``__no_kcsan_or_inline`` should be
- preferred instead.
-
* To disable data race detection for a particular compilation unit, add to the
``Makefile``::
--
2.26.2.761.g0e0b3e54be-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH -tip v2 07/11] kcsan: Update Documentation to change supported compilers
2020-05-21 11:08 ` [PATCH -tip v2 07/11] kcsan: Update Documentation to change " Marco Elver
@ 2020-05-21 13:33 ` Will Deacon
2020-05-21 13:35 ` Marco Elver
0 siblings, 1 reply; 21+ messages in thread
From: Will Deacon @ 2020-05-21 13:33 UTC (permalink / raw)
To: Marco Elver
Cc: paulmck, dvyukov, glider, andreyknvl, kasan-dev, linux-kernel,
tglx, mingo, peterz, clang-built-linux, bp
On Thu, May 21, 2020 at 01:08:50PM +0200, Marco Elver wrote:
> Signed-off-by: Marco Elver <elver@google.com>
> ---
> Documentation/dev-tools/kcsan.rst | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
-ENOCOMMITMSG
Will
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH -tip v2 07/11] kcsan: Update Documentation to change supported compilers
2020-05-21 13:33 ` Will Deacon
@ 2020-05-21 13:35 ` Marco Elver
0 siblings, 0 replies; 21+ messages in thread
From: Marco Elver @ 2020-05-21 13:35 UTC (permalink / raw)
To: Will Deacon
Cc: Paul E. McKenney, Dmitry Vyukov, Alexander Potapenko,
Andrey Konovalov, kasan-dev, LKML, Thomas Gleixner, Ingo Molnar,
Peter Zijlstra, clang-built-linux, Borislav Petkov
On Thu, 21 May 2020 at 15:33, Will Deacon <will@kernel.org> wrote:
>
> On Thu, May 21, 2020 at 01:08:50PM +0200, Marco Elver wrote:
> > Signed-off-by: Marco Elver <elver@google.com>
> > ---
> > Documentation/dev-tools/kcsan.rst | 9 +--------
> > 1 file changed, 1 insertion(+), 8 deletions(-)
>
> -ENOCOMMITMSG
Oops. Ok, then there will be a v3.
> Will
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20200521133322.GC6608%40willie-the-truck.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH -tip v2 08/11] READ_ONCE, WRITE_ONCE: Remove data_race() and unnecessary checks
2020-05-21 11:08 [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11) Marco Elver
` (6 preceding siblings ...)
2020-05-21 11:08 ` [PATCH -tip v2 07/11] kcsan: Update Documentation to change " Marco Elver
@ 2020-05-21 11:08 ` Marco Elver
2020-05-21 11:08 ` [PATCH -tip v2 09/11] data_race: Avoid nested statement expression Marco Elver
` (3 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Marco Elver @ 2020-05-21 11:08 UTC (permalink / raw)
To: elver
Cc: paulmck, dvyukov, glider, andreyknvl, kasan-dev, linux-kernel,
tglx, mingo, peterz, will, clang-built-linux, bp
The volatile accesses no longer need to be wrapped in data_race(),
because we require compilers that emit instrumentation distinguishing
volatile accesses. Consequently, we also no longer require the explicit
kcsan_check_atomic*(), since the compiler emits instrumentation
distinguishing the volatile accesses. Finally, simplify
__READ_ONCE_SCALAR and remove __WRITE_ONCE_SCALAR.
Signed-off-by: Marco Elver <elver@google.com>
---
v2:
* Remove unnecessary kcsan_check_atomic*() in *_ONCE.
* Simplify __READ_ONCE_SCALAR and remove __WRITE_ONCE_SCALAR. This
effectively restores Will Deacon's pre-KCSAN version:
https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/tree/include/linux/compiler.h?h=rwonce/cleanup#n202
---
include/linux/compiler.h | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 17c98b215572..7444f026eead 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -228,9 +228,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
#define __READ_ONCE_SCALAR(x) \
({ \
- typeof(x) *__xp = &(x); \
- __unqual_scalar_typeof(x) __x = data_race(__READ_ONCE(*__xp)); \
- kcsan_check_atomic_read(__xp, sizeof(*__xp)); \
+ __unqual_scalar_typeof(x) __x = __READ_ONCE(x); \
smp_read_barrier_depends(); \
(typeof(x))__x; \
})
@@ -246,17 +244,10 @@ do { \
*(volatile typeof(x) *)&(x) = (val); \
} while (0)
-#define __WRITE_ONCE_SCALAR(x, val) \
-do { \
- typeof(x) *__xp = &(x); \
- kcsan_check_atomic_write(__xp, sizeof(*__xp)); \
- data_race(({ __WRITE_ONCE(*__xp, val); 0; })); \
-} while (0)
-
#define WRITE_ONCE(x, val) \
do { \
compiletime_assert_rwonce_type(x); \
- __WRITE_ONCE_SCALAR(x, val); \
+ __WRITE_ONCE(x, val); \
} while (0)
#ifdef CONFIG_KASAN
--
2.26.2.761.g0e0b3e54be-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH -tip v2 09/11] data_race: Avoid nested statement expression
2020-05-21 11:08 [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11) Marco Elver
` (7 preceding siblings ...)
2020-05-21 11:08 ` [PATCH -tip v2 08/11] READ_ONCE, WRITE_ONCE: Remove data_race() and unnecessary checks Marco Elver
@ 2020-05-21 11:08 ` Marco Elver
2020-05-21 13:31 ` Will Deacon
2020-05-21 11:08 ` [PATCH -tip v2 10/11] compiler.h: Move function attributes to compiler_types.h Marco Elver
` (2 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Marco Elver @ 2020-05-21 11:08 UTC (permalink / raw)
To: elver
Cc: paulmck, dvyukov, glider, andreyknvl, kasan-dev, linux-kernel,
tglx, mingo, peterz, will, clang-built-linux, bp
It appears that compilers have trouble with nested statements
expressions, as such make the data_race() macro be only a single
statement expression. This will help us avoid potential problems in
future as its usage increases.
Link: https://lkml.kernel.org/r/20200520221712.GA21166@zn.tnic
Signed-off-by: Marco Elver <elver@google.com>
---
v2:
* Add patch to series in response to above linked discussion.
---
include/linux/compiler.h | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 7444f026eead..1f9bd9f35368 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -211,12 +211,11 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
*/
#define data_race(expr) \
({ \
+ __unqual_scalar_typeof(({ expr; })) __v; \
__kcsan_disable_current(); \
- ({ \
- __unqual_scalar_typeof(({ expr; })) __v = ({ expr; }); \
- __kcsan_enable_current(); \
- __v; \
- }); \
+ __v = ({ expr; }); \
+ __kcsan_enable_current(); \
+ __v; \
})
/*
--
2.26.2.761.g0e0b3e54be-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH -tip v2 09/11] data_race: Avoid nested statement expression
2020-05-21 11:08 ` [PATCH -tip v2 09/11] data_race: Avoid nested statement expression Marco Elver
@ 2020-05-21 13:31 ` Will Deacon
2020-05-21 13:39 ` Marco Elver
0 siblings, 1 reply; 21+ messages in thread
From: Will Deacon @ 2020-05-21 13:31 UTC (permalink / raw)
To: Marco Elver
Cc: paulmck, dvyukov, glider, andreyknvl, kasan-dev, linux-kernel,
tglx, mingo, peterz, clang-built-linux, bp
On Thu, May 21, 2020 at 01:08:52PM +0200, Marco Elver wrote:
> It appears that compilers have trouble with nested statements
> expressions, as such make the data_race() macro be only a single
> statement expression. This will help us avoid potential problems in
> future as its usage increases.
>
> Link: https://lkml.kernel.org/r/20200520221712.GA21166@zn.tnic
> Signed-off-by: Marco Elver <elver@google.com>
> ---
> v2:
> * Add patch to series in response to above linked discussion.
> ---
> include/linux/compiler.h | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 7444f026eead..1f9bd9f35368 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -211,12 +211,11 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> */
> #define data_race(expr) \
> ({ \
> + __unqual_scalar_typeof(({ expr; })) __v; \
> __kcsan_disable_current(); \
> - ({ \
> - __unqual_scalar_typeof(({ expr; })) __v = ({ expr; }); \
> - __kcsan_enable_current(); \
> - __v; \
> - }); \
> + __v = ({ expr; }); \
> + __kcsan_enable_current(); \
> + __v; \
Hopefully it doesn't matter, but this will run into issues with 'const'
non-scalar expressions.
Will
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH -tip v2 09/11] data_race: Avoid nested statement expression
2020-05-21 13:31 ` Will Deacon
@ 2020-05-21 13:39 ` Marco Elver
0 siblings, 0 replies; 21+ messages in thread
From: Marco Elver @ 2020-05-21 13:39 UTC (permalink / raw)
To: Will Deacon
Cc: Paul E. McKenney, Dmitry Vyukov, Alexander Potapenko,
Andrey Konovalov, kasan-dev, LKML, Thomas Gleixner, Ingo Molnar,
Peter Zijlstra, clang-built-linux, Borislav Petkov
On Thu, 21 May 2020 at 15:31, Will Deacon <will@kernel.org> wrote:
>
> On Thu, May 21, 2020 at 01:08:52PM +0200, Marco Elver wrote:
> > It appears that compilers have trouble with nested statements
> > expressions, as such make the data_race() macro be only a single
> > statement expression. This will help us avoid potential problems in
> > future as its usage increases.
> >
> > Link: https://lkml.kernel.org/r/20200520221712.GA21166@zn.tnic
> > Signed-off-by: Marco Elver <elver@google.com>
> > ---
> > v2:
> > * Add patch to series in response to above linked discussion.
> > ---
> > include/linux/compiler.h | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> > index 7444f026eead..1f9bd9f35368 100644
> > --- a/include/linux/compiler.h
> > +++ b/include/linux/compiler.h
> > @@ -211,12 +211,11 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
> > */
> > #define data_race(expr) \
> > ({ \
> > + __unqual_scalar_typeof(({ expr; })) __v; \
> > __kcsan_disable_current(); \
> > - ({ \
> > - __unqual_scalar_typeof(({ expr; })) __v = ({ expr; }); \
> > - __kcsan_enable_current(); \
> > - __v; \
> > - }); \
> > + __v = ({ expr; }); \
> > + __kcsan_enable_current(); \
> > + __v; \
>
> Hopefully it doesn't matter, but this will run into issues with 'const'
> non-scalar expressions.
Good point. We could move the kcsan_disable_current() into ({
__kcsan_disable_current(); expr; }).
Will fix for v3.
Thanks,
-- Marco
> Will
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20200521133150.GB6608%40willie-the-truck.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH -tip v2 10/11] compiler.h: Move function attributes to compiler_types.h
2020-05-21 11:08 [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11) Marco Elver
` (8 preceding siblings ...)
2020-05-21 11:08 ` [PATCH -tip v2 09/11] data_race: Avoid nested statement expression Marco Elver
@ 2020-05-21 11:08 ` Marco Elver
2020-05-21 11:08 ` [PATCH -tip v2 11/11] compiler_types.h, kasan: Use __SANITIZE_ADDRESS__ instead of CONFIG_KASAN to decide inlining Marco Elver
2020-05-21 13:36 ` [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11) Will Deacon
11 siblings, 0 replies; 21+ messages in thread
From: Marco Elver @ 2020-05-21 11:08 UTC (permalink / raw)
To: elver
Cc: paulmck, dvyukov, glider, andreyknvl, kasan-dev, linux-kernel,
tglx, mingo, peterz, will, clang-built-linux, bp
Cleanup and move the KASAN and KCSAN related function attributes to
compiler_types.h, where the rest of the same kind live.
No functional change intended.
Signed-off-by: Marco Elver <elver@google.com>
---
include/linux/compiler.h | 29 -----------------------------
include/linux/compiler_types.h | 29 +++++++++++++++++++++++++++++
2 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 1f9bd9f35368..8d3d03f9d562 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -249,35 +249,6 @@ do { \
__WRITE_ONCE(x, val); \
} while (0)
-#ifdef CONFIG_KASAN
-/*
- * We can't declare function 'inline' because __no_sanitize_address conflicts
- * with inlining. Attempt to inline it may cause a build failure.
- * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
- * '__maybe_unused' allows us to avoid defined-but-not-used warnings.
- */
-# define __no_kasan_or_inline __no_sanitize_address notrace __maybe_unused
-# define __no_sanitize_or_inline __no_kasan_or_inline
-#else
-# define __no_kasan_or_inline __always_inline
-#endif
-
-#define __no_kcsan __no_sanitize_thread
-#ifdef __SANITIZE_THREAD__
-/*
- * Rely on __SANITIZE_THREAD__ instead of CONFIG_KCSAN, to avoid not inlining in
- * compilation units where instrumentation is disabled.
- */
-# define __no_kcsan_or_inline __no_kcsan notrace __maybe_unused
-# define __no_sanitize_or_inline __no_kcsan_or_inline
-#else
-# define __no_kcsan_or_inline __always_inline
-#endif
-
-#ifndef __no_sanitize_or_inline
-#define __no_sanitize_or_inline __always_inline
-#endif
-
static __no_sanitize_or_inline
unsigned long __read_once_word_nocheck(const void *addr)
{
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 6ed0612bc143..b190a12e7089 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -167,6 +167,35 @@ struct ftrace_likely_data {
*/
#define noinline_for_stack noinline
+#ifdef CONFIG_KASAN
+/*
+ * We can't declare function 'inline' because __no_sanitize_address conflicts
+ * with inlining. Attempt to inline it may cause a build failure.
+ * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
+ * '__maybe_unused' allows us to avoid defined-but-not-used warnings.
+ */
+# define __no_kasan_or_inline __no_sanitize_address notrace __maybe_unused
+# define __no_sanitize_or_inline __no_kasan_or_inline
+#else
+# define __no_kasan_or_inline __always_inline
+#endif
+
+#define __no_kcsan __no_sanitize_thread
+#ifdef __SANITIZE_THREAD__
+/*
+ * Rely on __SANITIZE_THREAD__ instead of CONFIG_KCSAN, to avoid not inlining in
+ * compilation units where instrumentation is disabled.
+ */
+# define __no_kcsan_or_inline __no_kcsan notrace __maybe_unused
+# define __no_sanitize_or_inline __no_kcsan_or_inline
+#else
+# define __no_kcsan_or_inline __always_inline
+#endif
+
+#ifndef __no_sanitize_or_inline
+#define __no_sanitize_or_inline __always_inline
+#endif
+
#endif /* __KERNEL__ */
#endif /* __ASSEMBLY__ */
--
2.26.2.761.g0e0b3e54be-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH -tip v2 11/11] compiler_types.h, kasan: Use __SANITIZE_ADDRESS__ instead of CONFIG_KASAN to decide inlining
2020-05-21 11:08 [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11) Marco Elver
` (9 preceding siblings ...)
2020-05-21 11:08 ` [PATCH -tip v2 10/11] compiler.h: Move function attributes to compiler_types.h Marco Elver
@ 2020-05-21 11:08 ` Marco Elver
2020-05-21 13:36 ` [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11) Will Deacon
11 siblings, 0 replies; 21+ messages in thread
From: Marco Elver @ 2020-05-21 11:08 UTC (permalink / raw)
To: elver
Cc: paulmck, dvyukov, glider, andreyknvl, kasan-dev, linux-kernel,
tglx, mingo, peterz, will, clang-built-linux, bp
Like is done for KCSAN, for KASAN we should also use __always_inline in
compilation units that have instrumentation disabled
(KASAN_SANITIZE_foo.o := n). Adds common documentation for KASAN and
KCSAN explaining the attribute.
Signed-off-by: Marco Elver <elver@google.com>
---
include/linux/compiler_types.h | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index b190a12e7089..5faf68eae204 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -167,7 +167,14 @@ struct ftrace_likely_data {
*/
#define noinline_for_stack noinline
-#ifdef CONFIG_KASAN
+/*
+ * Sanitizer helper attributes: Because using __always_inline and
+ * __no_sanitize_* conflict, provide helper attributes that will either expand
+ * to __no_sanitize_* in compilation units where instrumentation is enabled
+ * (__SANITIZE_*__), or __always_inline in compilation units without
+ * instrumentation (__SANITIZE_*__ undefined).
+ */
+#ifdef __SANITIZE_ADDRESS__
/*
* We can't declare function 'inline' because __no_sanitize_address conflicts
* with inlining. Attempt to inline it may cause a build failure.
@@ -182,10 +189,6 @@ struct ftrace_likely_data {
#define __no_kcsan __no_sanitize_thread
#ifdef __SANITIZE_THREAD__
-/*
- * Rely on __SANITIZE_THREAD__ instead of CONFIG_KCSAN, to avoid not inlining in
- * compilation units where instrumentation is disabled.
- */
# define __no_kcsan_or_inline __no_kcsan notrace __maybe_unused
# define __no_sanitize_or_inline __no_kcsan_or_inline
#else
--
2.26.2.761.g0e0b3e54be-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11)
2020-05-21 11:08 [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11) Marco Elver
` (10 preceding siblings ...)
2020-05-21 11:08 ` [PATCH -tip v2 11/11] compiler_types.h, kasan: Use __SANITIZE_ADDRESS__ instead of CONFIG_KASAN to decide inlining Marco Elver
@ 2020-05-21 13:36 ` Will Deacon
2020-05-21 13:42 ` Marco Elver
11 siblings, 1 reply; 21+ messages in thread
From: Will Deacon @ 2020-05-21 13:36 UTC (permalink / raw)
To: Marco Elver
Cc: paulmck, dvyukov, glider, andreyknvl, kasan-dev, linux-kernel,
tglx, mingo, peterz, clang-built-linux, bp
On Thu, May 21, 2020 at 01:08:43PM +0200, Marco Elver wrote:
> This patch series is the conclusion to [1], where we determined that due
> to various interactions with no_sanitize attributes and the new
> {READ,WRITE}_ONCE(), KCSAN will require Clang 11 or later. Other
> sanitizers are largely untouched, and only KCSAN now has a hard
> dependency on Clang 11. To test, a recent Clang development version will
> suffice [2]. While a little inconvenient for now, it is hoped that in
> future we may be able to fix GCC and re-enable GCC support.
>
> The patch "kcsan: Restrict supported compilers" contains a detailed list
> of requirements that led to this decision.
>
> Most of the patches are related to KCSAN, however, the first patch also
> includes an UBSAN related fix and is a dependency for the remaining
> ones. The last 2 patches clean up the attributes by moving them to the
> right place, and fix KASAN's way of defining __no_kasan_or_inline,
> making it consistent with KCSAN.
>
> The series has been tested by running kcsan-test several times and
> completed successfully.
I've left a few minor comments, but the only one that probably needs a bit
of thought is using data_race() with const non-scalar expressions, since I
think that's now prohibited by these changes. We don't have too many
data_race() users yet, so probably not a big deal, but worth bearing in
mind.
Other than that,
Acked-by: Will Deacon <will@kernel.org>
Thanks!
Will
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11)
2020-05-21 13:36 ` [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11) Will Deacon
@ 2020-05-21 13:42 ` Marco Elver
2020-05-21 13:42 ` Will Deacon
0 siblings, 1 reply; 21+ messages in thread
From: Marco Elver @ 2020-05-21 13:42 UTC (permalink / raw)
To: Will Deacon
Cc: Paul E. McKenney, Dmitry Vyukov, Alexander Potapenko,
Andrey Konovalov, kasan-dev, LKML, Thomas Gleixner, Ingo Molnar,
Peter Zijlstra, clang-built-linux, Borislav Petkov
On Thu, 21 May 2020 at 15:36, Will Deacon <will@kernel.org> wrote:
>
> On Thu, May 21, 2020 at 01:08:43PM +0200, Marco Elver wrote:
> > This patch series is the conclusion to [1], where we determined that due
> > to various interactions with no_sanitize attributes and the new
> > {READ,WRITE}_ONCE(), KCSAN will require Clang 11 or later. Other
> > sanitizers are largely untouched, and only KCSAN now has a hard
> > dependency on Clang 11. To test, a recent Clang development version will
> > suffice [2]. While a little inconvenient for now, it is hoped that in
> > future we may be able to fix GCC and re-enable GCC support.
> >
> > The patch "kcsan: Restrict supported compilers" contains a detailed list
> > of requirements that led to this decision.
> >
> > Most of the patches are related to KCSAN, however, the first patch also
> > includes an UBSAN related fix and is a dependency for the remaining
> > ones. The last 2 patches clean up the attributes by moving them to the
> > right place, and fix KASAN's way of defining __no_kasan_or_inline,
> > making it consistent with KCSAN.
> >
> > The series has been tested by running kcsan-test several times and
> > completed successfully.
>
> I've left a few minor comments, but the only one that probably needs a bit
> of thought is using data_race() with const non-scalar expressions, since I
> think that's now prohibited by these changes. We don't have too many
> data_race() users yet, so probably not a big deal, but worth bearing in
> mind.
If you don't mind, I'll do a v3 with that fixed.
> Other than that,
>
> Acked-by: Will Deacon <will@kernel.org>
Thank you!
-- Marco
> Thanks!
>
> Will
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH -tip v2 00/11] Fix KCSAN for new ONCE (require Clang 11)
2020-05-21 13:42 ` Marco Elver
@ 2020-05-21 13:42 ` Will Deacon
0 siblings, 0 replies; 21+ messages in thread
From: Will Deacon @ 2020-05-21 13:42 UTC (permalink / raw)
To: Marco Elver
Cc: Paul E. McKenney, Dmitry Vyukov, Alexander Potapenko,
Andrey Konovalov, kasan-dev, LKML, Thomas Gleixner, Ingo Molnar,
Peter Zijlstra, clang-built-linux, Borislav Petkov
On Thu, May 21, 2020 at 03:42:12PM +0200, Marco Elver wrote:
> On Thu, 21 May 2020 at 15:36, Will Deacon <will@kernel.org> wrote:
> >
> > On Thu, May 21, 2020 at 01:08:43PM +0200, Marco Elver wrote:
> > > This patch series is the conclusion to [1], where we determined that due
> > > to various interactions with no_sanitize attributes and the new
> > > {READ,WRITE}_ONCE(), KCSAN will require Clang 11 or later. Other
> > > sanitizers are largely untouched, and only KCSAN now has a hard
> > > dependency on Clang 11. To test, a recent Clang development version will
> > > suffice [2]. While a little inconvenient for now, it is hoped that in
> > > future we may be able to fix GCC and re-enable GCC support.
> > >
> > > The patch "kcsan: Restrict supported compilers" contains a detailed list
> > > of requirements that led to this decision.
> > >
> > > Most of the patches are related to KCSAN, however, the first patch also
> > > includes an UBSAN related fix and is a dependency for the remaining
> > > ones. The last 2 patches clean up the attributes by moving them to the
> > > right place, and fix KASAN's way of defining __no_kasan_or_inline,
> > > making it consistent with KCSAN.
> > >
> > > The series has been tested by running kcsan-test several times and
> > > completed successfully.
> >
> > I've left a few minor comments, but the only one that probably needs a bit
> > of thought is using data_race() with const non-scalar expressions, since I
> > think that's now prohibited by these changes. We don't have too many
> > data_race() users yet, so probably not a big deal, but worth bearing in
> > mind.
>
> If you don't mind, I'll do a v3 with that fixed.
Works for me, thanks.
Will
^ permalink raw reply [flat|nested] 21+ messages in thread