| Message ID | 20230216050938.2188488-1-rmclure@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems | expand |
Le 16/02/2023 à 06:09, Rohan McLure a écrit : > KCSAN instruments calls to atomic builtins, and will in turn call these > builtins itself. As such, architectures supporting KCSAN must have > compiler support for these atomic primitives. > > Since 32-bit systems are unlikely to have 64-bit compiler builtins, > provide a stub for each missing builtin, and use BUG() to assert > unreachability. > > In commit 725aea873261 ("xtensa: enable KCSAN"), xtensa implements these > locally. Move these definitions to be accessible to all 32-bit > architectures that do not provide the necessary builtins, with opt in > for PowerPC and xtensa. > > Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> > Reviewed-by: Max Filippov <jcmvbkbc@gmail.com> This series should also be addressed to KCSAN Maintainers, shouldn't it ? KCSAN M: Marco Elver <elver@google.com> R: Dmitry Vyukov <dvyukov@google.com> L: kasan-dev@googlegroups.com S: Maintained F: Documentation/dev-tools/kcsan.rst F: include/linux/kcsan*.h F: kernel/kcsan/ F: lib/Kconfig.kcsan F: scripts/Makefile.kcsan > --- > Previously issued as a part of a patch series adding KCSAN support to > 64-bit. > Link: https://lore.kernel.org/linuxppc-dev/167646486000.1421441.10070059569986228558.b4-ty@ellerman.id.au/T/#t > v1: Remove __has_builtin check, as gcc is not obligated to inline > builtins detected using this check, but instead is permitted to supply > them in libatomic: > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108734 > Instead, opt-in PPC32 and xtensa. > --- > arch/xtensa/lib/Makefile | 1 - > kernel/kcsan/Makefile | 2 ++ > arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c | 0 > 3 files changed, 2 insertions(+), 1 deletion(-) > rename arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c (100%) > > diff --git a/arch/xtensa/lib/Makefile b/arch/xtensa/lib/Makefile > index 7ecef0519a27..d69356dc97df 100644 > --- a/arch/xtensa/lib/Makefile > +++ b/arch/xtensa/lib/Makefile > @@ -8,5 +8,4 @@ lib-y += memcopy.o memset.o checksum.o \ > divsi3.o udivsi3.o modsi3.o umodsi3.o mulsi3.o umulsidi3.o \ > usercopy.o strncpy_user.o strnlen_user.o > lib-$(CONFIG_PCI) += pci-auto.o > -lib-$(CONFIG_KCSAN) += kcsan-stubs.o > KCSAN_SANITIZE_kcsan-stubs.o := n > diff --git a/kernel/kcsan/Makefile b/kernel/kcsan/Makefile > index 8cf70f068d92..86dd713d8855 100644 > --- a/kernel/kcsan/Makefile > +++ b/kernel/kcsan/Makefile > @@ -12,6 +12,8 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \ > -fno-stack-protector -DDISABLE_BRANCH_PROFILING > > obj-y := core.o debugfs.o report.o > +obj-$(CONFIG_PPC32) += stubs.o > +obj-$(CONFIG_XTENSA) += stubs.o Not sure it is acceptable to do it that way. There should likely be something like a CONFIG_ARCH_WANTS_KCSAN_STUBS in KCSAN's Kconfig then PPC32 and XTENSA should select it. > > KCSAN_INSTRUMENT_BARRIERS_selftest.o := y > obj-$(CONFIG_KCSAN_SELFTEST) += selftest.o > diff --git a/arch/xtensa/lib/kcsan-stubs.c b/kernel/kcsan/stubs.c > similarity index 100% > rename from arch/xtensa/lib/kcsan-stubs.c > rename to kernel/kcsan/stubs.c
On Thu, Feb 16, 2023 at 07:12AM +0000, Christophe Leroy wrote: > > > Le 16/02/2023 à 06:09, Rohan McLure a écrit : > > KCSAN instruments calls to atomic builtins, and will in turn call these > > builtins itself. As such, architectures supporting KCSAN must have > > compiler support for these atomic primitives. > > > > Since 32-bit systems are unlikely to have 64-bit compiler builtins, > > provide a stub for each missing builtin, and use BUG() to assert > > unreachability. > > > > In commit 725aea873261 ("xtensa: enable KCSAN"), xtensa implements these > > locally. Move these definitions to be accessible to all 32-bit > > architectures that do not provide the necessary builtins, with opt in > > for PowerPC and xtensa. > > > > Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> > > Reviewed-by: Max Filippov <jcmvbkbc@gmail.com> > > This series should also be addressed to KCSAN Maintainers, shouldn't it ? > > KCSAN > M: Marco Elver <elver@google.com> > R: Dmitry Vyukov <dvyukov@google.com> > L: kasan-dev@googlegroups.com > S: Maintained > F: Documentation/dev-tools/kcsan.rst > F: include/linux/kcsan*.h > F: kernel/kcsan/ > F: lib/Kconfig.kcsan > F: scripts/Makefile.kcsan > > > > --- > > Previously issued as a part of a patch series adding KCSAN support to > > 64-bit. > > Link: https://lore.kernel.org/linuxppc-dev/167646486000.1421441.10070059569986228558.b4-ty@ellerman.id.au/T/#t > > v1: Remove __has_builtin check, as gcc is not obligated to inline > > builtins detected using this check, but instead is permitted to supply > > them in libatomic: > > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108734 > > Instead, opt-in PPC32 and xtensa. > > --- > > arch/xtensa/lib/Makefile | 1 - > > kernel/kcsan/Makefile | 2 ++ > > arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c | 0 > > 3 files changed, 2 insertions(+), 1 deletion(-) > > rename arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c (100%) > > > > diff --git a/arch/xtensa/lib/Makefile b/arch/xtensa/lib/Makefile > > index 7ecef0519a27..d69356dc97df 100644 > > --- a/arch/xtensa/lib/Makefile > > +++ b/arch/xtensa/lib/Makefile > > @@ -8,5 +8,4 @@ lib-y += memcopy.o memset.o checksum.o \ > > divsi3.o udivsi3.o modsi3.o umodsi3.o mulsi3.o umulsidi3.o \ > > usercopy.o strncpy_user.o strnlen_user.o > > lib-$(CONFIG_PCI) += pci-auto.o > > -lib-$(CONFIG_KCSAN) += kcsan-stubs.o > > KCSAN_SANITIZE_kcsan-stubs.o := n > > diff --git a/kernel/kcsan/Makefile b/kernel/kcsan/Makefile > > index 8cf70f068d92..86dd713d8855 100644 > > --- a/kernel/kcsan/Makefile > > +++ b/kernel/kcsan/Makefile > > @@ -12,6 +12,8 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \ > > -fno-stack-protector -DDISABLE_BRANCH_PROFILING > > > > obj-y := core.o debugfs.o report.o > > +obj-$(CONFIG_PPC32) += stubs.o > > +obj-$(CONFIG_XTENSA) += stubs.o > > Not sure it is acceptable to do it that way. > > There should likely be something like a CONFIG_ARCH_WANTS_KCSAN_STUBS in > KCSAN's Kconfig then PPC32 and XTENSA should select it. The longer I think about it, since these stubs all BUG() anyway, perhaps we ought to just avoid them altogether. If you delete all the stubs from ppc and xtensa, but do this: | diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c | index 54d077e1a2dc..8169d6dadd0e 100644 | --- a/kernel/kcsan/core.c | +++ b/kernel/kcsan/core.c | @@ -1261,7 +1261,9 @@ static __always_inline void kcsan_atomic_builtin_memorder(int memorder) | DEFINE_TSAN_ATOMIC_OPS(8); | DEFINE_TSAN_ATOMIC_OPS(16); | DEFINE_TSAN_ATOMIC_OPS(32); | +#ifdef CONFIG_64BIT | DEFINE_TSAN_ATOMIC_OPS(64); | +#endif | | void __tsan_atomic_thread_fence(int memorder); | void __tsan_atomic_thread_fence(int memorder) Does that work?
> On 16 Feb 2023, at 7:09 pm, Marco Elver <elver@google.com> wrote: > > On Thu, Feb 16, 2023 at 07:12AM +0000, Christophe Leroy wrote: >> >> >> Le 16/02/2023 à 06:09, Rohan McLure a écrit : >>> KCSAN instruments calls to atomic builtins, and will in turn call these >>> builtins itself. As such, architectures supporting KCSAN must have >>> compiler support for these atomic primitives. >>> >>> Since 32-bit systems are unlikely to have 64-bit compiler builtins, >>> provide a stub for each missing builtin, and use BUG() to assert >>> unreachability. >>> >>> In commit 725aea873261 ("xtensa: enable KCSAN"), xtensa implements these >>> locally. Move these definitions to be accessible to all 32-bit >>> architectures that do not provide the necessary builtins, with opt in >>> for PowerPC and xtensa. >>> >>> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> >>> Reviewed-by: Max Filippov <jcmvbkbc@gmail.com> >> >> This series should also be addressed to KCSAN Maintainers, shouldn't it ? >> >> KCSAN >> M: Marco Elver <elver@google.com> >> R: Dmitry Vyukov <dvyukov@google.com> >> L: kasan-dev@googlegroups.com >> S: Maintained >> F: Documentation/dev-tools/kcsan.rst >> F: include/linux/kcsan*.h >> F: kernel/kcsan/ >> F: lib/Kconfig.kcsan >> F: scripts/Makefile.kcsan >> >> >>> --- >>> Previously issued as a part of a patch series adding KCSAN support to >>> 64-bit. >>> Link: https://lore.kernel.org/linuxppc-dev/167646486000.1421441.10070059569986228558.b4-ty@ellerman.id.au/T/#t >>> v1: Remove __has_builtin check, as gcc is not obligated to inline >>> builtins detected using this check, but instead is permitted to supply >>> them in libatomic: >>> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108734 >>> Instead, opt-in PPC32 and xtensa. >>> --- >>> arch/xtensa/lib/Makefile | 1 - >>> kernel/kcsan/Makefile | 2 ++ >>> arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c | 0 >>> 3 files changed, 2 insertions(+), 1 deletion(-) >>> rename arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c (100%) >>> >>> diff --git a/arch/xtensa/lib/Makefile b/arch/xtensa/lib/Makefile >>> index 7ecef0519a27..d69356dc97df 100644 >>> --- a/arch/xtensa/lib/Makefile >>> +++ b/arch/xtensa/lib/Makefile >>> @@ -8,5 +8,4 @@ lib-y += memcopy.o memset.o checksum.o \ >>> divsi3.o udivsi3.o modsi3.o umodsi3.o mulsi3.o umulsidi3.o \ >>> usercopy.o strncpy_user.o strnlen_user.o >>> lib-$(CONFIG_PCI) += pci-auto.o >>> -lib-$(CONFIG_KCSAN) += kcsan-stubs.o >>> KCSAN_SANITIZE_kcsan-stubs.o := n >>> diff --git a/kernel/kcsan/Makefile b/kernel/kcsan/Makefile >>> index 8cf70f068d92..86dd713d8855 100644 >>> --- a/kernel/kcsan/Makefile >>> +++ b/kernel/kcsan/Makefile >>> @@ -12,6 +12,8 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \ >>> -fno-stack-protector -DDISABLE_BRANCH_PROFILING >>> >>> obj-y := core.o debugfs.o report.o >>> +obj-$(CONFIG_PPC32) += stubs.o >>> +obj-$(CONFIG_XTENSA) += stubs.o >> >> Not sure it is acceptable to do it that way. >> >> There should likely be something like a CONFIG_ARCH_WANTS_KCSAN_STUBS in >> KCSAN's Kconfig then PPC32 and XTENSA should select it. > > The longer I think about it, since these stubs all BUG() anyway, perhaps > we ought to just avoid them altogether. If you delete all the stubs from > ppc and xtensa, but do this: > > | diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c > | index 54d077e1a2dc..8169d6dadd0e 100644 > | --- a/kernel/kcsan/core.c > | +++ b/kernel/kcsan/core.c > | @@ -1261,7 +1261,9 @@ static __always_inline void kcsan_atomic_builtin_memorder(int memorder) > | DEFINE_TSAN_ATOMIC_OPS(8); > | DEFINE_TSAN_ATOMIC_OPS(16); > | DEFINE_TSAN_ATOMIC_OPS(32); > | +#ifdef CONFIG_64BIT > | DEFINE_TSAN_ATOMIC_OPS(64); > | +#endif > | > | void __tsan_atomic_thread_fence(int memorder); > | void __tsan_atomic_thread_fence(int memorder) > > Does that work? This makes much more sense. Rather than assume that kcsan is the only consumer of __atomic_*_8, and stubbing accordingly, we should just remove its mention from relevant sub-archs.
diff --git a/arch/xtensa/lib/Makefile b/arch/xtensa/lib/Makefile index 7ecef0519a27..d69356dc97df 100644 --- a/arch/xtensa/lib/Makefile +++ b/arch/xtensa/lib/Makefile @@ -8,5 +8,4 @@ lib-y += memcopy.o memset.o checksum.o \ divsi3.o udivsi3.o modsi3.o umodsi3.o mulsi3.o umulsidi3.o \ usercopy.o strncpy_user.o strnlen_user.o lib-$(CONFIG_PCI) += pci-auto.o -lib-$(CONFIG_KCSAN) += kcsan-stubs.o KCSAN_SANITIZE_kcsan-stubs.o := n diff --git a/kernel/kcsan/Makefile b/kernel/kcsan/Makefile index 8cf70f068d92..86dd713d8855 100644 --- a/kernel/kcsan/Makefile +++ b/kernel/kcsan/Makefile @@ -12,6 +12,8 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \ -fno-stack-protector -DDISABLE_BRANCH_PROFILING obj-y := core.o debugfs.o report.o +obj-$(CONFIG_PPC32) += stubs.o +obj-$(CONFIG_XTENSA) += stubs.o KCSAN_INSTRUMENT_BARRIERS_selftest.o := y obj-$(CONFIG_KCSAN_SELFTEST) += selftest.o diff --git a/arch/xtensa/lib/kcsan-stubs.c b/kernel/kcsan/stubs.c similarity index 100% rename from arch/xtensa/lib/kcsan-stubs.c rename to kernel/kcsan/stubs.c
KCSAN instruments calls to atomic builtins, and will in turn call these builtins itself. As such, architectures supporting KCSAN must have compiler support for these atomic primitives. Since 32-bit systems are unlikely to have 64-bit compiler builtins, provide a stub for each missing builtin, and use BUG() to assert unreachability. In commit 725aea873261 ("xtensa: enable KCSAN"), xtensa implements these locally. Move these definitions to be accessible to all 32-bit architectures that do not provide the necessary builtins, with opt in for PowerPC and xtensa. Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> Reviewed-by: Max Filippov <jcmvbkbc@gmail.com> --- Previously issued as a part of a patch series adding KCSAN support to 64-bit. Link: https://lore.kernel.org/linuxppc-dev/167646486000.1421441.10070059569986228558.b4-ty@ellerman.id.au/T/#t v1: Remove __has_builtin check, as gcc is not obligated to inline builtins detected using this check, but instead is permitted to supply them in libatomic: Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108734 Instead, opt-in PPC32 and xtensa. --- arch/xtensa/lib/Makefile | 1 - kernel/kcsan/Makefile | 2 ++ arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c | 0 3 files changed, 2 insertions(+), 1 deletion(-) rename arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c (100%)