[PATCH 0/4] Cleanup instrumentation avoidance

[PATCH 0/4] Cleanup instrumentation avoidance

[Mark Rutland]

Recently we spotted that some code with special ABI requirements was
being instrumented by KCOV [1], and we need to avoid all instrumentation
in this case.

While it's possible to disable each instrumentation option manually,
this is tedious and error prone as new options are introduced.

This series adds a new CC_INSTRUMENT option which can be used to disable
all instrumentation of a given file or directory, and makes use of this
throught arm64.

There are other places where this could be used (e.g. the EFI stub), but
I've avoided altering those for now in the hope that this can be taken
via the arm64 tree.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-September/533105.html

Mark Rutland (4):
  kbuild: allow global override of CC instrumentation
  arm64: prevent instrumentation of LL/SC atomics
  kvm/arm64: simplify CC instrumentation opt-out
  arm64: vdso: remove pointless gcov option

 arch/arm64/kernel/vdso/Makefile |  3 ---
 arch/arm64/kvm/hyp/Makefile     |  5 +----
 arch/arm64/lib/Makefile         |  1 +
 scripts/Makefile.lib            | 17 +++++++++++++----
 4 files changed, 15 insertions(+), 11 deletions(-)

-- 
2.11.0

[PATCH 1/4] kbuild: allow global override of CC instrumentation

[Mark Rutland]

Currently, we have a number of compiler instrumentation features, each
with its own overrides to disable instrumentation of a file or
directory.

In a few cases, there are files with special ABI requirements, for which
we need to avoid all instrumentation. Having to apply each override
manually is tedious, and error-prone as new instrumentation features are
introduced.

To make matters easier, this patch adds a new CC_INSTRUMENT override,
allowing instrumentation to be avoided for certain files or directories.
This can also be overridden on a per-file or per-directory basis, to
allow opting in to some instrumentation.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Andrey Konovalov <adech.fo@gmail.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <mmarek@suse.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: linux-kbuild@vger.kernel.org
---
 scripts/Makefile.lib | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 5e975fee0f5b..c7d71f482b57 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -109,6 +109,15 @@ orig_a_flags   = $(KBUILD_CPPFLAGS) $(KBUILD_AFLAGS) $(KBUILD_SUBDIR_ASFLAGS) \
 _a_flags       = $(filter-out $(AFLAGS_REMOVE_$(basetarget).o), $(orig_a_flags))
 _cpp_flags     = $(KBUILD_CPPFLAGS) $(cppflags-y) $(CPPFLAGS_$(@F))
 
+# Each of the compiler instrumentation features below can be selectively
+# enabled or disable for files or directoies. In decreasing order of
+# predence, each option foo is controlled by:
+# FOO_obj.o := [yn]
+# CC_INSTRUMENT_obj.o := [yn]
+# FOO := [yn]
+# CC_INSTRUMENT := [yn]
+# FOO_all := [yn]
+
 #
 # Enable gcov profiling flags for a file, directory or for all files depending
 # on variables GCOV_PROFILE_obj.o, GCOV_PROFILE and CONFIG_GCOV_PROFILE_ALL
@@ -116,7 +125,7 @@ _cpp_flags     = $(KBUILD_CPPFLAGS) $(cppflags-y) $(CPPFLAGS_$(@F))
 #
 ifeq ($(CONFIG_GCOV_KERNEL),y)
 _c_flags += $(if $(patsubst n%,, \
-		$(GCOV_PROFILE_$(basetarget).o)$(GCOV_PROFILE)$(CONFIG_GCOV_PROFILE_ALL)), \
+		$(GCOV_PROFILE_$(basetarget).o)$(CC_INSTRUMENT_$(basetarget).o)$(GCOV_PROFILE)$(CC_INSTRUMENT)$(CONFIG_GCOV_PROFILE_ALL)), \
 		$(CFLAGS_GCOV))
 endif
 
@@ -126,19 +135,19 @@ endif
 #
 ifeq ($(CONFIG_KASAN),y)
 _c_flags += $(if $(patsubst n%,, \
-		$(KASAN_SANITIZE_$(basetarget).o)$(KASAN_SANITIZE)y), \
+		$(KASAN_SANITIZE_$(basetarget).o)$(CC_INSTRUMENT_$(basetarget).o)$(KASAN_SANITIZE)$(CC_INSTRUMENT)y), \
 		$(CFLAGS_KASAN))
 endif
 
 ifeq ($(CONFIG_UBSAN),y)
 _c_flags += $(if $(patsubst n%,, \
-		$(UBSAN_SANITIZE_$(basetarget).o)$(UBSAN_SANITIZE)$(CONFIG_UBSAN_SANITIZE_ALL)), \
+		$(UBSAN_SANITIZE_$(basetarget).o)$(CC_INSTRUMENT_$(basetarget).o)$(UBSAN_SANITIZE)$(CC_INSTRUMENT)$(CONFIG_UBSAN_SANITIZE_ALL)), \
 		$(CFLAGS_UBSAN))
 endif
 
 ifeq ($(CONFIG_KCOV),y)
 _c_flags += $(if $(patsubst n%,, \
-	$(KCOV_INSTRUMENT_$(basetarget).o)$(KCOV_INSTRUMENT)$(CONFIG_KCOV_INSTRUMENT_ALL)), \
+	$(KCOV_INSTRUMENT_$(basetarget).o)$(CC_INSTRUMENT_$(basetarget).o)$(KCOV_INSTRUMENT)$(CC_INSTRUMENT)$(CONFIG_KCOV_INSTRUMENT_ALL)), \
 	$(CFLAGS_KCOV))
 endif
 
-- 
2.11.0

[PATCH 2/4] arm64: prevent instrumentation of LL/SC atomics

[Mark Rutland]

While we build the LL/SC atomics as a C object file, this does not
follow the AAPCS. This does not interoperate with other C code, and can
only be called from special wrapper assembly.

Bulding a kernel with CONFIG_KCOV and CONFIG_ARM64_LSE_ATOMICS results
in the cmopiler inserting calls to __sanitizer_cov_trace_pc within the
LL/SC atomics. As __sanitizer_cov_trace_pc is built per the AAPCS, these
calls corrupt register values, resulting in failures at boot time.

Avoid this (and other similar issues) by opting out of all compiler
instrumentation. We can opt-in to specific instrumentation in future if
we want to.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/lib/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
index a0abc142c92b..af77516f71b2 100644
--- a/arch/arm64/lib/Makefile
+++ b/arch/arm64/lib/Makefile
@@ -17,5 +17,6 @@ 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
+CC_INSTRUMENT_atomic_ll_sc.o := n
 
 lib-$(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE) += uaccess_flushcache.o
-- 
2.11.0

[PATCH 3/4] kvm/arm64: simplify CC instrumentation opt-out

[Mark Rutland]

Now that we can opt out of all instrumentation with a single option,
make use of this in the KVM hyp code. Hopefully, this will also avoid
fragility as new options are introduced.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Marc Zyngier <mark.zyngier@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kvm/hyp/Makefile | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
index 14c4e3b14bcb..a8a212eb66f1 100644
--- a/arch/arm64/kvm/hyp/Makefile
+++ b/arch/arm64/kvm/hyp/Makefile
@@ -22,7 +22,4 @@ obj-$(CONFIG_KVM_ARM_HOST) += s2-setup.o
 # KVM code is run at a different exception code with a different map, so
 # compiler instrumentation that inserts callbacks or checks into the code may
 # cause crashes. Just disable it.
-GCOV_PROFILE	:= n
-KASAN_SANITIZE	:= n
-UBSAN_SANITIZE	:= n
-KCOV_INSTRUMENT	:= n
+CC_INSTRUMENT	:= n
-- 
2.11.0

[PATCH 4/4] arm64: vdso: remove pointless gcov option

[Mark Rutland]

The arm64 VDSO Makefile explicitly disables gcov; a practice we seem to
have inherited from other VDSO Makefiles.

The arm64 VDSO is written entirely in assembly, so this is not
necessary. Further, if the VDSO were written in C it would be necessary
to disable all other instrumentation, and this alone would not be
sufficient.

This patch remosves the redundant GCOV option for now.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Reported-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/kernel/vdso/Makefile | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
index 62c84f7cb01b..350190b749dd 100644
--- a/arch/arm64/kernel/vdso/Makefile
+++ b/arch/arm64/kernel/vdso/Makefile
@@ -15,9 +15,6 @@ ccflags-y := -shared -fno-common -fno-builtin
 ccflags-y += -nostdlib -Wl,-soname=linux-vdso.so.1 \
 		$(call cc-ldoption, -Wl$(comma)--hash-style=sysv)
 
-# Disable gcov profiling for VDSO code
-GCOV_PROFILE := n
-
 # Workaround for bare-metal (ELF) toolchains that neglect to pass -shared
 # down to collect2, resulting in silent corruption of the vDSO image.
 ccflags-y += -Wl,-shared
-- 
2.11.0

Re: [PATCH 0/4] Cleanup instrumentation avoidance

[Mark Rutland]

Sorry Marc, I somehow typo'd your name when sending this out...

Mark.

On Mon, Oct 16, 2017 at 02:24:36PM +0100, Mark Rutland wrote:
> Recently we spotted that some code with special ABI requirements was
> being instrumented by KCOV [1], and we need to avoid all instrumentation
> in this case.
> 
> While it's possible to disable each instrumentation option manually,
> this is tedious and error prone as new options are introduced.
> 
> This series adds a new CC_INSTRUMENT option which can be used to disable
> all instrumentation of a given file or directory, and makes use of this
> throught arm64.
> 
> There are other places where this could be used (e.g. the EFI stub), but
> I've avoided altering those for now in the hope that this can be taken
> via the arm64 tree.
> 
> Thanks,
> Mark.
> 
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-September/533105.html
> 
> Mark Rutland (4):
>   kbuild: allow global override of CC instrumentation
>   arm64: prevent instrumentation of LL/SC atomics
>   kvm/arm64: simplify CC instrumentation opt-out
>   arm64: vdso: remove pointless gcov option
> 
>  arch/arm64/kernel/vdso/Makefile |  3 ---
>  arch/arm64/kvm/hyp/Makefile     |  5 +----
>  arch/arm64/lib/Makefile         |  1 +
>  scripts/Makefile.lib            | 17 +++++++++++++----
>  4 files changed, 15 insertions(+), 11 deletions(-)
> 
> -- 
> 2.11.0
> 

Re: [PATCH 2/4] arm64: prevent instrumentation of LL/SC atomics

[Will Deacon]

On Mon, Oct 16, 2017 at 02:24:38PM +0100, Mark Rutland wrote:
> While we build the LL/SC atomics as a C object file, this does not
> follow the AAPCS. This does not interoperate with other C code, and can
> only be called from special wrapper assembly.
> 
> Bulding a kernel with CONFIG_KCOV and CONFIG_ARM64_LSE_ATOMICS results
> in the cmopiler inserting calls to __sanitizer_cov_trace_pc within the

cmopiler

> LL/SC atomics. As __sanitizer_cov_trace_pc is built per the AAPCS, these
> calls corrupt register values, resulting in failures at boot time.
> 
> Avoid this (and other similar issues) by opting out of all compiler
> instrumentation. We can opt-in to specific instrumentation in future if
> we want to.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/lib/Makefile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> index a0abc142c92b..af77516f71b2 100644
> --- a/arch/arm64/lib/Makefile
> +++ b/arch/arm64/lib/Makefile
> @@ -17,5 +17,6 @@ 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
> +CC_INSTRUMENT_atomic_ll_sc.o := n

Does this mean we can lose the "notrace" definition of __LL_SC_INLINE
when generating the out-of-line atomics?

Will

Re: [PATCH 1/4] kbuild: allow global override of CC instrumentation

[Will Deacon]

On Mon, Oct 16, 2017 at 02:24:37PM +0100, Mark Rutland wrote:
> Currently, we have a number of compiler instrumentation features, each
> with its own overrides to disable instrumentation of a file or
> directory.
> 
> In a few cases, there are files with special ABI requirements, for which
> we need to avoid all instrumentation. Having to apply each override
> manually is tedious, and error-prone as new instrumentation features are
> introduced.
> 
> To make matters easier, this patch adds a new CC_INSTRUMENT override,
> allowing instrumentation to be avoided for certain files or directories.
> This can also be overridden on a per-file or per-directory basis, to
> allow opting in to some instrumentation.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Andrey Konovalov <adech.fo@gmail.com>
> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Michal Marek <mmarek@suse.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: linux-kbuild@vger.kernel.org
> ---
>  scripts/Makefile.lib | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 5e975fee0f5b..c7d71f482b57 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -109,6 +109,15 @@ orig_a_flags   = $(KBUILD_CPPFLAGS) $(KBUILD_AFLAGS) $(KBUILD_SUBDIR_ASFLAGS) \
>  _a_flags       = $(filter-out $(AFLAGS_REMOVE_$(basetarget).o), $(orig_a_flags))
>  _cpp_flags     = $(KBUILD_CPPFLAGS) $(cppflags-y) $(CPPFLAGS_$(@F))
>  
> +# Each of the compiler instrumentation features below can be selectively
> +# enabled or disable for files or directoies. In decreasing order of

Should be "disabled". Also, typo: "directoies".

> +# predence, each option foo is controlled by:

Typo: "predence"

> +# FOO_obj.o := [yn]
> +# CC_INSTRUMENT_obj.o := [yn]
> +# FOO := [yn]
> +# CC_INSTRUMENT := [yn]
> +# FOO_all := [yn]

I'm not sure it's worth mentioning FOO_all here: for KASAN it's just 'y'
and for the others it's driven by a CONFIG option that would be better
controlled using Kconfig rather than in Makefiles.

Otherwise,

Acked-by: Will Deacon <will.deacon@arm.com>

Will

Re: [PATCH 2/4] arm64: prevent instrumentation of LL/SC atomics

[Mark Rutland]

On Tue, Oct 17, 2017 at 11:03:15AM +0100, Will Deacon wrote:
> On Mon, Oct 16, 2017 at 02:24:38PM +0100, Mark Rutland wrote:
> > While we build the LL/SC atomics as a C object file, this does not
> > follow the AAPCS. This does not interoperate with other C code, and can
> > only be called from special wrapper assembly.
> > 
> > Bulding a kernel with CONFIG_KCOV and CONFIG_ARM64_LSE_ATOMICS results
> > in the cmopiler inserting calls to __sanitizer_cov_trace_pc within the
> 
> cmopiler

Bulding, too. ;)

> > LL/SC atomics. As __sanitizer_cov_trace_pc is built per the AAPCS, these
> > calls corrupt register values, resulting in failures at boot time.
> > 
> > Avoid this (and other similar issues) by opting out of all compiler
> > instrumentation. We can opt-in to specific instrumentation in future if
> > we want to.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > ---
> >  arch/arm64/lib/Makefile | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> > index a0abc142c92b..af77516f71b2 100644
> > --- a/arch/arm64/lib/Makefile
> > +++ b/arch/arm64/lib/Makefile
> > @@ -17,5 +17,6 @@ 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
> > +CC_INSTRUMENT_atomic_ll_sc.o := n
> 
> Does this mean we can lose the "notrace" definition of __LL_SC_INLINE
> when generating the out-of-line atomics?

Unfortunately not.

I'd missed -pg, since that isn't handled in scripts/Makefile.lib, and
doesn't seem to have a makefile-level disable.

I'll see if that can be remedied.

Thanks,
Mark.

Re: [PATCH 2/4] arm64: prevent instrumentation of LL/SC atomics

[Will Deacon]

On Tue, Oct 17, 2017 at 11:54:54AM +0100, Mark Rutland wrote:
> On Tue, Oct 17, 2017 at 11:03:15AM +0100, Will Deacon wrote:
> > On Mon, Oct 16, 2017 at 02:24:38PM +0100, Mark Rutland wrote:
> > > While we build the LL/SC atomics as a C object file, this does not
> > > follow the AAPCS. This does not interoperate with other C code, and can
> > > only be called from special wrapper assembly.
> > > 
> > > Bulding a kernel with CONFIG_KCOV and CONFIG_ARM64_LSE_ATOMICS results
> > > in the cmopiler inserting calls to __sanitizer_cov_trace_pc within the
> > 
> > cmopiler
> 
> Bulding, too. ;)
> 
> > > LL/SC atomics. As __sanitizer_cov_trace_pc is built per the AAPCS, these
> > > calls corrupt register values, resulting in failures at boot time.
> > > 
> > > Avoid this (and other similar issues) by opting out of all compiler
> > > instrumentation. We can opt-in to specific instrumentation in future if
> > > we want to.
> > > 
> > > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > Cc: Will Deacon <will.deacon@arm.com>
> > > ---
> > >  arch/arm64/lib/Makefile | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> > > index a0abc142c92b..af77516f71b2 100644
> > > --- a/arch/arm64/lib/Makefile
> > > +++ b/arch/arm64/lib/Makefile
> > > @@ -17,5 +17,6 @@ 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
> > > +CC_INSTRUMENT_atomic_ll_sc.o := n
> > 
> > Does this mean we can lose the "notrace" definition of __LL_SC_INLINE
> > when generating the out-of-line atomics?
> 
> Unfortunately not.
> 
> I'd missed -pg, since that isn't handled in scripts/Makefile.lib, and
> doesn't seem to have a makefile-level disable.
> 
> I'll see if that can be remedied.

Thanks. It's a real shame to have a "just use this option to avoid
instrumentation" if it doesn't actually catch everything. We probably
need to think about kprobes too, but not really sure what you can do there
on a per-file basis.

Will

Re: [PATCH 2/4] arm64: prevent instrumentation of LL/SC atomics

[Mark Rutland]

On Tue, Oct 17, 2017 at 11:58:58AM +0100, Will Deacon wrote:
> On Tue, Oct 17, 2017 at 11:54:54AM +0100, Mark Rutland wrote:
> > On Tue, Oct 17, 2017 at 11:03:15AM +0100, Will Deacon wrote:
> > > On Mon, Oct 16, 2017 at 02:24:38PM +0100, Mark Rutland wrote:
> > > > While we build the LL/SC atomics as a C object file, this does not
> > > > follow the AAPCS. This does not interoperate with other C code, and can
> > > > only be called from special wrapper assembly.
> > > > 
> > > > Bulding a kernel with CONFIG_KCOV and CONFIG_ARM64_LSE_ATOMICS results
> > > > in the cmopiler inserting calls to __sanitizer_cov_trace_pc within the
> > > > LL/SC atomics. As __sanitizer_cov_trace_pc is built per the AAPCS, these
> > > > calls corrupt register values, resulting in failures at boot time.
> > > > 
> > > > Avoid this (and other similar issues) by opting out of all compiler
> > > > instrumentation. We can opt-in to specific instrumentation in future if
> > > > we want to.

> > > > diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> > > > index a0abc142c92b..af77516f71b2 100644
> > > > --- a/arch/arm64/lib/Makefile
> > > > +++ b/arch/arm64/lib/Makefile
> > > > @@ -17,5 +17,6 @@ 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
> > > > +CC_INSTRUMENT_atomic_ll_sc.o := n
> > > 
> > > Does this mean we can lose the "notrace" definition of __LL_SC_INLINE
> > > when generating the out-of-line atomics?
> > 
> > Unfortunately not.
> > 
> > I'd missed -pg, since that isn't handled in scripts/Makefile.lib, and
> > doesn't seem to have a makefile-level disable.
> > 
> > I'll see if that can be remedied.
> 
> Thanks. It's a real shame to have a "just use this option to avoid
> instrumentation" if it doesn't actually catch everything. 

Agreed; it defeats the purpose of the exercise.

> We probably need to think about kprobes too, but not really sure what
> you can do there on a per-file basis.

Ugh; that's a much more painful one, yes. :(

Does that rely on any compiler options at all? I thought was all a
runtime thing.

Arguably it is somewhat separate for compiler instrumentation, and it
might make sense for that to be a separate option.

Thanks,
Mark.

Re: [PATCH 2/4] arm64: prevent instrumentation of LL/SC atomics

[Will Deacon]

On Tue, Oct 17, 2017 at 12:10:33PM +0100, Mark Rutland wrote:
> On Tue, Oct 17, 2017 at 11:58:58AM +0100, Will Deacon wrote:
> > On Tue, Oct 17, 2017 at 11:54:54AM +0100, Mark Rutland wrote:
> > > On Tue, Oct 17, 2017 at 11:03:15AM +0100, Will Deacon wrote:
> > > > On Mon, Oct 16, 2017 at 02:24:38PM +0100, Mark Rutland wrote:
> > > > > While we build the LL/SC atomics as a C object file, this does not
> > > > > follow the AAPCS. This does not interoperate with other C code, and can
> > > > > only be called from special wrapper assembly.
> > > > > 
> > > > > Bulding a kernel with CONFIG_KCOV and CONFIG_ARM64_LSE_ATOMICS results
> > > > > in the cmopiler inserting calls to __sanitizer_cov_trace_pc within the
> > > > > LL/SC atomics. As __sanitizer_cov_trace_pc is built per the AAPCS, these
> > > > > calls corrupt register values, resulting in failures at boot time.
> > > > > 
> > > > > Avoid this (and other similar issues) by opting out of all compiler
> > > > > instrumentation. We can opt-in to specific instrumentation in future if
> > > > > we want to.
> 
> > > > > diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> > > > > index a0abc142c92b..af77516f71b2 100644
> > > > > --- a/arch/arm64/lib/Makefile
> > > > > +++ b/arch/arm64/lib/Makefile
> > > > > @@ -17,5 +17,6 @@ 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
> > > > > +CC_INSTRUMENT_atomic_ll_sc.o := n
> > > > 
> > > > Does this mean we can lose the "notrace" definition of __LL_SC_INLINE
> > > > when generating the out-of-line atomics?
> > > 
> > > Unfortunately not.
> > > 
> > > I'd missed -pg, since that isn't handled in scripts/Makefile.lib, and
> > > doesn't seem to have a makefile-level disable.
> > > 
> > > I'll see if that can be remedied.
> > 
> > Thanks. It's a real shame to have a "just use this option to avoid
> > instrumentation" if it doesn't actually catch everything. 
> 
> Agreed; it defeats the purpose of the exercise.
> 
> > We probably need to think about kprobes too, but not really sure what
> > you can do there on a per-file basis.
> 
> Ugh; that's a much more painful one, yes. :(
> 
> Does that rely on any compiler options at all? I thought was all a
> runtime thing.
> 
> Arguably it is somewhat separate for compiler instrumentation, and it
> might make sense for that to be a separate option.

Yes, I suppose the problem here is that opting out of dynamic tracing
requires function attributes such as notrace and __kprobes, rather than a
compiler flag.  If there's no way to say to the compiler "act as though
every function in this compilation unit is tagged with this attribute" then
we probably can't do anything to solve this easily.

We should probably add __kprobes to __LL_SC_INLINE though.

Will

Re: [PATCH 2/4] arm64: prevent instrumentation of LL/SC atomics

[Mark Rutland]

On Tue, Oct 17, 2017 at 12:38:14PM +0100, Will Deacon wrote:
> On Tue, Oct 17, 2017 at 12:10:33PM +0100, Mark Rutland wrote:
> > On Tue, Oct 17, 2017 at 11:58:58AM +0100, Will Deacon wrote:
> > > On Tue, Oct 17, 2017 at 11:54:54AM +0100, Mark Rutland wrote:
> > > > On Tue, Oct 17, 2017 at 11:03:15AM +0100, Will Deacon wrote:
> > > > > On Mon, Oct 16, 2017 at 02:24:38PM +0100, Mark Rutland wrote:
> > > > > > While we build the LL/SC atomics as a C object file, this does not
> > > > > > follow the AAPCS. This does not interoperate with other C code, and can
> > > > > > only be called from special wrapper assembly.
> > > > > > 
> > > > > > Bulding a kernel with CONFIG_KCOV and CONFIG_ARM64_LSE_ATOMICS results
> > > > > > in the cmopiler inserting calls to __sanitizer_cov_trace_pc within the
> > > > > > LL/SC atomics. As __sanitizer_cov_trace_pc is built per the AAPCS, these
> > > > > > calls corrupt register values, resulting in failures at boot time.
> > > > > > 
> > > > > > Avoid this (and other similar issues) by opting out of all compiler
> > > > > > instrumentation. We can opt-in to specific instrumentation in future if
> > > > > > we want to.
> > 
> > > > > > diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> > > > > > index a0abc142c92b..af77516f71b2 100644
> > > > > > --- a/arch/arm64/lib/Makefile
> > > > > > +++ b/arch/arm64/lib/Makefile
> > > > > > @@ -17,5 +17,6 @@ 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
> > > > > > +CC_INSTRUMENT_atomic_ll_sc.o := n
> > > > > 
> > > > > Does this mean we can lose the "notrace" definition of __LL_SC_INLINE
> > > > > when generating the out-of-line atomics?
> > > > 
> > > > Unfortunately not.
> > > > 
> > > > I'd missed -pg, since that isn't handled in scripts/Makefile.lib, and
> > > > doesn't seem to have a makefile-level disable.
> > > > 
> > > > I'll see if that can be remedied.
> > > 
> > > Thanks. It's a real shame to have a "just use this option to avoid
> > > instrumentation" if it doesn't actually catch everything. 
> > 
> > Agreed; it defeats the purpose of the exercise.
> > 
> > > We probably need to think about kprobes too, but not really sure what
> > > you can do there on a per-file basis.
> > 
> > Ugh; that's a much more painful one, yes. :(
> > 
> > Does that rely on any compiler options at all? I thought was all a
> > runtime thing.
> > 
> > Arguably it is somewhat separate for compiler instrumentation, and it
> > might make sense for that to be a separate option.
> 
> Yes, I suppose the problem here is that opting out of dynamic tracing
> requires function attributes such as notrace and __kprobes, rather than a
> compiler flag.  If there's no way to say to the compiler "act as though
> every function in this compilation unit is tagged with this attribute" then
> we probably can't do anything to solve this easily.

Unfortunately, I'm not aware of any way to do that short of using a
linker script to rewrite sections.

> We should probably add __kprobes to __LL_SC_INLINE though.

Agreed.

It's a different case, but kprobes can use atomics behind the scenes
(e.g. via aarch64_insn_patch_text_cb()), and so those need to be
blacklisted.

I'll add a patch to this series, unless you plan to put one together.

Thanks,
Mark.

Re: [PATCH 4/4] arm64: vdso: remove pointless gcov option

[Mark Rutland]

On Mon, Oct 16, 2017 at 02:24:40PM +0100, Mark Rutland wrote:
> The arm64 VDSO Makefile explicitly disables gcov; a practice we seem to
> have inherited from other VDSO Makefiles.
> 
> The arm64 VDSO is written entirely in assembly, so this is not
> necessary. Further, if the VDSO were written in C it would be necessary
> to disable all other instrumentation, and this alone would not be
> sufficient.

Evidently, I haven't looked hrough the history thoroughly enough. It
turns out that this was deliberately added in commit:

  543097843ca7c9ac ("arm64: build vdso without libgcov")

... this is necessary because we use the compiler to do the actual
linking. That causes us to have -lgcov in the compilr flags, and the
link can fail.

Please ignore this patch for now.

Thanks,
Mark.

> This patch remosves the redundant GCOV option for now.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Reported-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  arch/arm64/kernel/vdso/Makefile | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> index 62c84f7cb01b..350190b749dd 100644
> --- a/arch/arm64/kernel/vdso/Makefile
> +++ b/arch/arm64/kernel/vdso/Makefile
> @@ -15,9 +15,6 @@ ccflags-y := -shared -fno-common -fno-builtin
>  ccflags-y += -nostdlib -Wl,-soname=linux-vdso.so.1 \
>  		$(call cc-ldoption, -Wl$(comma)--hash-style=sysv)
>  
> -# Disable gcov profiling for VDSO code
> -GCOV_PROFILE := n
> -
>  # Workaround for bare-metal (ELF) toolchains that neglect to pass -shared
>  # down to collect2, resulting in silent corruption of the vDSO image.
>  ccflags-y += -Wl,-shared
> -- 
> 2.11.0
> 

Re: [PATCH 3/4] kvm/arm64: simplify CC instrumentation opt-out

[Christoffer Dall]

On Mon, Oct 16, 2017 at 02:24:39PM +0100, Mark Rutland wrote:
> Now that we can opt out of all instrumentation with a single option,
> make use of this in the KVM hyp code. Hopefully, this will also avoid
> fragility as new options are introduced.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Marc Zyngier <mark.zyngier@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

> ---
>  arch/arm64/kvm/hyp/Makefile | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
> index 14c4e3b14bcb..a8a212eb66f1 100644
> --- a/arch/arm64/kvm/hyp/Makefile
> +++ b/arch/arm64/kvm/hyp/Makefile
> @@ -22,7 +22,4 @@ obj-$(CONFIG_KVM_ARM_HOST) += s2-setup.o
>  # KVM code is run at a different exception code with a different map, so
>  # compiler instrumentation that inserts callbacks or checks into the code may
>  # cause crashes. Just disable it.
> -GCOV_PROFILE	:= n
> -KASAN_SANITIZE	:= n
> -UBSAN_SANITIZE	:= n
> -KCOV_INSTRUMENT	:= n
> +CC_INSTRUMENT	:= n
> -- 
> 2.11.0
> 

Re: [PATCH 2/4] arm64: prevent instrumentation of LL/SC atomics

[Will Deacon]

On Tue, Oct 17, 2017 at 01:55:16PM +0100, Mark Rutland wrote:
> On Tue, Oct 17, 2017 at 12:38:14PM +0100, Will Deacon wrote:
> > On Tue, Oct 17, 2017 at 12:10:33PM +0100, Mark Rutland wrote:
> > > On Tue, Oct 17, 2017 at 11:58:58AM +0100, Will Deacon wrote:
> > > > On Tue, Oct 17, 2017 at 11:54:54AM +0100, Mark Rutland wrote:
> > > > > On Tue, Oct 17, 2017 at 11:03:15AM +0100, Will Deacon wrote:
> > > > > > On Mon, Oct 16, 2017 at 02:24:38PM +0100, Mark Rutland wrote:
> > > > > > > While we build the LL/SC atomics as a C object file, this does not
> > > > > > > follow the AAPCS. This does not interoperate with other C code, and can
> > > > > > > only be called from special wrapper assembly.
> > > > > > > 
> > > > > > > Bulding a kernel with CONFIG_KCOV and CONFIG_ARM64_LSE_ATOMICS results
> > > > > > > in the cmopiler inserting calls to __sanitizer_cov_trace_pc within the
> > > > > > > LL/SC atomics. As __sanitizer_cov_trace_pc is built per the AAPCS, these
> > > > > > > calls corrupt register values, resulting in failures at boot time.
> > > > > > > 
> > > > > > > Avoid this (and other similar issues) by opting out of all compiler
> > > > > > > instrumentation. We can opt-in to specific instrumentation in future if
> > > > > > > we want to.
> > > 
> > > > > > > diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> > > > > > > index a0abc142c92b..af77516f71b2 100644
> > > > > > > --- a/arch/arm64/lib/Makefile
> > > > > > > +++ b/arch/arm64/lib/Makefile
> > > > > > > @@ -17,5 +17,6 @@ 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
> > > > > > > +CC_INSTRUMENT_atomic_ll_sc.o := n
> > > > > > 
> > > > > > Does this mean we can lose the "notrace" definition of __LL_SC_INLINE
> > > > > > when generating the out-of-line atomics?
> > > > > 
> > > > > Unfortunately not.
> > > > > 
> > > > > I'd missed -pg, since that isn't handled in scripts/Makefile.lib, and
> > > > > doesn't seem to have a makefile-level disable.
> > > > > 
> > > > > I'll see if that can be remedied.
> > > > 
> > > > Thanks. It's a real shame to have a "just use this option to avoid
> > > > instrumentation" if it doesn't actually catch everything. 
> > > 
> > > Agreed; it defeats the purpose of the exercise.
> > > 
> > > > We probably need to think about kprobes too, but not really sure what
> > > > you can do there on a per-file basis.
> > > 
> > > Ugh; that's a much more painful one, yes. :(
> > > 
> > > Does that rely on any compiler options at all? I thought was all a
> > > runtime thing.
> > > 
> > > Arguably it is somewhat separate for compiler instrumentation, and it
> > > might make sense for that to be a separate option.
> > 
> > Yes, I suppose the problem here is that opting out of dynamic tracing
> > requires function attributes such as notrace and __kprobes, rather than a
> > compiler flag.  If there's no way to say to the compiler "act as though
> > every function in this compilation unit is tagged with this attribute" then
> > we probably can't do anything to solve this easily.
> 
> Unfortunately, I'm not aware of any way to do that short of using a
> linker script to rewrite sections.
> 
> > We should probably add __kprobes to __LL_SC_INLINE though.
> 
> Agreed.
> 
> It's a different case, but kprobes can use atomics behind the scenes
> (e.g. via aarch64_insn_patch_text_cb()), and so those need to be
> blacklisted.
> 
> I'll add a patch to this series, unless you plan to put one together.

Don't mind either way. If you post the next version without, I can just
add it on top.

Will