* [PATCH] lkdtm: turn off kcov for lkdtm_rodata_do_nothing:
@ 2017-03-28 9:57 Arnd Bergmann
2017-03-28 10:00 ` Dmitry Vyukov
0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2017-03-28 9:57 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman; +Cc: Dmitry Vyukov, Kees Cook, linux-kernel
I ran into a link error on ARM64 for lkdtm_rodata_do_nothing:
drivers/misc/built-in.o: In function `lkdtm_rodata_do_nothing':
:(.rodata+0x68c8): relocation truncated to fit: R_AARCH64_CALL26 against symbol `__sanitizer_cov_trace_pc' defined in .text section in kernel/built-in.o
I did not analyze this further, but my theory is that we would need a trampoline
to call __sanitizer_cov_trace_pc(), but the linker (correctly) only adds trampolines
for callers in executable sections.
Disabling KCOV for this one file avoids the build failure with no
other practical downsides I can think of.
The problem can only happen on kernels that contain both kcov and
lkdtm, so if we want to backport this, it should be in the earliest
version that has both (v4.8).
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Kees Cook <keescook@chromium.org>
Fixes: 5c9a8750a640 ("kernel: add kcov code coverage")
Fixes: 9a49a528dcf3 ("lkdtm: add function for testing .rodata section")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/misc/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 4925ea8e1952..7a321047bfbe 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -63,6 +63,8 @@ lkdtm-$(CONFIG_LKDTM) += lkdtm_perms.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_rodata_objcopy.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_usercopy.o
+KCOV_INSTRUMENT_lkdtm_rodata.o := n
+
OBJCOPYFLAGS :=
OBJCOPYFLAGS_lkdtm_rodata_objcopy.o := \
--set-section-flags .text=alloc,readonly \
--
2.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] lkdtm: turn off kcov for lkdtm_rodata_do_nothing: 2017-03-28 9:57 [PATCH] lkdtm: turn off kcov for lkdtm_rodata_do_nothing: Arnd Bergmann @ 2017-03-28 10:00 ` Dmitry Vyukov 2017-03-28 10:22 ` Mark Rutland 2017-03-28 16:57 ` Kees Cook 0 siblings, 2 replies; 6+ messages in thread From: Dmitry Vyukov @ 2017-03-28 10:00 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Greg Kroah-Hartman, Kees Cook, LKML, Mark Rutland On Tue, Mar 28, 2017 at 11:57 AM, Arnd Bergmann <arnd@arndb.de> wrote: > I ran into a link error on ARM64 for lkdtm_rodata_do_nothing: > > drivers/misc/built-in.o: In function `lkdtm_rodata_do_nothing': > :(.rodata+0x68c8): relocation truncated to fit: R_AARCH64_CALL26 against symbol `__sanitizer_cov_trace_pc' defined in .text section in kernel/built-in.o > > I did not analyze this further, but my theory is that we would need a trampoline > to call __sanitizer_cov_trace_pc(), but the linker (correctly) only adds trampolines > for callers in executable sections. > > Disabling KCOV for this one file avoids the build failure with no > other practical downsides I can think of. > > The problem can only happen on kernels that contain both kcov and > lkdtm, so if we want to backport this, it should be in the earliest > version that has both (v4.8). > > Cc: Dmitry Vyukov <dvyukov@google.com> > Cc: Kees Cook <keescook@chromium.org> > Fixes: 5c9a8750a640 ("kernel: add kcov code coverage") > Fixes: 9a49a528dcf3 ("lkdtm: add function for testing .rodata section") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/misc/Makefile | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > index 4925ea8e1952..7a321047bfbe 100644 > --- a/drivers/misc/Makefile > +++ b/drivers/misc/Makefile > @@ -63,6 +63,8 @@ lkdtm-$(CONFIG_LKDTM) += lkdtm_perms.o > lkdtm-$(CONFIG_LKDTM) += lkdtm_rodata_objcopy.o > lkdtm-$(CONFIG_LKDTM) += lkdtm_usercopy.o > > +KCOV_INSTRUMENT_lkdtm_rodata.o := n > + > OBJCOPYFLAGS := > OBJCOPYFLAGS_lkdtm_rodata_objcopy.o := \ > --set-section-flags .text=alloc,readonly \ > -- > 2.9.0 Acked-by: Dmitry Vyukov <dvyukov@google.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] lkdtm: turn off kcov for lkdtm_rodata_do_nothing: 2017-03-28 10:00 ` Dmitry Vyukov @ 2017-03-28 10:22 ` Mark Rutland 2017-03-28 12:35 ` Arnd Bergmann 2017-03-28 16:57 ` Kees Cook 1 sibling, 1 reply; 6+ messages in thread From: Mark Rutland @ 2017-03-28 10:22 UTC (permalink / raw) To: Dmitry Vyukov, Arnd Bergmann; +Cc: Greg Kroah-Hartman, Kees Cook, LKML Hi, On Tue, Mar 28, 2017 at 12:00:15PM +0200, Dmitry Vyukov wrote: > On Tue, Mar 28, 2017 at 11:57 AM, Arnd Bergmann <arnd@arndb.de> wrote: > > I ran into a link error on ARM64 for lkdtm_rodata_do_nothing: > > > > drivers/misc/built-in.o: In function `lkdtm_rodata_do_nothing': > > :(.rodata+0x68c8): relocation truncated to fit: R_AARCH64_CALL26 against symbol `__sanitizer_cov_trace_pc' defined in .text section in kernel/built-in.o > > > > I did not analyze this further, but my theory is that we would need a trampoline > > to call __sanitizer_cov_trace_pc(), but the linker (correctly) only adds trampolines > > for callers in executable sections. For reference, is this a "large" config, e.g. allyesconfig? I'm aware that at least as recently as GCC 6 there were issues with veneer generation for calls across sections (which I personally saw with calls from .init.text to .text) when the kernel was sufficiently large. FWIW, I have no problem building a v4.11-rc3 kernel with both KCOV and LKDTM using the Linaro 15.08 aarch64-linux-gnu GCC 5 toolchain. Which toolchain are you using? No strong feelings on this patch, but it may be indicative of a larger problem, and we probably don't want to play whack-a-mole to fix relocation truncation more generally. Thanks, Mark. > > Disabling KCOV for this one file avoids the build failure with no > > other practical downsides I can think of. > > > > The problem can only happen on kernels that contain both kcov and > > lkdtm, so if we want to backport this, it should be in the earliest > > version that has both (v4.8). > > > > Cc: Dmitry Vyukov <dvyukov@google.com> > > Cc: Kees Cook <keescook@chromium.org> > > Fixes: 5c9a8750a640 ("kernel: add kcov code coverage") > > Fixes: 9a49a528dcf3 ("lkdtm: add function for testing .rodata section") > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > --- > > drivers/misc/Makefile | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > > index 4925ea8e1952..7a321047bfbe 100644 > > --- a/drivers/misc/Makefile > > +++ b/drivers/misc/Makefile > > @@ -63,6 +63,8 @@ lkdtm-$(CONFIG_LKDTM) += lkdtm_perms.o > > lkdtm-$(CONFIG_LKDTM) += lkdtm_rodata_objcopy.o > > lkdtm-$(CONFIG_LKDTM) += lkdtm_usercopy.o > > > > +KCOV_INSTRUMENT_lkdtm_rodata.o := n > > + > > OBJCOPYFLAGS := > > OBJCOPYFLAGS_lkdtm_rodata_objcopy.o := \ > > --set-section-flags .text=alloc,readonly \ > > -- > > 2.9.0 > > > Acked-by: Dmitry Vyukov <dvyukov@google.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] lkdtm: turn off kcov for lkdtm_rodata_do_nothing: 2017-03-28 10:22 ` Mark Rutland @ 2017-03-28 12:35 ` Arnd Bergmann 0 siblings, 0 replies; 6+ messages in thread From: Arnd Bergmann @ 2017-03-28 12:35 UTC (permalink / raw) To: Mark Rutland; +Cc: Dmitry Vyukov, Greg Kroah-Hartman, Kees Cook, LKML On Tue, Mar 28, 2017 at 12:22 PM, Mark Rutland <mark.rutland@arm.com> wrote: > Hi, > > On Tue, Mar 28, 2017 at 12:00:15PM +0200, Dmitry Vyukov wrote: >> On Tue, Mar 28, 2017 at 11:57 AM, Arnd Bergmann <arnd@arndb.de> wrote: >> > I ran into a link error on ARM64 for lkdtm_rodata_do_nothing: >> > >> > drivers/misc/built-in.o: In function `lkdtm_rodata_do_nothing': >> > :(.rodata+0x68c8): relocation truncated to fit: R_AARCH64_CALL26 against symbol `__sanitizer_cov_trace_pc' defined in .text section in kernel/built-in.o >> > >> > I did not analyze this further, but my theory is that we would need a trampoline >> > to call __sanitizer_cov_trace_pc(), but the linker (correctly) only adds trampolines >> > for callers in executable sections. > > For reference, is this a "large" config, e.g. allyesconfig? It's a randconfig build, and I'm guessing that it's large, but I have not checked the size of the sections. This was the first time I ran into it after months of building randconfig kernels without anything having triggered it recently. > I'm aware that at least as recently as GCC 6 there were issues with > veneer generation for calls across sections (which I personally saw with > calls from .init.text to .text) when the kernel was sufficiently large. We have had problems with veneers no arm32 in the past, and ld definitely doesn't create them for calls from inside of non-executable sections, so the typical fix was to put all code that needs veneers into executable sections. Clearly that cannot work for this particular case. > FWIW, I have no problem building a v4.11-rc3 kernel with both KCOV and > LKDTM using the Linaro 15.08 aarch64-linux-gnu GCC 5 toolchain. Which > toolchain are you using? This is a gcc-7.0.1 snapshot from last week, and a somewhat older binutils: GNU ld (GNU Binutils) 2.27.51.20161215 > No strong feelings on this patch, but it may be indicative of a larger > problem, and we probably don't want to play whack-a-mole to fix > relocation truncation more generally. I have not seen this problem in allyesconfig or any other randconfig build, so it's certainly likely that some other setting must be there to trigger this. I've recreated the problem using an allyesconfig build with this change to turn off -mcmodel=large: @@ -283,7 +283,6 @@ CONFIG_HAVE_IRQ_TIME_ACCOUNTING=y CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE=y CONFIG_HAVE_ARCH_HUGE_VMAP=y -CONFIG_HAVE_MOD_ARCH_SPECIFIC=y CONFIG_MODULES_USE_ELF_RELA=y CONFIG_ARCH_HAS_ELF_RANDOMIZE=y CONFIG_HAVE_ARCH_MMAP_RND_BITS=y @@ -529,7 +528,7 @@ CONFIG_ARM64_ERRATUM_832075=y CONFIG_ARM64_ERRATUM_834220=y CONFIG_ARM64_ERRATUM_845719=y -CONFIG_ARM64_ERRATUM_843419=y +# CONFIG_ARM64_ERRATUM_843419 is not set CONFIG_CAVIUM_ERRATUM_22375=y CONFIG_CAVIUM_ERRATUM_23144=y CONFIG_CAVIUM_ERRATUM_23154=y @@ -643,10 +642,7 @@ # ARMv8.2 architectural features # CONFIG_ARM64_UAO=y -CONFIG_ARM64_MODULE_CMODEL_LARGE=y -CONFIG_ARM64_MODULE_PLTS=y -CONFIG_RELOCATABLE=y -CONFIG_RANDOMIZE_BASE=y +# CONFIG_RANDOMIZE_BASE is not set # # Boot options Arnd ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] lkdtm: turn off kcov for lkdtm_rodata_do_nothing: 2017-03-28 10:00 ` Dmitry Vyukov 2017-03-28 10:22 ` Mark Rutland @ 2017-03-28 16:57 ` Kees Cook 2017-03-28 17:12 ` Greg Kroah-Hartman 1 sibling, 1 reply; 6+ messages in thread From: Kees Cook @ 2017-03-28 16:57 UTC (permalink / raw) To: Arnd Bergmann, Greg Kroah-Hartman; +Cc: Dmitry Vyukov, LKML, Mark Rutland On Tue, Mar 28, 2017 at 3:00 AM, Dmitry Vyukov <dvyukov@google.com> wrote: > On Tue, Mar 28, 2017 at 11:57 AM, Arnd Bergmann <arnd@arndb.de> wrote: >> I ran into a link error on ARM64 for lkdtm_rodata_do_nothing: >> >> drivers/misc/built-in.o: In function `lkdtm_rodata_do_nothing': >> :(.rodata+0x68c8): relocation truncated to fit: R_AARCH64_CALL26 against symbol `__sanitizer_cov_trace_pc' defined in .text section in kernel/built-in.o >> >> I did not analyze this further, but my theory is that we would need a trampoline >> to call __sanitizer_cov_trace_pc(), but the linker (correctly) only adds trampolines >> for callers in executable sections. Yup, this function should be inherently uncallable (which is what lkdtm is testing), so it makes sense to turn off kcov for it. >> >> Disabling KCOV for this one file avoids the build failure with no >> other practical downsides I can think of. >> >> The problem can only happen on kernels that contain both kcov and >> lkdtm, so if we want to backport this, it should be in the earliest >> version that has both (v4.8). >> >> Cc: Dmitry Vyukov <dvyukov@google.com> >> Cc: Kees Cook <keescook@chromium.org> >> Fixes: 5c9a8750a640 ("kernel: add kcov code coverage") >> Fixes: 9a49a528dcf3 ("lkdtm: add function for testing .rodata section") >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> --- >> drivers/misc/Makefile | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile >> index 4925ea8e1952..7a321047bfbe 100644 >> --- a/drivers/misc/Makefile >> +++ b/drivers/misc/Makefile >> @@ -63,6 +63,8 @@ lkdtm-$(CONFIG_LKDTM) += lkdtm_perms.o >> lkdtm-$(CONFIG_LKDTM) += lkdtm_rodata_objcopy.o >> lkdtm-$(CONFIG_LKDTM) += lkdtm_usercopy.o >> >> +KCOV_INSTRUMENT_lkdtm_rodata.o := n >> + >> OBJCOPYFLAGS := >> OBJCOPYFLAGS_lkdtm_rodata_objcopy.o := \ >> --set-section-flags .text=alloc,readonly \ >> -- >> 2.9.0 > > > Acked-by: Dmitry Vyukov <dvyukov@google.com> Acked-by: Kees Cook <keescook@chromium.org> Greg, can you please add this to the drivers/misc tree when you get a chance? Thanks! -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] lkdtm: turn off kcov for lkdtm_rodata_do_nothing: 2017-03-28 16:57 ` Kees Cook @ 2017-03-28 17:12 ` Greg Kroah-Hartman 0 siblings, 0 replies; 6+ messages in thread From: Greg Kroah-Hartman @ 2017-03-28 17:12 UTC (permalink / raw) To: Kees Cook; +Cc: Arnd Bergmann, Dmitry Vyukov, LKML, Mark Rutland On Tue, Mar 28, 2017 at 09:57:34AM -0700, Kees Cook wrote: > On Tue, Mar 28, 2017 at 3:00 AM, Dmitry Vyukov <dvyukov@google.com> wrote: > > On Tue, Mar 28, 2017 at 11:57 AM, Arnd Bergmann <arnd@arndb.de> wrote: > >> I ran into a link error on ARM64 for lkdtm_rodata_do_nothing: > >> > >> drivers/misc/built-in.o: In function `lkdtm_rodata_do_nothing': > >> :(.rodata+0x68c8): relocation truncated to fit: R_AARCH64_CALL26 against symbol `__sanitizer_cov_trace_pc' defined in .text section in kernel/built-in.o > >> > >> I did not analyze this further, but my theory is that we would need a trampoline > >> to call __sanitizer_cov_trace_pc(), but the linker (correctly) only adds trampolines > >> for callers in executable sections. > > Yup, this function should be inherently uncallable (which is what > lkdtm is testing), so it makes sense to turn off kcov for it. > > >> > >> Disabling KCOV for this one file avoids the build failure with no > >> other practical downsides I can think of. > >> > >> The problem can only happen on kernels that contain both kcov and > >> lkdtm, so if we want to backport this, it should be in the earliest > >> version that has both (v4.8). > >> > >> Cc: Dmitry Vyukov <dvyukov@google.com> > >> Cc: Kees Cook <keescook@chromium.org> > >> Fixes: 5c9a8750a640 ("kernel: add kcov code coverage") > >> Fixes: 9a49a528dcf3 ("lkdtm: add function for testing .rodata section") > >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > >> --- > >> drivers/misc/Makefile | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > >> index 4925ea8e1952..7a321047bfbe 100644 > >> --- a/drivers/misc/Makefile > >> +++ b/drivers/misc/Makefile > >> @@ -63,6 +63,8 @@ lkdtm-$(CONFIG_LKDTM) += lkdtm_perms.o > >> lkdtm-$(CONFIG_LKDTM) += lkdtm_rodata_objcopy.o > >> lkdtm-$(CONFIG_LKDTM) += lkdtm_usercopy.o > >> > >> +KCOV_INSTRUMENT_lkdtm_rodata.o := n > >> + > >> OBJCOPYFLAGS := > >> OBJCOPYFLAGS_lkdtm_rodata_objcopy.o := \ > >> --set-section-flags .text=alloc,readonly \ > >> -- > >> 2.9.0 > > > > > > Acked-by: Dmitry Vyukov <dvyukov@google.com> > > Acked-by: Kees Cook <keescook@chromium.org> > > Greg, can you please add this to the drivers/misc tree when you get a chance? Will do. thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-28 17:13 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-03-28 9:57 [PATCH] lkdtm: turn off kcov for lkdtm_rodata_do_nothing: Arnd Bergmann 2017-03-28 10:00 ` Dmitry Vyukov 2017-03-28 10:22 ` Mark Rutland 2017-03-28 12:35 ` Arnd Bergmann 2017-03-28 16:57 ` Kees Cook 2017-03-28 17:12 ` Greg Kroah-Hartman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.