* [PATCH] lkdtm: don't move ctors to .rodata @ 2020-12-07 17:05 Mark Rutland 2020-12-08 21:20 ` Kees Cook 0 siblings, 1 reply; 9+ messages in thread From: Mark Rutland @ 2020-12-07 17:05 UTC (permalink / raw) To: linux-kernel; +Cc: Mark Rutland, Arnd Bergmann, Greg Kroah-Hartman, Kees Cook 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 ]--- Signed-off-by: Mark Rutland <mark.rutland@arm.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Kees Cook <keescook@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 c70b3822013f..30c8ac24635d 100644 --- a/drivers/misc/lkdtm/Makefile +++ b/drivers/misc/lkdtm/Makefile @@ -16,7 +16,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.11.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] lkdtm: don't move ctors to .rodata 2020-12-07 17:05 [PATCH] lkdtm: don't move ctors to .rodata Mark Rutland @ 2020-12-08 21:20 ` Kees Cook 2020-12-09 14:51 ` Greg Kroah-Hartman 0 siblings, 1 reply; 9+ messages in thread From: Kees Cook @ 2020-12-08 21:20 UTC (permalink / raw) To: Greg Kroah-Hartman, Mark Rutland; +Cc: linux-kernel, Arnd Bergmann On Mon, Dec 07, 2020 at 05:05:33PM +0000, Mark Rutland wrote: > 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 ]--- > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> Oh, eek. Why was a ctor generated at all? But yes, this looks good. Greg, can you pick this up please? Acked-by: Kees Cook <keescook@chromium.org> -- Kees Cook ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] lkdtm: don't move ctors to .rodata 2020-12-08 21:20 ` Kees Cook @ 2020-12-09 14:51 ` Greg Kroah-Hartman 2021-02-11 0:36 ` Stephen Boyd 0 siblings, 1 reply; 9+ messages in thread From: Greg Kroah-Hartman @ 2020-12-09 14:51 UTC (permalink / raw) To: Kees Cook; +Cc: Mark Rutland, linux-kernel, Arnd Bergmann On Tue, Dec 08, 2020 at 01:20:56PM -0800, Kees Cook wrote: > On Mon, Dec 07, 2020 at 05:05:33PM +0000, Mark Rutland wrote: > > 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 ]--- > > > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > > Oh, eek. Why was a ctor generated at all? But yes, this looks good. > Greg, can you pick this up please? > > Acked-by: Kees Cook <keescook@chromium.org> Now picked up, thanks. greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] lkdtm: don't move ctors to .rodata 2020-12-09 14:51 ` Greg Kroah-Hartman @ 2021-02-11 0:36 ` Stephen Boyd 2021-02-11 14:23 ` Greg Kroah-Hartman 0 siblings, 1 reply; 9+ messages in thread From: Stephen Boyd @ 2021-02-11 0:36 UTC (permalink / raw) To: Greg Kroah-Hartman, Kees Cook Cc: Mark Rutland, linux-kernel, Arnd Bergmann, stable Quoting Greg Kroah-Hartman (2020-12-09 06:51:33) > On Tue, Dec 08, 2020 at 01:20:56PM -0800, Kees Cook wrote: > > On Mon, Dec 07, 2020 at 05:05:33PM +0000, Mark Rutland wrote: > > > 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 ]--- > > > > > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > > > > Oh, eek. Why was a ctor generated at all? But yes, this looks good. > > Greg, can you pick this up please? > > > > Acked-by: Kees Cook <keescook@chromium.org> > > Now picked up, thanks. > Can this be backported to 5.4 and 5.10 stable trees? I just ran across this trying to use kasan on 5.4 with lkdtm and it blows up early. This patch applies on 5.4 cleanly but doesn't compile because it's missing noinstr. Here's a version of the patch that introduces noinstr on 5.4.97 so this patch can be picked to 5.4 stable trees. ----8<---- From: Thomas Gleixner <tglx@linutronix.de> Date: Mon, 9 Mar 2020 22:47:17 +0100 Subject: [PATCH] vmlinux.lds.h: Create section for protection against instrumentation 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 [swboyd@chromium.org: Account for commit eff8728fe698 ("vmlinux.lds.h: Add PGO and AutoFDO input sections") getting picked first] Signed-off-by: Stephen Boyd <swboyd@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 | 53 +++++++++++++++++++++++++++++++ include/linux/compiler_types.h | 4 +++ scripts/mod/modpost.c | 2 +- 6 files changed, 72 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S index a4e576019d79..3ea360cad337 100644 --- a/arch/powerpc/kernel/vmlinux.lds.S +++ b/arch/powerpc/kernel/vmlinux.lds.S @@ -102,6 +102,7 @@ SECTIONS #ifdef CONFIG_PPC64 *(.tramp.ftrace.text); #endif + NOINSTR_TEXT SCHED_TEXT CPUIDLE_TEXT LOCK_TEXT diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h index d1779d442aa5..66397ed10acb 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 130f16cc0b86..9a4a5a43e886 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -510,6 +510,15 @@ #define RODATA RO_DATA_SECTION(4096) #define RO_DATA(align) RO_DATA_SECTION(align) +/* + * 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 @@ -524,6 +533,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 f164a9b12813..9446e8fbe55c 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -134,12 +134,65 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val, /* Annotate a C jump table to allow objtool to follow the code flow */ #define __annotate_jump_table __section(.rodata..c_jump_table) +#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() #define __annotate_jump_table #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 77433633572e..b94d08d055ff 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -118,6 +118,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 52f1152c9838..13cda6aa2688 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -960,7 +960,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" -- https://chromeos.dev ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] lkdtm: don't move ctors to .rodata 2021-02-11 0:36 ` Stephen Boyd @ 2021-02-11 14:23 ` Greg Kroah-Hartman 2021-02-11 18:53 ` Stephen Boyd 0 siblings, 1 reply; 9+ messages in thread From: Greg Kroah-Hartman @ 2021-02-11 14:23 UTC (permalink / raw) To: Stephen Boyd; +Cc: Kees Cook, Mark Rutland, linux-kernel, Arnd Bergmann, stable On Wed, Feb 10, 2021 at 04:36:08PM -0800, Stephen Boyd wrote: > Quoting Greg Kroah-Hartman (2020-12-09 06:51:33) > > On Tue, Dec 08, 2020 at 01:20:56PM -0800, Kees Cook wrote: > > > On Mon, Dec 07, 2020 at 05:05:33PM +0000, Mark Rutland wrote: > > > > 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 ]--- > > > > > > > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > > > > > > Oh, eek. Why was a ctor generated at all? But yes, this looks good. > > > Greg, can you pick this up please? > > > > > > Acked-by: Kees Cook <keescook@chromium.org> > > > > Now picked up, thanks. > > > > Can this be backported to 5.4 and 5.10 stable trees? I just ran across > this trying to use kasan on 5.4 with lkdtm and it blows up early. This > patch applies on 5.4 cleanly but doesn't compile because it's missing > noinstr. Here's a version of the patch that introduces noinstr on 5.4.97 > so this patch can be picked to 5.4 stable trees. Why 5.10? This showed up in 5.8, so how would it be needed there? confused, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] lkdtm: don't move ctors to .rodata 2021-02-11 14:23 ` Greg Kroah-Hartman @ 2021-02-11 18:53 ` Stephen Boyd 2021-02-14 17:16 ` Greg Kroah-Hartman 0 siblings, 1 reply; 9+ messages in thread From: Stephen Boyd @ 2021-02-11 18:53 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Kees Cook, Mark Rutland, linux-kernel, Arnd Bergmann, stable Quoting Greg Kroah-Hartman (2021-02-11 06:23:10) > On Wed, Feb 10, 2021 at 04:36:08PM -0800, Stephen Boyd wrote: > > Quoting Greg Kroah-Hartman (2020-12-09 06:51:33) > > > On Tue, Dec 08, 2020 at 01:20:56PM -0800, Kees Cook wrote: > > > > On Mon, Dec 07, 2020 at 05:05:33PM +0000, Mark Rutland wrote: > > > > > [ 0.969110] Code: 00000003 00000000 00000000 00000000 (00000000) > > > > > [ 0.970815] ---[ end trace b5339784e20d015c ]--- > > > > > > > > > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > > > > > > > > Oh, eek. Why was a ctor generated at all? But yes, this looks good. > > > > Greg, can you pick this up please? > > > > > > > > Acked-by: Kees Cook <keescook@chromium.org> > > > > > > Now picked up, thanks. > > > > > > > Can this be backported to 5.4 and 5.10 stable trees? I just ran across > > this trying to use kasan on 5.4 with lkdtm and it blows up early. This > > patch applies on 5.4 cleanly but doesn't compile because it's missing > > noinstr. Here's a version of the patch that introduces noinstr on 5.4.97 > > so this patch can be picked to 5.4 stable trees. > > Why 5.10? This showed up in 5.8, so how would it be needed there? > Sorry for the confusion. Can commit 655389666643 ("vmlinux.lds.h: Create section for protection against instrumentation") and commit 3f618ab33234 ("lkdtm: don't move ctors to .rodata") be backported to 5.4.y and only commit 3f618ab3323407ee4c6a6734a37eb6e9663ebfb9 be backported to 5.10.y? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] lkdtm: don't move ctors to .rodata 2021-02-11 18:53 ` Stephen Boyd @ 2021-02-14 17:16 ` Greg Kroah-Hartman 2021-02-14 20:41 ` Stephen Boyd 2021-02-14 22:32 ` Sasha Levin 0 siblings, 2 replies; 9+ messages in thread From: Greg Kroah-Hartman @ 2021-02-14 17:16 UTC (permalink / raw) To: Stephen Boyd; +Cc: Kees Cook, Mark Rutland, linux-kernel, Arnd Bergmann, stable On Thu, Feb 11, 2021 at 10:53:10AM -0800, Stephen Boyd wrote: > Quoting Greg Kroah-Hartman (2021-02-11 06:23:10) > > On Wed, Feb 10, 2021 at 04:36:08PM -0800, Stephen Boyd wrote: > > > Quoting Greg Kroah-Hartman (2020-12-09 06:51:33) > > > > On Tue, Dec 08, 2020 at 01:20:56PM -0800, Kees Cook wrote: > > > > > On Mon, Dec 07, 2020 at 05:05:33PM +0000, Mark Rutland wrote: > > > > > > [ 0.969110] Code: 00000003 00000000 00000000 00000000 (00000000) > > > > > > [ 0.970815] ---[ end trace b5339784e20d015c ]--- > > > > > > > > > > > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > > > > > > > > > > Oh, eek. Why was a ctor generated at all? But yes, this looks good. > > > > > Greg, can you pick this up please? > > > > > > > > > > Acked-by: Kees Cook <keescook@chromium.org> > > > > > > > > Now picked up, thanks. > > > > > > > > > > Can this be backported to 5.4 and 5.10 stable trees? I just ran across > > > this trying to use kasan on 5.4 with lkdtm and it blows up early. This > > > patch applies on 5.4 cleanly but doesn't compile because it's missing > > > noinstr. Here's a version of the patch that introduces noinstr on 5.4.97 > > > so this patch can be picked to 5.4 stable trees. > > > > Why 5.10? This showed up in 5.8, so how would it be needed there? > > > > Sorry for the confusion. Can commit 655389666643 ("vmlinux.lds.h: Create > section for protection against instrumentation") and commit 3f618ab33234 > ("lkdtm: don't move ctors to .rodata") be backported to 5.4.y and only > commit 3f618ab3323407ee4c6a6734a37eb6e9663ebfb9 be backported to 5.10.y? 655389666643 ("vmlinux.lds.h: Create section for protection against instrumentation") does not apply cleanly to 5.4.y, so can you provide a working backport for both of those patches to 5.4.y that you have tested? thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] lkdtm: don't move ctors to .rodata 2021-02-14 17:16 ` Greg Kroah-Hartman @ 2021-02-14 20:41 ` Stephen Boyd 2021-02-14 22:32 ` Sasha Levin 1 sibling, 0 replies; 9+ messages in thread From: Stephen Boyd @ 2021-02-14 20:41 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Kees Cook, Mark Rutland, linux-kernel, Arnd Bergmann, stable Quoting Greg Kroah-Hartman (2021-02-14 09:16:29) > On Thu, Feb 11, 2021 at 10:53:10AM -0800, Stephen Boyd wrote: > > > > Sorry for the confusion. Can commit 655389666643 ("vmlinux.lds.h: Create > > section for protection against instrumentation") and commit 3f618ab33234 > > ("lkdtm: don't move ctors to .rodata") be backported to 5.4.y and only > > commit 3f618ab3323407ee4c6a6734a37eb6e9663ebfb9 be backported to 5.10.y? > > 655389666643 ("vmlinux.lds.h: Create section for protection against > instrumentation") does not apply cleanly to 5.4.y, so can you provide a > working backport for both of those patches to 5.4.y that you have > tested? > Ok, will do. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] lkdtm: don't move ctors to .rodata 2021-02-14 17:16 ` Greg Kroah-Hartman 2021-02-14 20:41 ` Stephen Boyd @ 2021-02-14 22:32 ` Sasha Levin 1 sibling, 0 replies; 9+ messages in thread From: Sasha Levin @ 2021-02-14 22:32 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Stephen Boyd, Kees Cook, Mark Rutland, linux-kernel, Arnd Bergmann, stable On Sun, Feb 14, 2021 at 06:16:29PM +0100, Greg Kroah-Hartman wrote: >On Thu, Feb 11, 2021 at 10:53:10AM -0800, Stephen Boyd wrote: >> Sorry for the confusion. Can commit 655389666643 ("vmlinux.lds.h: Create >> section for protection against instrumentation") and commit 3f618ab33234 >> ("lkdtm: don't move ctors to .rodata") be backported to 5.4.y and only >> commit 3f618ab3323407ee4c6a6734a37eb6e9663ebfb9 be backported to 5.10.y? > >655389666643 ("vmlinux.lds.h: Create section for protection against >instrumentation") does not apply cleanly to 5.4.y, so can you provide a >working backport for both of those patches to 5.4.y that you have >tested? It was due to a backport of eff8728fe698 ("vmlinux.lds.h: Add PGO and AutoFDO input sections"). Taking 655389666643 and 3f618ab33234 converged us back with Linus's tree as eff8728fe698 worked around not having those in 5.4. I've fixed it up and queued both of those patches. -- Thanks, Sasha ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-02-14 22:33 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-12-07 17:05 [PATCH] lkdtm: don't move ctors to .rodata Mark Rutland 2020-12-08 21:20 ` Kees Cook 2020-12-09 14:51 ` Greg Kroah-Hartman 2021-02-11 0:36 ` Stephen Boyd 2021-02-11 14:23 ` Greg Kroah-Hartman 2021-02-11 18:53 ` Stephen Boyd 2021-02-14 17:16 ` Greg Kroah-Hartman 2021-02-14 20:41 ` Stephen Boyd 2021-02-14 22:32 ` Sasha Levin
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).