Skip to contentSkip to conversation

arm64: avoid instrumenting atomic_ll_sc.o

PATCH New Mark Rutland 2 participants 3 messages Sep 26, 2017 – Sep 27, 2017 arm64

lore
Share & tools
lore

Mark Rutland

author reviewer
#0 lore raw

PATCH 1/1 arm64: avoid instrumenting atomic_ll_sc.o

Our out-of-line atomics are built with a special calling convention, preventing pointless stack spilling, and allowing us to patch call sites with ARMv8.1 atomic instructions.

Instrumentation inserted by the compiler may result in calls to functions not following this special calling convention, resulting in registers being unexpectedly clobbered, and various problems resulting from this.

For example, if a kernel is built with KCOV and ARM64_LSE_ATOMICS, the compiler inserts calls to __sanitizer_cov_trace_pc in the prologues of the atomic functions. This has been observed to result in spurious cmpxchg failures, leading to a hang early on in the boot process.

This patch avoids such issues by preventing instrumentation of our out-of-line atomics.

Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/lib/Makefile | 4 ++++
 1 file changed, 4 insertions(+)
arch/arm64/lib/Makefile +4 -0
index a0abc14..8fea178 100644
@@ -17,5 +17,9 @@ CFLAGS_atomic_ll_sc.o	:= -fcall-used-x0 -ffixed-x1 -ffixed-x2		\
 		   -fcall-saved-x10 -fcall-saved-x11 -fcall-saved-x12	\
 		   -fcall-saved-x13 -fcall-saved-x14 -fcall-saved-x15	\
 		   -fcall-saved-x18
+GCOV_PROFILE_atomic_ll_sc.o	:= n
+KASAN_SANITIZE_atomic_ll_sc.o	:= n
+KCOV_INSTRUMENT_atomic_ll_sc.o	:= n
+UBSAN_SANITIZE_atomic_ll_sc.o	:= n
 
 lib-$(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE) += uaccess_flushcache.o
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 arm64: avoid instrumenting atomic_ll_sc.o

Quoted 9 lines · Mark Rutland

Our out-of-line atomics are built with a special calling convention, preventing pointless stack spilling, and allowing us to patch call sites with ARMv8.1 atomic instructions.

Instrumentation inserted by the compiler may result in calls to functions not following this special calling convention, resulting in registers being unexpectedly clobbered, and various problems resulting from this.

Quoted 7 lines

For example, if a kernel is built with KCOV and ARM64_LSE_ATOMICS, the compiler inserts calls to __sanitizer_cov_trace_pc in the prologues of the atomic functions. This has been observed to result in spurious cmpxchg failures, leading to a hang early on in the boot process.

This patch avoids such issues by preventing instrumentation of our out-of-line atomics.

Why doesn't decorating them with "notrace" like we do via the __LL_SC_INLINE macro help here? Do we just need to extend that somehow?

Quoted 12 lines
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/lib/Makefile | 4 ++++
 1 file changed, 4 insertions(+)
arch/arm64/lib/Makefile +0 -0
index a0abc14..8fea178 100644

arch/arm64/lib/Makefile CFLAGS_atomic_ll_sc.o := -fcall-used-x0 -ffixed-x1 -ffixed-x2 \ · line 23

Quoted 7 lines
arch/arm64/lib/Makefile +4 -0
@@ -17,5 +17,9 @@ CFLAGS_atomic_ll_sc.o	:= -fcall-used-x0 -ffixed-x1 -ffixed-x2		\
 		   -fcall-saved-x10 -fcall-saved-x11 -fcall-saved-x12	\
 		   -fcall-saved-x13 -fcall-saved-x14 -fcall-saved-x15	\
 		   -fcall-saved-x18
+GCOV_PROFILE_atomic_ll_sc.o	:= n
+KASAN_SANITIZE_atomic_ll_sc.o	:= n
+KCOV_INSTRUMENT_atomic_ll_sc.o	:= n
+UBSAN_SANITIZE_atomic_ll_sc.o	:= n

Hmm... do we need similar things for the vDSO? I can only spot the GCOV entry, and other architectures have a mixed bag of these. It would be nice if there was something a little less ad-hoc and error prone provided by kbuild.

Will

author reviewer
#2 lore raw

PATCH 1/1 arm64: avoid instrumenting atomic_ll_sc.o

Quoted 15 lines · Will Deacon
Quoted 14 lines · Mark Rutland

Our out-of-line atomics are built with a special calling convention, preventing pointless stack spilling, and allowing us to patch call sites with ARMv8.1 atomic instructions.

Instrumentation inserted by the compiler may result in calls to functions not following this special calling convention, resulting in registers being unexpectedly clobbered, and various problems resulting from this.

For example, if a kernel is built with KCOV and ARM64_LSE_ATOMICS, the compiler inserts calls to __sanitizer_cov_trace_pc in the prologues of the atomic functions. This has been observed to result in spurious cmpxchg failures, leading to a hang early on in the boot process.

Quoted 5 lines
Quoted 2 lines

This patch avoids such issues by preventing instrumentation of our out-of-line atomics.

Why doesn't decorating them with "notrace" like we do via the __LL_SC_INLINE macro help here? Do we just need to extend that somehow?

AFAICT, notrace only guarantees that profiling function calls won't be generated, and that won't inhibit KASAN, UBSAN, KCOV, etc.

I think we need __noinstrument / NOINSTRUMENT_obj.o helpers that inhibit *all* automated instrumentation, since there are a sufficient number of places where we want that (vdso, efi stub, hyp, etc).

Quoted 7 lines
Quoted 7 lines
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
---
 arch/arm64/lib/Makefile | 4 ++++
 1 file changed, 4 insertions(+)
Quoted 14 lines

arch/arm64/lib/Makefile CFLAGS_atomic_ll_sc.o := -fcall-used-x0 -ffixed-x1 -ffixed-x2 \ · line 23

Quoted 12 lines
arch/arm64/lib/Makefile +4 -0
index a0abc14..8fea178 100644
@@ -17,5 +17,9 @@ CFLAGS_atomic_ll_sc.o	:= -fcall-used-x0 -ffixed-x1 -ffixed-x2		\
 		   -fcall-saved-x10 -fcall-saved-x11 -fcall-saved-x12	\
 		   -fcall-saved-x13 -fcall-saved-x14 -fcall-saved-x15	\
 		   -fcall-saved-x18
+GCOV_PROFILE_atomic_ll_sc.o	:= n
+KASAN_SANITIZE_atomic_ll_sc.o	:= n
+KCOV_INSTRUMENT_atomic_ll_sc.o	:= n
+UBSAN_SANITIZE_atomic_ll_sc.o	:= n

Hmm... do we need similar things for the vDSO?

If it were written in C, yes.

Quoted 2 lines

I can only spot the GCOV entry, and other architectures have a mixed bag of these.

I don't think we need that currently, given our vDSO is written in assembly. I think we cargo-culted that from x86, where the vDSO is written in C.

Quoted 2 lines

It would be nice if there was something a little less ad-hoc and error prone provided by kbuild.

I completely agree. I'll have a go at the noinstrument approach above.

Thanks,
Mark.