* [for-stable-4.19 PATCH 0/2] Backport patches to fix KASAN+LKDTM with recent clang on ARM64 @ 2021-03-18 23:54 Nicolas Boichat 2021-03-18 23:54 ` [for-stable-4.19 PATCH 1/2] vmlinux.lds.h: Create section for protection against instrumentation Nicolas Boichat ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Nicolas Boichat @ 2021-03-18 23:54 UTC (permalink / raw) To: stable Cc: Nicolas Boichat, Alexandre Chartre, Arnd Bergmann, Benjamin Herrenschmidt, Christopher Li, Daniel Axtens, Greg Kroah-Hartman, Kees Cook, Masahiro Yamada, Michael Ellerman, Michal Marek, Naveen N. Rao, Nicholas Piggin, Paul Mackerras, Peter Zijlstra, Sasha Levin, Thomas Gleixner, clang-built-linux, linux-arch, linux-kbuild, linux-kernel, linux-sparse, linuxppc-dev Backport 2 patches that are required to make KASAN+LKDTM work with recent clang (patch 2/2 has a complete description). Tested on our chromeos-4.19 branch. Patch 1/2 is context conflict only, and 2/2 is a clean backport. These patches have been merged to 5.4 stable already. We might need to backport to older stable branches, but this is what I could test for now. Mark Rutland (1): lkdtm: don't move ctors to .rodata Thomas Gleixner (1): vmlinux.lds.h: Create section for protection against instrumentation arch/powerpc/kernel/vmlinux.lds.S | 1 + drivers/misc/lkdtm/Makefile | 2 +- drivers/misc/lkdtm/rodata.c | 2 +- include/asm-generic/sections.h | 3 ++ include/asm-generic/vmlinux.lds.h | 10 ++++++ include/linux/compiler.h | 54 +++++++++++++++++++++++++++++++ include/linux/compiler_types.h | 4 +++ scripts/mod/modpost.c | 2 +- 8 files changed, 75 insertions(+), 3 deletions(-) -- 2.31.0.rc2.261.g7f71774620-goog ^ permalink raw reply [flat|nested] 8+ messages in thread
* [for-stable-4.19 PATCH 1/2] vmlinux.lds.h: Create section for protection against instrumentation 2021-03-18 23:54 [for-stable-4.19 PATCH 0/2] Backport patches to fix KASAN+LKDTM with recent clang on ARM64 Nicolas Boichat @ 2021-03-18 23:54 ` Nicolas Boichat 2021-03-19 10:39 ` Greg Kroah-Hartman 2021-03-18 23:54 ` [for-stable-4.19 PATCH 2/2] lkdtm: don't move ctors to .rodata Nicolas Boichat 2021-03-19 10:30 ` [for-stable-4.19 PATCH 0/2] Backport patches to fix KASAN+LKDTM with recent clang on ARM64 Greg Kroah-Hartman 2 siblings, 1 reply; 8+ messages in thread From: Nicolas Boichat @ 2021-03-18 23:54 UTC (permalink / raw) To: stable Cc: Thomas Gleixner, Alexandre Chartre, Peter Zijlstra, Nicolas Boichat, Arnd Bergmann, Benjamin Herrenschmidt, Christopher Li, Daniel Axtens, Greg Kroah-Hartman, Masahiro Yamada, Michael Ellerman, Michal Marek, Naveen N. Rao, Nicholas Piggin, Paul Mackerras, Sasha Levin, linux-arch, linux-kbuild, linux-kernel, linux-sparse, linuxppc-dev From: Thomas Gleixner <tglx@linutronix.de> commit 6553896666433e7efec589838b400a2a652b3ffa upstream. Some code pathes, especially the low level entry code, must be protected against instrumentation for various reasons: - Low level entry code can be a fragile beast, especially on x86. - With NO_HZ_FULL RCU state needs to be established before using it. Having a dedicated section for such code allows to validate with tooling that no unsafe functions are invoked. Add the .noinstr.text section and the noinstr attribute to mark functions. noinstr implies notrace. Kprobes will gain a section check later. Provide also a set of markers: instrumentation_begin()/end() These are used to mark code inside a noinstr function which calls into regular instrumentable text section as safe. The instrumentation markers are only active when CONFIG_DEBUG_ENTRY is enabled as the end marker emits a NOP to prevent the compiler from merging the annotation points. This means the objtool verification requires a kernel compiled with this option. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com> Acked-by: Peter Zijlstra <peterz@infradead.org> Link: https://lkml.kernel.org/r/20200505134100.075416272@linutronix.de [Nicolas: context conflicts in: arch/powerpc/kernel/vmlinux.lds.S include/asm-generic/vmlinux.lds.h include/linux/compiler.h include/linux/compiler_types.h] Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> --- arch/powerpc/kernel/vmlinux.lds.S | 1 + include/asm-generic/sections.h | 3 ++ include/asm-generic/vmlinux.lds.h | 10 ++++++ include/linux/compiler.h | 54 +++++++++++++++++++++++++++++++ include/linux/compiler_types.h | 4 +++ scripts/mod/modpost.c | 2 +- 6 files changed, 73 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S index 695432965f20..9b346f3d2814 100644 --- a/arch/powerpc/kernel/vmlinux.lds.S +++ b/arch/powerpc/kernel/vmlinux.lds.S @@ -99,6 +99,7 @@ SECTIONS #endif /* careful! __ftr_alt_* sections need to be close to .text */ *(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup __ftr_alt_* .ref.text); + NOINSTR_TEXT SCHED_TEXT CPUIDLE_TEXT LOCK_TEXT diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h index 849cd8eb5ca0..ea5987bb0b84 100644 --- a/include/asm-generic/sections.h +++ b/include/asm-generic/sections.h @@ -53,6 +53,9 @@ extern char __ctors_start[], __ctors_end[]; /* Start and end of .opd section - used for function descriptors. */ extern char __start_opd[], __end_opd[]; +/* Start and end of instrumentation protected text section */ +extern char __noinstr_text_start[], __noinstr_text_end[]; + extern __visible const void __nosave_begin, __nosave_end; /* Function descriptor handling (if any). Override in asm/sections.h */ diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index 2d632a74cc5e..88484ee023ca 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -482,6 +482,15 @@ __security_initcall_end = .; \ } +/* + * Non-instrumentable text section + */ +#define NOINSTR_TEXT \ + ALIGN_FUNCTION(); \ + __noinstr_text_start = .; \ + *(.noinstr.text) \ + __noinstr_text_end = .; + /* * .text section. Map to function alignment to avoid address changes * during second ld run in second ld pass when generating System.map @@ -496,6 +505,7 @@ *(TEXT_MAIN .text.fixup) \ *(.text.unlikely .text.unlikely.*) \ *(.text.unknown .text.unknown.*) \ + NOINSTR_TEXT \ *(.text..refcount) \ *(.ref.text) \ MEM_KEEP(init.text*) \ diff --git a/include/linux/compiler.h b/include/linux/compiler.h index 6b6505e3b2c7..6a53300cbd1e 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -129,11 +129,65 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val, ".pushsection .discard.unreachable\n\t" \ ".long 999b - .\n\t" \ ".popsection\n\t" + +#ifdef CONFIG_DEBUG_ENTRY +/* Begin/end of an instrumentation safe region */ +#define instrumentation_begin() ({ \ + asm volatile("%c0:\n\t" \ + ".pushsection .discard.instr_begin\n\t" \ + ".long %c0b - .\n\t" \ + ".popsection\n\t" : : "i" (__COUNTER__)); \ +}) + +/* + * Because instrumentation_{begin,end}() can nest, objtool validation considers + * _begin() a +1 and _end() a -1 and computes a sum over the instructions. + * When the value is greater than 0, we consider instrumentation allowed. + * + * There is a problem with code like: + * + * noinstr void foo() + * { + * instrumentation_begin(); + * ... + * if (cond) { + * instrumentation_begin(); + * ... + * instrumentation_end(); + * } + * bar(); + * instrumentation_end(); + * } + * + * If instrumentation_end() would be an empty label, like all the other + * annotations, the inner _end(), which is at the end of a conditional block, + * would land on the instruction after the block. + * + * If we then consider the sum of the !cond path, we'll see that the call to + * bar() is with a 0-value, even though, we meant it to happen with a positive + * value. + * + * To avoid this, have _end() be a NOP instruction, this ensures it will be + * part of the condition block and does not escape. + */ +#define instrumentation_end() ({ \ + asm volatile("%c0: nop\n\t" \ + ".pushsection .discard.instr_end\n\t" \ + ".long %c0b - .\n\t" \ + ".popsection\n\t" : : "i" (__COUNTER__)); \ +}) +#endif /* CONFIG_DEBUG_ENTRY */ + #else #define annotate_reachable() #define annotate_unreachable() #endif +#ifndef instrumentation_begin +#define instrumentation_begin() do { } while(0) +#define instrumentation_end() do { } while(0) +#endif + #ifndef ASM_UNREACHABLE # define ASM_UNREACHABLE #endif diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index 2b8ed70c4c77..a9b0495051a3 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -234,6 +234,10 @@ struct ftrace_likely_data { #define notrace __attribute__((no_instrument_function)) #endif +/* Section for code which can't be instrumented at all */ +#define noinstr \ + noinline notrace __attribute((__section__(".noinstr.text"))) + /* * it doesn't make sense on ARM (currently the only user of __naked) * to trace naked functions because then mcount is called without diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 91a80036c05d..7c693bd775c1 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -895,7 +895,7 @@ static void check_section(const char *modname, struct elf_info *elf, #define DATA_SECTIONS ".data", ".data.rel" #define TEXT_SECTIONS ".text", ".text.unlikely", ".sched.text", \ - ".kprobes.text", ".cpuidle.text" + ".kprobes.text", ".cpuidle.text", ".noinstr.text" #define OTHER_TEXT_SECTIONS ".ref.text", ".head.text", ".spinlock.text", \ ".fixup", ".entry.text", ".exception.text", ".text.*", \ ".coldtext" -- 2.31.0.rc2.261.g7f71774620-goog ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [for-stable-4.19 PATCH 1/2] vmlinux.lds.h: Create section for protection against instrumentation 2021-03-18 23:54 ` [for-stable-4.19 PATCH 1/2] vmlinux.lds.h: Create section for protection against instrumentation Nicolas Boichat @ 2021-03-19 10:39 ` Greg Kroah-Hartman 2021-03-19 11:20 ` Alexandre Chartre 0 siblings, 1 reply; 8+ messages in thread From: Greg Kroah-Hartman @ 2021-03-19 10:39 UTC (permalink / raw) To: Nicolas Boichat Cc: stable, Thomas Gleixner, Alexandre Chartre, Peter Zijlstra, Arnd Bergmann, Benjamin Herrenschmidt, Christopher Li, Daniel Axtens, Masahiro Yamada, Michael Ellerman, Michal Marek, Naveen N. Rao, Nicholas Piggin, Paul Mackerras, Sasha Levin, linux-arch, linux-kbuild, linux-kernel, linux-sparse, linuxppc-dev On Fri, Mar 19, 2021 at 07:54:15AM +0800, Nicolas Boichat wrote: > From: Thomas Gleixner <tglx@linutronix.de> > > commit 6553896666433e7efec589838b400a2a652b3ffa upstream. > > Some code pathes, especially the low level entry code, must be protected > against instrumentation for various reasons: > > - Low level entry code can be a fragile beast, especially on x86. > > - With NO_HZ_FULL RCU state needs to be established before using it. > > Having a dedicated section for such code allows to validate with tooling > that no unsafe functions are invoked. > > Add the .noinstr.text section and the noinstr attribute to mark > functions. noinstr implies notrace. Kprobes will gain a section check > later. > > Provide also a set of markers: instrumentation_begin()/end() > > These are used to mark code inside a noinstr function which calls > into regular instrumentable text section as safe. > > The instrumentation markers are only active when CONFIG_DEBUG_ENTRY is > enabled as the end marker emits a NOP to prevent the compiler from merging > the annotation points. This means the objtool verification requires a > kernel compiled with this option. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com> > Acked-by: Peter Zijlstra <peterz@infradead.org> > Link: https://lkml.kernel.org/r/20200505134100.075416272@linutronix.de > > [Nicolas: context conflicts in: > arch/powerpc/kernel/vmlinux.lds.S > include/asm-generic/vmlinux.lds.h > include/linux/compiler.h > include/linux/compiler_types.h] > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> Did you build this on x86? I get the following build error: ld:./arch/x86/kernel/vmlinux.lds:20: syntax error And that line looks like: . = ALIGN(8); *(.text.hot .text.hot.*) *(.text .text.fixup) *(.text.unlikely .text.unlikely.*) *(.text.unknown .text.unknown.*) . = ALIGN(8); __noinstr_text_start = .; *(.__attribute__((noinline)) __attribute__((no_instrument_function)) __attribute((__section__(".noinstr.text"))).text) __noinstr_text_end = .; *(.text..refcount) *(.ref.text) *(.meminit.text*) *(.memexit.text*) So I'm going to drop both of these patches from the queue. thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [for-stable-4.19 PATCH 1/2] vmlinux.lds.h: Create section for protection against instrumentation 2021-03-19 10:39 ` Greg Kroah-Hartman @ 2021-03-19 11:20 ` Alexandre Chartre 2021-03-19 11:55 ` Greg Kroah-Hartman 0 siblings, 1 reply; 8+ messages in thread From: Alexandre Chartre @ 2021-03-19 11:20 UTC (permalink / raw) To: Greg Kroah-Hartman, Nicolas Boichat Cc: stable, Thomas Gleixner, Peter Zijlstra, Arnd Bergmann, Benjamin Herrenschmidt, Christopher Li, Daniel Axtens, Masahiro Yamada, Michael Ellerman, Michal Marek, Naveen N. Rao, Nicholas Piggin, Paul Mackerras, Sasha Levin, linux-arch, linux-kbuild, linux-kernel, linux-sparse, linuxppc-dev On 3/19/21 11:39 AM, Greg Kroah-Hartman wrote: > On Fri, Mar 19, 2021 at 07:54:15AM +0800, Nicolas Boichat wrote: >> From: Thomas Gleixner <tglx@linutronix.de> >> >> commit 6553896666433e7efec589838b400a2a652b3ffa upstream. >> >> Some code pathes, especially the low level entry code, must be protected >> against instrumentation for various reasons: >> >> - Low level entry code can be a fragile beast, especially on x86. >> >> - With NO_HZ_FULL RCU state needs to be established before using it. >> >> Having a dedicated section for such code allows to validate with tooling >> that no unsafe functions are invoked. >> >> Add the .noinstr.text section and the noinstr attribute to mark >> functions. noinstr implies notrace. Kprobes will gain a section check >> later. >> >> Provide also a set of markers: instrumentation_begin()/end() >> >> These are used to mark code inside a noinstr function which calls >> into regular instrumentable text section as safe. >> >> The instrumentation markers are only active when CONFIG_DEBUG_ENTRY is >> enabled as the end marker emits a NOP to prevent the compiler from merging >> the annotation points. This means the objtool verification requires a >> kernel compiled with this option. >> >> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> >> Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com> >> Acked-by: Peter Zijlstra <peterz@infradead.org> >> Link: https://lkml.kernel.org/r/20200505134100.075416272@linutronix.de >> >> [Nicolas: context conflicts in: >> arch/powerpc/kernel/vmlinux.lds.S >> include/asm-generic/vmlinux.lds.h >> include/linux/compiler.h >> include/linux/compiler_types.h] >> Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > > Did you build this on x86? > > I get the following build error: > > ld:./arch/x86/kernel/vmlinux.lds:20: syntax error > > And that line looks like: > > . = ALIGN(8); *(.text.hot .text.hot.*) *(.text .text.fixup) *(.text.unlikely .text.unlikely.*) *(.text.unknown .text.unknown.*) . = ALIGN(8); __noinstr_text_start = .; *(.__attribute__((noinline)) __attribute__((no_instrument_function)) __attribute((__section__(".noinstr.text"))).text) __noinstr_text_end = .; *(.text..refcount) *(.ref.text) *(.meminit.text*) *(.memexit.text*) > In the NOINSTR_TEXT macro, noinstr is expanded with the value of the noinstr macro from linux/compiler_types.h while it shouldn't. The problem is possibly that the noinstr macro is defined for assembly. Make sure that the macro is not defined for assembly e.g.: #ifndef __ASSEMBLY__ /* Section for code which can't be instrumented at all */ #define noinstr \ noinline notrace __attribute((__section__(".noinstr.text"))) #endif alex. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [for-stable-4.19 PATCH 1/2] vmlinux.lds.h: Create section for protection against instrumentation 2021-03-19 11:20 ` Alexandre Chartre @ 2021-03-19 11:55 ` Greg Kroah-Hartman 2021-03-19 22:48 ` Nicolas Boichat 0 siblings, 1 reply; 8+ messages in thread From: Greg Kroah-Hartman @ 2021-03-19 11:55 UTC (permalink / raw) To: Alexandre Chartre Cc: Nicolas Boichat, stable, Thomas Gleixner, Peter Zijlstra, Arnd Bergmann, Benjamin Herrenschmidt, Christopher Li, Daniel Axtens, Masahiro Yamada, Michael Ellerman, Michal Marek, Naveen N. Rao, Nicholas Piggin, Paul Mackerras, Sasha Levin, linux-arch, linux-kbuild, linux-kernel, linux-sparse, linuxppc-dev On Fri, Mar 19, 2021 at 12:20:22PM +0100, Alexandre Chartre wrote: > > On 3/19/21 11:39 AM, Greg Kroah-Hartman wrote: > > On Fri, Mar 19, 2021 at 07:54:15AM +0800, Nicolas Boichat wrote: > > > From: Thomas Gleixner <tglx@linutronix.de> > > > > > > commit 6553896666433e7efec589838b400a2a652b3ffa upstream. > > > > > > Some code pathes, especially the low level entry code, must be protected > > > against instrumentation for various reasons: > > > > > > - Low level entry code can be a fragile beast, especially on x86. > > > > > > - With NO_HZ_FULL RCU state needs to be established before using it. > > > > > > Having a dedicated section for such code allows to validate with tooling > > > that no unsafe functions are invoked. > > > > > > Add the .noinstr.text section and the noinstr attribute to mark > > > functions. noinstr implies notrace. Kprobes will gain a section check > > > later. > > > > > > Provide also a set of markers: instrumentation_begin()/end() > > > > > > These are used to mark code inside a noinstr function which calls > > > into regular instrumentable text section as safe. > > > > > > The instrumentation markers are only active when CONFIG_DEBUG_ENTRY is > > > enabled as the end marker emits a NOP to prevent the compiler from merging > > > the annotation points. This means the objtool verification requires a > > > kernel compiled with this option. > > > > > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > > > Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com> > > > Acked-by: Peter Zijlstra <peterz@infradead.org> > > > Link: https://lkml.kernel.org/r/20200505134100.075416272@linutronix.de > > > > > > [Nicolas: context conflicts in: > > > arch/powerpc/kernel/vmlinux.lds.S > > > include/asm-generic/vmlinux.lds.h > > > include/linux/compiler.h > > > include/linux/compiler_types.h] > > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > > > > Did you build this on x86? > > > > I get the following build error: > > > > ld:./arch/x86/kernel/vmlinux.lds:20: syntax error > > > > And that line looks like: > > > > . = ALIGN(8); *(.text.hot .text.hot.*) *(.text .text.fixup) *(.text.unlikely .text.unlikely.*) *(.text.unknown .text.unknown.*) . = ALIGN(8); __noinstr_text_start = .; *(.__attribute__((noinline)) __attribute__((no_instrument_function)) __attribute((__section__(".noinstr.text"))).text) __noinstr_text_end = .; *(.text..refcount) *(.ref.text) *(.meminit.text*) *(.memexit.text*) > > > > In the NOINSTR_TEXT macro, noinstr is expanded with the value of the noinstr > macro from linux/compiler_types.h while it shouldn't. > > The problem is possibly that the noinstr macro is defined for assembly. Make > sure that the macro is not defined for assembly e.g.: > > #ifndef __ASSEMBLY__ > > /* Section for code which can't be instrumented at all */ > #define noinstr \ > noinline notrace __attribute((__section__(".noinstr.text"))) > > #endif This implies that the backport is incorrect, so I'll wait for an updated version... thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [for-stable-4.19 PATCH 1/2] vmlinux.lds.h: Create section for protection against instrumentation 2021-03-19 11:55 ` Greg Kroah-Hartman @ 2021-03-19 22:48 ` Nicolas Boichat 0 siblings, 0 replies; 8+ messages in thread From: Nicolas Boichat @ 2021-03-19 22:48 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Alexandre Chartre, stable, Thomas Gleixner, Peter Zijlstra, Arnd Bergmann, Benjamin Herrenschmidt, Christopher Li, Daniel Axtens, Masahiro Yamada, Michael Ellerman, Michal Marek, Naveen N. Rao, Nicholas Piggin, Paul Mackerras, Sasha Levin, linux-arch, linux-kbuild, lkml, linux-sparse, linuxppc-dev, Guenter Roeck On Fri, Mar 19, 2021 at 7:55 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Fri, Mar 19, 2021 at 12:20:22PM +0100, Alexandre Chartre wrote: > > > > On 3/19/21 11:39 AM, Greg Kroah-Hartman wrote: > > > On Fri, Mar 19, 2021 at 07:54:15AM +0800, Nicolas Boichat wrote: > > > > From: Thomas Gleixner <tglx@linutronix.de> > > > > > > > > commit 6553896666433e7efec589838b400a2a652b3ffa upstream. > > > > > > > > Some code pathes, especially the low level entry code, must be protected > > > > against instrumentation for various reasons: > > > > > > > > - Low level entry code can be a fragile beast, especially on x86. > > > > > > > > - With NO_HZ_FULL RCU state needs to be established before using it. > > > > > > > > Having a dedicated section for such code allows to validate with tooling > > > > that no unsafe functions are invoked. > > > > > > > > Add the .noinstr.text section and the noinstr attribute to mark > > > > functions. noinstr implies notrace. Kprobes will gain a section check > > > > later. > > > > > > > > Provide also a set of markers: instrumentation_begin()/end() > > > > > > > > These are used to mark code inside a noinstr function which calls > > > > into regular instrumentable text section as safe. > > > > > > > > The instrumentation markers are only active when CONFIG_DEBUG_ENTRY is > > > > enabled as the end marker emits a NOP to prevent the compiler from merging > > > > the annotation points. This means the objtool verification requires a > > > > kernel compiled with this option. > > > > > > > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > > > > Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com> > > > > Acked-by: Peter Zijlstra <peterz@infradead.org> > > > > Link: https://lkml.kernel.org/r/20200505134100.075416272@linutronix.de > > > > > > > > [Nicolas: context conflicts in: > > > > arch/powerpc/kernel/vmlinux.lds.S > > > > include/asm-generic/vmlinux.lds.h > > > > include/linux/compiler.h > > > > include/linux/compiler_types.h] > > > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > > > > > > Did you build this on x86? > > > > > > I get the following build error: > > > > > > ld:./arch/x86/kernel/vmlinux.lds:20: syntax error > > > > > > And that line looks like: > > > > > > . = ALIGN(8); *(.text.hot .text.hot.*) *(.text .text.fixup) *(.text.unlikely .text.unlikely.*) *(.text.unknown .text.unknown.*) . = ALIGN(8); __noinstr_text_start = .; *(.__attribute__((noinline)) __attribute__((no_instrument_function)) __attribute((__section__(".noinstr.text"))).text) __noinstr_text_end = .; *(.text..refcount) *(.ref.text) *(.meminit.text*) *(.memexit.text*) > > > > > > > In the NOINSTR_TEXT macro, noinstr is expanded with the value of the noinstr > > macro from linux/compiler_types.h while it shouldn't. > > > > The problem is possibly that the noinstr macro is defined for assembly. Make > > sure that the macro is not defined for assembly e.g.: > > > > #ifndef __ASSEMBLY__ > > > > /* Section for code which can't be instrumented at all */ > > #define noinstr \ > > noinline notrace __attribute((__section__(".noinstr.text"))) > > > > #endif > > This implies that the backport is incorrect, so I'll wait for an updated > version... Yep, sorry about that. I did test on ARM64 only and these patches happily went through our Chrome OS CQ (we don't have gcc coverage though). Guenter has a fixup here with explanation: https://crrev.com/c/2776332, I'll look carefully and resubmit. Thanks, > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* [for-stable-4.19 PATCH 2/2] lkdtm: don't move ctors to .rodata 2021-03-18 23:54 [for-stable-4.19 PATCH 0/2] Backport patches to fix KASAN+LKDTM with recent clang on ARM64 Nicolas Boichat 2021-03-18 23:54 ` [for-stable-4.19 PATCH 1/2] vmlinux.lds.h: Create section for protection against instrumentation Nicolas Boichat @ 2021-03-18 23:54 ` Nicolas Boichat 2021-03-19 10:30 ` [for-stable-4.19 PATCH 0/2] Backport patches to fix KASAN+LKDTM with recent clang on ARM64 Greg Kroah-Hartman 2 siblings, 0 replies; 8+ messages in thread From: Nicolas Boichat @ 2021-03-18 23:54 UTC (permalink / raw) To: stable Cc: Mark Rutland, Arnd Bergmann, Greg Kroah-Hartman, Kees Cook, Nicolas Boichat, clang-built-linux, linux-kernel From: Mark Rutland <mark.rutland@arm.com> commit 3f618ab3323407ee4c6a6734a37eb6e9663ebfb9 upstream. When building with KASAN and LKDTM, clang may implictly generate an asan.module_ctor function in the LKDTM rodata object. The Makefile moves the lkdtm_rodata_do_nothing() function into .rodata by renaming the file's .text section to .rodata, and consequently also moves the ctor function into .rodata, leading to a boot time crash (splat below) when the ctor is invoked by do_ctors(). Let's prevent this by marking the function as noinstr rather than notrace, and renaming the file's .noinstr.text to .rodata. Marking the function as noinstr will prevent tracing and kprobes, and will inhibit any undesireable compiler instrumentation. The ctor function (if any) will be placed in .text and will work correctly. Example splat before this patch is applied: [ 0.916359] Unable to handle kernel execute from non-executable memory at virtual address ffffa0006b60f5ac [ 0.922088] Mem abort info: [ 0.922828] ESR = 0x8600000e [ 0.923635] EC = 0x21: IABT (current EL), IL = 32 bits [ 0.925036] SET = 0, FnV = 0 [ 0.925838] EA = 0, S1PTW = 0 [ 0.926714] swapper pgtable: 4k pages, 48-bit VAs, pgdp=00000000427b3000 [ 0.928489] [ffffa0006b60f5ac] pgd=000000023ffff003, p4d=000000023ffff003, pud=000000023fffe003, pmd=0068000042000f01 [ 0.931330] Internal error: Oops: 8600000e [#1] PREEMPT SMP [ 0.932806] Modules linked in: [ 0.933617] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.10.0-rc7 #2 [ 0.935620] Hardware name: linux,dummy-virt (DT) [ 0.936924] pstate: 40400005 (nZcv daif +PAN -UAO -TCO BTYPE=--) [ 0.938609] pc : asan.module_ctor+0x0/0x14 [ 0.939759] lr : do_basic_setup+0x4c/0x70 [ 0.940889] sp : ffff27b600177e30 [ 0.941815] x29: ffff27b600177e30 x28: 0000000000000000 [ 0.943306] x27: 0000000000000000 x26: 0000000000000000 [ 0.944803] x25: 0000000000000000 x24: 0000000000000000 [ 0.946289] x23: 0000000000000001 x22: 0000000000000000 [ 0.947777] x21: ffffa0006bf4a890 x20: ffffa0006befb6c0 [ 0.949271] x19: ffffa0006bef9358 x18: 0000000000000068 [ 0.950756] x17: fffffffffffffff8 x16: 0000000000000000 [ 0.952246] x15: 0000000000000000 x14: 0000000000000000 [ 0.953734] x13: 00000000838a16d5 x12: 0000000000000001 [ 0.955223] x11: ffff94000da74041 x10: dfffa00000000000 [ 0.956715] x9 : 0000000000000000 x8 : ffffa0006b60f5ac [ 0.958199] x7 : f9f9f9f9f9f9f9f9 x6 : 000000000000003f [ 0.959683] x5 : 0000000000000040 x4 : 0000000000000000 [ 0.961178] x3 : ffffa0006bdc15a0 x2 : 0000000000000005 [ 0.962662] x1 : 00000000000000f9 x0 : ffffa0006bef9350 [ 0.964155] Call trace: [ 0.964844] asan.module_ctor+0x0/0x14 [ 0.965895] kernel_init_freeable+0x158/0x198 [ 0.967115] kernel_init+0x14/0x19c [ 0.968104] ret_from_fork+0x10/0x30 [ 0.969110] Code: 00000003 00000000 00000000 00000000 (00000000) [ 0.970815] ---[ end trace b5339784e20d015c ]--- Cc: Arnd Bergmann <arnd@arndb.de> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Kees Cook <keescook@chromium.org> Acked-by: Kees Cook <keescook@chromium.org> Signed-off-by: Mark Rutland <mark.rutland@arm.com> Link: https://lore.kernel.org/r/20201207170533.10738-1-mark.rutland@arm.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> --- drivers/misc/lkdtm/Makefile | 2 +- drivers/misc/lkdtm/rodata.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile index cce47a15a79f..aeb960cb096d 100644 --- a/drivers/misc/lkdtm/Makefile +++ b/drivers/misc/lkdtm/Makefile @@ -13,7 +13,7 @@ KCOV_INSTRUMENT_rodata.o := n OBJCOPYFLAGS := OBJCOPYFLAGS_rodata_objcopy.o := \ - --rename-section .text=.rodata,alloc,readonly,load + --rename-section .noinstr.text=.rodata,alloc,readonly,load targets += rodata.o rodata_objcopy.o $(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE $(call if_changed,objcopy) diff --git a/drivers/misc/lkdtm/rodata.c b/drivers/misc/lkdtm/rodata.c index 58d180af72cf..baacb876d1d9 100644 --- a/drivers/misc/lkdtm/rodata.c +++ b/drivers/misc/lkdtm/rodata.c @@ -5,7 +5,7 @@ */ #include "lkdtm.h" -void notrace lkdtm_rodata_do_nothing(void) +void noinstr lkdtm_rodata_do_nothing(void) { /* Does nothing. We just want an architecture agnostic "return". */ } -- 2.31.0.rc2.261.g7f71774620-goog ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [for-stable-4.19 PATCH 0/2] Backport patches to fix KASAN+LKDTM with recent clang on ARM64 2021-03-18 23:54 [for-stable-4.19 PATCH 0/2] Backport patches to fix KASAN+LKDTM with recent clang on ARM64 Nicolas Boichat 2021-03-18 23:54 ` [for-stable-4.19 PATCH 1/2] vmlinux.lds.h: Create section for protection against instrumentation Nicolas Boichat 2021-03-18 23:54 ` [for-stable-4.19 PATCH 2/2] lkdtm: don't move ctors to .rodata Nicolas Boichat @ 2021-03-19 10:30 ` Greg Kroah-Hartman 2 siblings, 0 replies; 8+ messages in thread From: Greg Kroah-Hartman @ 2021-03-19 10:30 UTC (permalink / raw) To: Nicolas Boichat Cc: stable, Alexandre Chartre, Arnd Bergmann, Benjamin Herrenschmidt, Christopher Li, Daniel Axtens, Kees Cook, Masahiro Yamada, Michael Ellerman, Michal Marek, Naveen N. Rao, Nicholas Piggin, Paul Mackerras, Peter Zijlstra, Sasha Levin, Thomas Gleixner, clang-built-linux, linux-arch, linux-kbuild, linux-kernel, linux-sparse, linuxppc-dev On Fri, Mar 19, 2021 at 07:54:14AM +0800, Nicolas Boichat wrote: > > Backport 2 patches that are required to make KASAN+LKDTM work > with recent clang (patch 2/2 has a complete description). > Tested on our chromeos-4.19 branch. > > Patch 1/2 is context conflict only, and 2/2 is a clean backport. > > These patches have been merged to 5.4 stable already. We might > need to backport to older stable branches, but this is what I > could test for now. Both now queued up, thanks. greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-03-19 22:49 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-18 23:54 [for-stable-4.19 PATCH 0/2] Backport patches to fix KASAN+LKDTM with recent clang on ARM64 Nicolas Boichat 2021-03-18 23:54 ` [for-stable-4.19 PATCH 1/2] vmlinux.lds.h: Create section for protection against instrumentation Nicolas Boichat 2021-03-19 10:39 ` Greg Kroah-Hartman 2021-03-19 11:20 ` Alexandre Chartre 2021-03-19 11:55 ` Greg Kroah-Hartman 2021-03-19 22:48 ` Nicolas Boichat 2021-03-18 23:54 ` [for-stable-4.19 PATCH 2/2] lkdtm: don't move ctors to .rodata Nicolas Boichat 2021-03-19 10:30 ` [for-stable-4.19 PATCH 0/2] Backport patches to fix KASAN+LKDTM with recent clang on ARM64 Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).