* [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX @ 2017-05-24 13:47 Thomas Gleixner 2017-05-24 14:33 ` Masami Hiramatsu ` (2 more replies) 0 siblings, 3 replies; 27+ messages in thread From: Thomas Gleixner @ 2017-05-24 13:47 UTC (permalink / raw) To: Steven Rostedt Cc: Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra ftrace uses module_alloc() to allocate trampoline pages. The mapping of module_alloc() is RWX, which makes sense as the memory is written to right after allocation. But nothing makes these pages RO after writing to them. This problem exists since ftrace uses trampolines on x86, but it went unnoticed because the W=X sanity check only triggers when the tracer builtin selftests are enabled. Though the mappings are also created W+X w/o the self tests when the tracer is used after booting. Add proper set_memory_rw/ro() calls to [un]protect the trampolines before and after modification. Fixes: f3bea49115b2 ("ftrace/x86: Add dynamic allocated trampoline for ftrace_ops") Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/kernel/ftrace.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -839,7 +839,7 @@ void arch_ftrace_update_trampoline(struc unsigned long offset; unsigned long ip; unsigned int size; - int ret; + int ret, npages; if (ops->trampoline) { /* @@ -848,11 +848,14 @@ void arch_ftrace_update_trampoline(struc */ if (!(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP)) return; + npages = PAGE_ALIGN(ops->trampoline_size) >> PAGE_SHIFT; + set_memory_rw(ops->trampoline, npages); } else { ops->trampoline = create_trampoline(ops, &size); if (!ops->trampoline) return; ops->trampoline_size = size; + npages = PAGE_ALIGN(size) >> PAGE_SHIFT; } offset = calc_trampoline_call_offset(ops->flags & FTRACE_OPS_FL_SAVE_REGS); @@ -863,6 +866,7 @@ void arch_ftrace_update_trampoline(struc /* Do a safe modify in case the trampoline is executing */ new = ftrace_call_replace(ip, (unsigned long)func); ret = update_ftrace_func(ip, new); + set_memory_ro(ops->trampoline, npages); /* The update should never fail */ WARN_ON(ret); ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-24 13:47 [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX Thomas Gleixner @ 2017-05-24 14:33 ` Masami Hiramatsu 2017-05-24 15:04 ` Steven Rostedt 2017-05-24 17:47 ` Steven Rostedt 2 siblings, 0 replies; 27+ messages in thread From: Masami Hiramatsu @ 2017-05-24 14:33 UTC (permalink / raw) To: Thomas Gleixner Cc: Steven Rostedt, Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra On Wed, 24 May 2017 15:47:17 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote: > ftrace uses module_alloc() to allocate trampoline pages. The mapping of > module_alloc() is RWX, which makes sense as the memory is written to right > after allocation. But nothing makes these pages RO after writing to them. > > This problem exists since ftrace uses trampolines on x86, but it went > unnoticed because the W=X sanity check only triggers when the tracer > builtin selftests are enabled. Though the mappings are also created W+X w/o > the self tests when the tracer is used after booting. > > Add proper set_memory_rw/ro() calls to [un]protect the trampolines before > and after modification. > This looks good to me. (same as I did for kprobes trampoline pages) Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org> Thank you, > Fixes: f3bea49115b2 ("ftrace/x86: Add dynamic allocated trampoline for ftrace_ops") > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > arch/x86/kernel/ftrace.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > --- a/arch/x86/kernel/ftrace.c > +++ b/arch/x86/kernel/ftrace.c > @@ -839,7 +839,7 @@ void arch_ftrace_update_trampoline(struc > unsigned long offset; > unsigned long ip; > unsigned int size; > - int ret; > + int ret, npages; > > if (ops->trampoline) { > /* > @@ -848,11 +848,14 @@ void arch_ftrace_update_trampoline(struc > */ > if (!(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP)) > return; > + npages = PAGE_ALIGN(ops->trampoline_size) >> PAGE_SHIFT; > + set_memory_rw(ops->trampoline, npages); > } else { > ops->trampoline = create_trampoline(ops, &size); > if (!ops->trampoline) > return; > ops->trampoline_size = size; > + npages = PAGE_ALIGN(size) >> PAGE_SHIFT; > } > > offset = calc_trampoline_call_offset(ops->flags & FTRACE_OPS_FL_SAVE_REGS); > @@ -863,6 +866,7 @@ void arch_ftrace_update_trampoline(struc > /* Do a safe modify in case the trampoline is executing */ > new = ftrace_call_replace(ip, (unsigned long)func); > ret = update_ftrace_func(ip, new); > + set_memory_ro(ops->trampoline, npages); > > /* The update should never fail */ > WARN_ON(ret); -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-24 13:47 [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX Thomas Gleixner 2017-05-24 14:33 ` Masami Hiramatsu @ 2017-05-24 15:04 ` Steven Rostedt 2017-05-24 17:47 ` Steven Rostedt 2 siblings, 0 replies; 27+ messages in thread From: Steven Rostedt @ 2017-05-24 15:04 UTC (permalink / raw) To: Thomas Gleixner Cc: Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra On Wed, 24 May 2017 15:47:17 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote: > ftrace uses module_alloc() to allocate trampoline pages. The mapping > of module_alloc() is RWX, which makes sense as the memory is written > to right after allocation. But nothing makes these pages RO after > writing to them. > > This problem exists since ftrace uses trampolines on x86, but it went > unnoticed because the W=X sanity check only triggers when the tracer > builtin selftests are enabled. Though the mappings are also created > W+X w/o the self tests when the tracer is used after booting. > > Add proper set_memory_rw/ro() calls to [un]protect the trampolines > before and after modification. > > Fixes: f3bea49115b2 ("ftrace/x86: Add dynamic allocated trampoline > for ftrace_ops") Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Thanks! I was thinking that this was the issue after your last reply. I'll send this to my box at home and run my tests against it. I'll let you know tomorrow the results. -- Steve > --- > arch/x86/kernel/ftrace.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > --- a/arch/x86/kernel/ftrace.c > +++ b/arch/x86/kernel/ftrace.c > @@ -839,7 +839,7 @@ void arch_ftrace_update_trampoline(struc > unsigned long offset; > unsigned long ip; > unsigned int size; > - int ret; > + int ret, npages; > > if (ops->trampoline) { > /* > @@ -848,11 +848,14 @@ void arch_ftrace_update_trampoline(struc > */ > if (!(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP)) > return; > + npages = PAGE_ALIGN(ops->trampoline_size) >> > PAGE_SHIFT; > + set_memory_rw(ops->trampoline, npages); > } else { > ops->trampoline = create_trampoline(ops, &size); > if (!ops->trampoline) > return; > ops->trampoline_size = size; > + npages = PAGE_ALIGN(size) >> PAGE_SHIFT; > } > > offset = calc_trampoline_call_offset(ops->flags & > FTRACE_OPS_FL_SAVE_REGS); @@ -863,6 +866,7 @@ void > arch_ftrace_update_trampoline(struc /* Do a safe modify in case the > trampoline is executing */ new = ftrace_call_replace(ip, (unsigned > long)func); ret = update_ftrace_func(ip, new); > + set_memory_ro(ops->trampoline, npages); > > /* The update should never fail */ > WARN_ON(ret); ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-24 13:47 [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX Thomas Gleixner 2017-05-24 14:33 ` Masami Hiramatsu 2017-05-24 15:04 ` Steven Rostedt @ 2017-05-24 17:47 ` Steven Rostedt 2017-05-24 18:16 ` Luis R. Rodriguez 2017-05-24 19:13 ` Thomas Gleixner 2 siblings, 2 replies; 27+ messages in thread From: Steven Rostedt @ 2017-05-24 17:47 UTC (permalink / raw) To: Thomas Gleixner Cc: Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra [-- Attachment #1: Type: text/plain, Size: 4866 bytes --] OK, it crashed on one of my tests. I removed the patch and it boots fine, and crashes when I add it back. Here's the crash: Running tests on all trace events: Testing all events: OK Testing ftrace filter: OK trace_kprobe: Testing kprobe tracing: BUG: unable to handle kernel paging request at ffff880214f5c000 IP: new_slab+0x1e8/0x2b4 PGD 338c067 P4D 338c067 PUD 338f067 PMD 212022063 PTE 8000000214f5c161 Oops: 0003 [#1] SMP Modules linked in: CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42 Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014 task: ffff8802153a8000 task.stack: ffffc90000c74000 RIP: 0010:new_slab+0x1e8/0x2b4 RSP: 0000:ffffc90000c77b28 EFLAGS: 00010282 RAX: 0000000040040000 RBX: ffff880216003f00 RCX: ffff880214f5c058 RDX: 0000000000000000 RSI: ffff880214f5c000 RDI: ffff880216003f00 RBP: ffffc90000c77b70 R08: 000000000000002a R09: 0000000000000000 R10: 00000000000201e2 R11: 0000000000020190 R12: ffff880214f5c000 R13: 000000000000002e R14: 0000000000000001 R15: ffffea000853d700 FS: 0000000000000000(0000) GS:ffff88021eb80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffff880214f5c000 CR3: 000000000221d000 CR4: 00000000001406e0 Call Trace: ? interleave_nodes+0x29/0x40 ___slab_alloc+0x2e8/0x49e ? trace_create_new_event+0x1f/0x63 ? trace_add_event_call+0x27/0x91 ? mark_lock+0x24/0x1ec ? trace_create_new_event+0x1f/0x63 __slab_alloc+0x46/0x6b ? __slab_alloc+0x46/0x6b ? trace_create_new_event+0x1f/0x63 kmem_cache_alloc+0x59/0x1b2 trace_create_new_event+0x1f/0x63 __trace_add_new_event+0xd/0x29 trace_add_event_call+0x66/0x91 create_trace_kprobe+0x722/0x7f5 ? argv_split+0x6c/0xce ? probes_open+0x3b/0x3b ? set_debug_rodata+0x17/0x17 traceprobe_command+0x42/0x57 kprobe_trace_self_tests_init+0x3c/0x338 ? kprobe_trace_selftest_target+0x13/0x13 ? set_debug_rodata+0x17/0x17 do_one_initcall+0x90/0x131 ? set_debug_rodata+0x17/0x17 kernel_init_freeable+0x1f4/0x27c ? rest_init+0x12d/0x12d kernel_init+0xe/0xfa ret_from_fork+0x2e/0x40 Code: ff ff 48 8b 53 48 48 85 d2 74 05 4c 89 e7 ff d2 66 41 8b 57 1a 81 e2 ff 7f 00 00 44 39 f2 48 63 53 20 7e 0d 48 63 4b 18 4c 01 e1 <49> 89 0c 14 eb 08 49 c7 04 14 00 00 00 00 48 63 53 18 41 ff c6 RIP: new_slab+0x1e8/0x2b4 RSP: ffffc90000c77b28 CR2: ffff880214f5c000 ---[ end trace e8a2200699d40525 ]--- Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009 Kernel Offset: disabled ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009 sched: Unexpected reschedule of offline CPU#1! ------------[ cut here ]------------ WARNING: CPU: 3 PID: 1 at /work/git/linux-trace.git/arch/x86/kernel/smp.c:128 native_smp_send_reschedule+0x23/0x3b Modules linked in: CPU: 3 PID: 1 Comm: swapper/0 Tainted: G D 4.12.0-rc2-test+ #42 Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014 task: ffff8802153a8000 task.stack: ffffc90000c74000 RIP: 0010:native_smp_send_reschedule+0x23/0x3b RSP: 0000:ffff88021eb83e60 EFLAGS: 00010086 RAX: 000000000000002e RBX: 0000000000000001 RCX: ffff8802153a8000 RDX: ffffffff8226ba60 RSI: ffffffff8109ff35 RDI: ffffffff8108e779 RBP: ffff88021eb83e60 R08: 0000000000000001 R09: 0000000000000001 R10: ffff88021eb83de8 R11: ffffffff832b238c R12: 0000000000000003 R13: 0000000000000003 R14: ffff880216002270 R15: 0000000000000001 FS: 0000000000000000(0000) GS:ffff88021eb80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffff880214f5c000 CR3: 000000000221d000 CR4: 00000000001406e0 Call Trace: <IRQ> trigger_load_balance+0x2dc/0x2e5 scheduler_tick+0x92/0x9b update_process_times+0x47/0x54 tick_sched_handle+0x44/0x53 tick_sched_timer+0x39/0x5f __hrtimer_run_queues+0x141/0x2c8 ? tick_sched_do_timer+0x2e/0x2e hrtimer_interrupt+0x6c/0x12e local_apic_timer_interrupt+0x4b/0x4e smp_apic_timer_interrupt+0x29/0x39 apic_timer_interrupt+0x90/0xa0 RIP: 0010:panic+0x1e1/0x21f RSP: 0000:ffffc90000c77e70 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff10 RAX: 00000003810a049e RBX: 0000000000000000 RCX: ffff8802153a8000 RDX: ffffc90000c77e00 RSI: ffffffff81133f1a RDI: ffffffff81091058 RBP: ffffc90000c77ee0 R08: 0000000000000001 R09: 0000000000000001 R10: ffffc90000c77d78 R11: ffffffff8265a60a R12: 0000000000000000 R13: 0000000000000000 R14: 0000000000000001 R15: 0000000000000009 </IRQ> ? panic+0x1de/0x21f ? trace_hardirqs_on+0xd/0xf do_exit+0x568/0xa40 rewind_stack_do_exit+0x17/0x20 Code: ff 90 a0 00 00 00 5d c3 0f 1f 44 00 00 55 89 f8 48 89 e5 48 0f a3 05 ec 05 41 01 72 12 89 fe 48 c7 c7 5d 0b fe 81 e8 7a 2a 10 00 <0f> ff eb 12 48 8b 05 31 62 0e 01 be fd 00 00 00 ff 90 a0 00 00 ---[ end trace e8a2200699d40526 ]--- I attached the config. I don't currently have time to look deeper into it. But I should have some time later today. -- Steve [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 29237 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-24 17:47 ` Steven Rostedt @ 2017-05-24 18:16 ` Luis R. Rodriguez 2017-05-24 18:53 ` Thomas Gleixner 2017-05-24 19:13 ` Thomas Gleixner 1 sibling, 1 reply; 27+ messages in thread From: Luis R. Rodriguez @ 2017-05-24 18:16 UTC (permalink / raw) To: Steven Rostedt Cc: Thomas Gleixner, Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra On Wed, May 24, 2017 at 01:47:28PM -0400, Steven Rostedt wrote: > OK, it crashed on one of my tests. I removed the patch and it boots > fine, and crashes when I add it back. > > Here's the crash: > > Running tests on all trace events: > Testing all events: OK > Testing ftrace filter: OK > trace_kprobe: Testing kprobe tracing: > BUG: unable to handle kernel paging request at ffff880214f5c000 > IP: new_slab+0x1e8/0x2b4 > PGD 338c067 > P4D 338c067 > PUD 338f067 > PMD 212022063 > PTE 8000000214f5c161 The good news is that this also fixes the other issue I reported with CONFIG_KPROBES_SANITY_TEST=y and the same WARN complaint without a crash [0]. Please note that the issue I reported actually is present since the infrastructure to report on these warnings was added through commit e1a58320a38dfa72be48a0f1a3a92273663ba6db ("x86/mm: Warn on W^X mappings") which was added through kernel 4.3.0-rc3. Thomas's patch says: "Fixes: f3bea49115b2 ("ftrace/x86: Add dynamic allocated trampoline for ftrace_ops"), and this was added through kernel v3.19-rc1 so it may well be the same issue given the traces on my issue also reflect a module_alloc() trace after arch_ftrace_update_trampoline() which matches what Thomas also saw when inspecting his buggy page with kmemleak echo dump [1]. [0] https://lkml.kernel.org/r/20170524175508.GZ8951@wotan.suse.de [1] https://lkml.kernel.org/r/alpine.DEB.2.20.1705232243520.2409@nanos BUT I'm also able to reprorduce a crash too though easily, as follows where as before this commit it was passing all 45 tests just fine: mcgrof@piggy ~/linux-next (git::next-20170519)$ sudo tools/testing/selftests/ftrace/ftracetest === Ftrace unit tests === [1] Basic trace file check [PASS] [2] Basic test for tracers [PASS] [3] Basic trace clock test [PASS] [4] Basic event tracing check [PASS] [5] event tracing - enable/disable with event level files [PASS] [6] event tracing - restricts events based on pid [PASS] [7] event tracing - enable/disable with subsystem level files [PASS] [8] event tracing - enable/disable with top level files [PASS] [9] ftrace - function graph filters with stack tracer [PASS] [10] ftrace - function graph filters [PASS] [11] ftrace - test for function event triggers <it hangs there> I managed to get the kernel log: [ 429.416279] in mmio_trace_init [ 429.504315] mmiotrace: Disabling non-boot CPUs... [ 429.545572] Unregister pv shared memory for cpu 1 [ 429.551725] smpboot: CPU 1 is now offline [ 429.553044] mmiotrace: CPU1 is down. [ 429.572874] Unregister pv shared memory for cpu 2 [ 429.576361] smpboot: CPU 2 is now offline [ 429.578355] mmiotrace: CPU2 is down. [ 429.596755] Unregister pv shared memory for cpu 3 [ 429.601946] smpboot: CPU 3 is now offline [ 429.602768] mmiotrace: CPU3 is down. [ 429.603418] mmiotrace: enabled. [ 429.605001] in mmio_trace_reset [ 429.605003] mmiotrace: Re-enabling CPUs... [ 429.605933] x86: Booting SMP configuration: [ 429.606505] smpboot: Booting Node 0 Processor 1 APIC 0x1 [ 429.617989] kvm-clock: cpu 1, msr 1:3fff0041, secondary cpu clock [ 429.640600] KVM setup async PF for cpu 1 [ 429.640603] kvm-stealtime: cpu 1, msr 13fc8da40 [ 429.642384] mmiotrace: enabled CPU1. [ 429.643391] smpboot: Booting Node 0 Processor 2 APIC 0x2 [ 429.654788] kvm-clock: cpu 2, msr 1:3fff0081, secondary cpu clock [ 429.675512] KVM setup async PF for cpu 2 [ 429.675921] kvm-stealtime: cpu 2, msr 13fd0da40 [ 429.676895] mmiotrace: enabled CPU2. [ 429.678531] smpboot: Booting Node 0 Processor 3 APIC 0x3 [ 429.689999] kvm-clock: cpu 3, msr 1:3fff00c1, secondary cpu clock [ 429.710808] KVM setup async PF for cpu 3 [ 429.711183] kvm-stealtime: cpu 3, msr 13fd8da40 [ 429.712123] mmiotrace: enabled CPU3. [ 429.728330] mmiotrace: disabled. [ 440.537183] BUG: unable to handle kernel paging request at ffff964cf9376000 [ 440.538899] IP: tlb_next_batch.isra.54+0x41/0x70 [ 440.540022] PGD b736f067 [ 440.540024] P4D b736f067 [ 440.540707] PUD b7373067 [ 440.541318] PMD 12ff82063 [ 440.541953] PTE 8000000139376161 [ 440.543555] Oops: 0003 [#1] SMP [ 440.543966] Modules linked in: ppdev(E) joydev(E) evdev(E) serio_raw(E) pcspkr(E) bochs_drm(E) drm_kms_helper(E) ttm(E) drm(E) i2c_piix4(E) sg(E) parport_pc(E) parport(E) acpi_cpufreq(E) button(E) sch_fq_codel(E) autofs4(E) crc32c_generic(E) ext4(E) crc16(E) jbd2(E) mbcache(E) sr_mod(E) cdrom(E) sd_mod(E) ata_generic(E) psmouse(E) floppy(E) ata_piix(E) e1000(E) libata(E) scsi_mod(E) [ 440.548057] CPU: 2 PID: 1765 Comm: ftracetest Tainted: G E 4.12.0-rc1-next-20170519+ #155 [ 440.549159] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014 [ 440.550329] task: ffff964cf14e2600 task.stack: ffffbe7c80b58000 [ 440.550921] RIP: 0010:tlb_next_batch.isra.54+0x41/0x70 [ 440.551385] RSP: 0018:ffffbe7c80b5bc40 EFLAGS: 00010202 [ 440.551855] RAX: ffff964cf9376000 RBX: ffffbe7c80b5bdf0 RCX: 000000000001dd18 [ 440.552493] RDX: 0000000000000000 RSI: 0000000000000011 RDI: 0000000000000246 [ 440.553134] RBP: ffffbe7c80b5bc50 R08: ffff964cf0b6d958 R09: 0000000000000000 [ 440.553750] R10: 0000000000000000 R11: 0000000000000000 R12: ffffbe7c80b5be48 [ 440.554386] R13: ffffbe7c80b5bdd0 R14: 00005560d7a30000 R15: ffff964cf035c178 [ 440.555025] FS: 0000000000000000(0000) GS:ffff964cffd00000(0000) knlGS:0000000000000000 [ 440.555743] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 440.556255] CR2: ffff964cf9376000 CR3: 0000000139688000 CR4: 00000000000006e0 [ 440.556895] Call Trace: [ 440.557107] unmap_page_range+0x722/0x930 [ 440.557443] ? cpumask_any_but+0x24/0x40 [ 440.557773] unmap_single_vma+0x59/0xe0 [ 440.558088] ? lru_deactivate_file_fn+0x360/0x360 [ 440.558485] unmap_vmas+0x51/0xa0 [ 440.558769] exit_mmap+0xa7/0x170 [ 440.559054] mmput+0x53/0x140 [ 440.559309] do_exit+0x286/0xb30 [ 440.559582] ? __do_page_fault+0x266/0x4e0 [ 440.559928] do_group_exit+0x43/0xb0 [ 440.560230] SyS_exit_group+0x14/0x20 [ 440.560539] entry_SYSCALL_64_fastpath+0x1e/0xa9 [ 440.560963] RIP: 0033:0x7f9533db24d8 [ 440.561287] RSP: 002b:00007ffda4899cf8 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7 [ 440.561963] RAX: ffffffffffffffda RBX: 00005560d821c110 RCX: 00007f9533db24d8 [ 440.562601] RDX: 0000000000000000 RSI: 000000000000003c RDI: 0000000000000000 [ 440.563253] RBP: 00007f9534092b00 R08: 00000000000000e7 R09: ffffffffffffff98 [ 440.563863] R10: 00005560d82208a0 R11: 0000000000000246 R12: 0000000000000000 [ 440.564437] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 [ 440.565023] Code: 00 00 c3 83 3e 13 74 4f 55 48 89 e5 41 54 53 49 89 f4 48 89 fb 31 f6 bf 00 02 00 01 e8 d9 f0 fc ff 48 85 c0 74 2d 41 83 04 24 01 <48> c7 00 00 00 00 00 c7 40 08 00 00 00 00 c7 40 0c fe 01 00 00 [ 440.566681] RIP: tlb_next_batch.isra.54+0x41/0x70 RSP: ffffbe7c80b5bc40 [ 440.567283] CR2: ffff964cf9376000 [ 440.567592] ---[ end trace 382f2593927e722f ]--- [ 440.567991] Fixing recursive fault but reboot is needed! Luis ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-24 18:16 ` Luis R. Rodriguez @ 2017-05-24 18:53 ` Thomas Gleixner 2017-05-24 19:34 ` Luis R. Rodriguez 0 siblings, 1 reply; 27+ messages in thread From: Thomas Gleixner @ 2017-05-24 18:53 UTC (permalink / raw) To: Luis R. Rodriguez Cc: Steven Rostedt, Kees Cook, LKML, x86, Masami Hiramatsu, Peter Zijlstra On Wed, 24 May 2017, Luis R. Rodriguez wrote: > [ 440.548057] CPU: 2 PID: 1765 Comm: ftracetest Tainted: G E 4.12.0-rc1-next-20170519+ #155 Can you please try that patch against plain 4.12-rc2? Thanks, tglx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-24 18:53 ` Thomas Gleixner @ 2017-05-24 19:34 ` Luis R. Rodriguez 0 siblings, 0 replies; 27+ messages in thread From: Luis R. Rodriguez @ 2017-05-24 19:34 UTC (permalink / raw) To: Thomas Gleixner Cc: Luis R. Rodriguez, Steven Rostedt, Kees Cook, LKML, x86, Masami Hiramatsu, Peter Zijlstra On Wed, May 24, 2017 at 08:53:00PM +0200, Thomas Gleixner wrote: > On Wed, 24 May 2017, Luis R. Rodriguez wrote: > > [ 440.548057] CPU: 2 PID: 1765 Comm: ftracetest Tainted: G E > > 4.12.0-rc1-next-20170519+ #155 > > Can you please try that patch against plain 4.12-rc2? Yup, just tried it. Same trace. Could we need to do something special at __unregister_ftrace_function() and friends ? Luis ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-24 17:47 ` Steven Rostedt 2017-05-24 18:16 ` Luis R. Rodriguez @ 2017-05-24 19:13 ` Thomas Gleixner 2017-05-24 22:25 ` Steven Rostedt 1 sibling, 1 reply; 27+ messages in thread From: Thomas Gleixner @ 2017-05-24 19:13 UTC (permalink / raw) To: Steven Rostedt Cc: Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra On Wed, 24 May 2017, Steven Rostedt wrote: > OK, it crashed on one of my tests. I removed the patch and it boots > fine, and crashes when I add it back. It reproduces here with your config. > BUG: unable to handle kernel paging request at ffff880214f5c000 > IP: new_slab+0x1e8/0x2b4 > PGD 338c067 > P4D 338c067 > PUD 338f067 > PMD 212022063 > PTE 8000000214f5c161 > > Oops: 0003 [#1] SMP > Modules linked in: > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42 > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014 > task: ffff8802153a8000 task.stack: ffffc90000c74000 > RIP: 0010:new_slab+0x1e8/0x2b4 > RSP: 0000:ffffc90000c77b28 EFLAGS: 00010282 > RAX: 0000000040040000 RBX: ffff880216003f00 RCX: ffff880214f5c058 > RDX: 0000000000000000 RSI: ffff880214f5c000 RDI: ffff880216003f00 > RBP: ffffc90000c77b70 R08: 000000000000002a R09: 0000000000000000 > R10: 00000000000201e2 R11: 0000000000020190 R12: ffff880214f5c000 > R13: 000000000000002e R14: 0000000000000001 R15: ffffea000853d700 > FS: 0000000000000000(0000) GS:ffff88021eb80000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: ffff880214f5c000 CR3: 000000000221d000 CR4: 00000000001406e0 > Call Trace: > ? interleave_nodes+0x29/0x40 > ___slab_alloc+0x2e8/0x49e That does not make any sense, but I'm digging into it. Thanks, tglx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-24 19:13 ` Thomas Gleixner @ 2017-05-24 22:25 ` Steven Rostedt 2017-05-24 23:18 ` Luis R. Rodriguez ` (2 more replies) 0 siblings, 3 replies; 27+ messages in thread From: Steven Rostedt @ 2017-05-24 22:25 UTC (permalink / raw) To: Thomas Gleixner Cc: Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra On Wed, 24 May 2017 21:13:27 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote: > > Oops: 0003 [#1] SMP > > Modules linked in: > > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42 > > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 > > 02/22/2014 task: ffff8802153a8000 task.stack: ffffc90000c74000 > > RIP: 0010:new_slab+0x1e8/0x2b4 > > RSP: 0000:ffffc90000c77b28 EFLAGS: 00010282 > > RAX: 0000000040040000 RBX: ffff880216003f00 RCX: ffff880214f5c058 > > RDX: 0000000000000000 RSI: ffff880214f5c000 RDI: ffff880216003f00 > > RBP: ffffc90000c77b70 R08: 000000000000002a R09: 0000000000000000 > > R10: 00000000000201e2 R11: 0000000000020190 R12: ffff880214f5c000 > > R13: 000000000000002e R14: 0000000000000001 R15: ffffea000853d700 > > FS: 0000000000000000(0000) GS:ffff88021eb80000(0000) > > knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: > > 0000000080050033 CR2: ffff880214f5c000 CR3: 000000000221d000 CR4: > > 00000000001406e0 Call Trace: > > ? interleave_nodes+0x29/0x40 > > ___slab_alloc+0x2e8/0x49e > > That does not make any sense, but I'm digging into it. The trampolines uses the module allocation, and it appears, that needs to become rw before freeing again. I applied this patch, and it appears to fix the bug for me. Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> -- Steve diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index 663a35d..5e93a9a 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned long size) { return module_alloc(size); } -static inline void tramp_free(void *tramp) +static inline void tramp_free(void *tramp, int size) { + int npages; + + npages = PAGE_ALIGN(size) >> PAGE_SHIFT; + set_memory_rw((unsigned long)tramp, npages); module_memfree(tramp); } #else @@ -699,7 +703,7 @@ static inline void *alloc_tramp(unsigned long size) { return NULL; } -static inline void tramp_free(void *tramp) { } +static inline void tramp_free(void *tramp, int size) { } #endif /* Defined as markers to the end of the ftrace default trampolines */ @@ -771,7 +775,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size) /* Copy ftrace_caller onto the trampoline memory */ ret = probe_kernel_read(trampoline, (void *)start_offset, size); if (WARN_ON(ret < 0)) { - tramp_free(trampoline); + tramp_free(trampoline, *tramp_size); return 0; } @@ -797,7 +801,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size) /* Are we pointing to the reference? */ if (WARN_ON(memcmp(op_ptr.op, op_ref, 3) != 0)) { - tramp_free(trampoline); + tramp_free(trampoline, *tramp_size); return 0; } @@ -943,7 +947,7 @@ void arch_ftrace_trampoline_free(struct ftrace_ops *ops) if (!ops || !(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP)) return; - tramp_free((void *)ops->trampoline); + tramp_free((void *)ops->trampoline, ops->trampoline_size); ops->trampoline = 0; } ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-24 22:25 ` Steven Rostedt @ 2017-05-24 23:18 ` Luis R. Rodriguez 2017-05-25 6:25 ` Thomas Gleixner 2017-05-25 9:09 ` [PATCH] " Masami Hiramatsu 2 siblings, 0 replies; 27+ messages in thread From: Luis R. Rodriguez @ 2017-05-24 23:18 UTC (permalink / raw) To: Steven Rostedt Cc: Thomas Gleixner, Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra On Wed, May 24, 2017 at 06:25:47PM -0400, Steven Rostedt wrote: > On Wed, 24 May 2017 21:13:27 +0200 (CEST) > Thomas Gleixner <tglx@linutronix.de> wrote: > > > > > Oops: 0003 [#1] SMP > > > Modules linked in: > > > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42 > > > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 > > > 02/22/2014 task: ffff8802153a8000 task.stack: ffffc90000c74000 > > > RIP: 0010:new_slab+0x1e8/0x2b4 > > > RSP: 0000:ffffc90000c77b28 EFLAGS: 00010282 > > > RAX: 0000000040040000 RBX: ffff880216003f00 RCX: ffff880214f5c058 > > > RDX: 0000000000000000 RSI: ffff880214f5c000 RDI: ffff880216003f00 > > > RBP: ffffc90000c77b70 R08: 000000000000002a R09: 0000000000000000 > > > R10: 00000000000201e2 R11: 0000000000020190 R12: ffff880214f5c000 > > > R13: 000000000000002e R14: 0000000000000001 R15: ffffea000853d700 > > > FS: 0000000000000000(0000) GS:ffff88021eb80000(0000) > > > knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: > > > 0000000080050033 CR2: ffff880214f5c000 CR3: 000000000221d000 CR4: > > > 00000000001406e0 Call Trace: > > > ? interleave_nodes+0x29/0x40 > > > ___slab_alloc+0x2e8/0x49e > > > > That does not make any sense, but I'm digging into it. > > The trampolines uses the module allocation, and it appears, that needs > to become rw before freeing again. > > I applied this patch, and it appears to fix the bug for me. > > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> Awesome, this also cures my panic: mcgrof@piggy ~/linux-next (git::20170524-fixwarn)$ sudo tools/testing/selftests/ftrace/ftracetest ... # of passed: 45 # of failed: 0 # of unresolved: 0 # of untested: 0 # of unsupported: 0 # of xfailed: 0 # of undefined(test bug): 0 So the combination of both: Tested-by: Luis R. Rodriguez <mcgrof@kernel.org> Luis ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-24 22:25 ` Steven Rostedt 2017-05-24 23:18 ` Luis R. Rodriguez @ 2017-05-25 6:25 ` Thomas Gleixner 2017-05-25 8:57 ` [PATCH V2] " Thomas Gleixner 2017-05-25 9:09 ` [PATCH] " Masami Hiramatsu 2 siblings, 1 reply; 27+ messages in thread From: Thomas Gleixner @ 2017-05-25 6:25 UTC (permalink / raw) To: Steven Rostedt Cc: Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra On Wed, 24 May 2017, Steven Rostedt wrote: > The trampolines uses the module allocation, and it appears, that needs > to become rw before freeing again. Indeed. I realized that when enabling more debug options, which led to a reliable triple fault. How intuitive. > I applied this patch, and it appears to fix the bug for me. It fixes the bug, but ... > -static inline void tramp_free(void *tramp) > +static inline void tramp_free(void *tramp, int size) > { > + int npages; > + > + npages = PAGE_ALIGN(size) >> PAGE_SHIFT; For correctness sake this wants set_memory_nx(...); as well. > + set_memory_rw((unsigned long)tramp, npages); > module_memfree(tramp); > } I'll clean that up and post a V2. Thanks, tglx ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-25 6:25 ` Thomas Gleixner @ 2017-05-25 8:57 ` Thomas Gleixner 2017-05-25 15:15 ` Steven Rostedt ` (2 more replies) 0 siblings, 3 replies; 27+ messages in thread From: Thomas Gleixner @ 2017-05-25 8:57 UTC (permalink / raw) To: Steven Rostedt Cc: Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra ftrace use module_alloc() to allocate trampoline pages. The mapping of module_alloc() is RWX, which makes sense as the memory is written to right after allocation. But nothing makes these pages RO after writing to them. Add proper set_memory_rw/ro() calls to protect the trampolines after modification. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/kernel/ftrace.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned { return module_alloc(size); } -static inline void tramp_free(void *tramp) +static inline void tramp_free(void *tramp, int size) { + int npages = PAGE_ALIGN(size) >> PAGE_SHIFT; + + set_memory_nx((unsigned long)tramp, npages); + set_memory_rw((unsigned long)tramp, npages); module_memfree(tramp); } #else @@ -699,7 +703,7 @@ static inline void *alloc_tramp(unsigned { return NULL; } -static inline void tramp_free(void *tramp) { } +static inline void tramp_free(void *tramp, int size) { } #endif /* Defined as markers to the end of the ftrace default trampolines */ @@ -771,7 +775,7 @@ create_trampoline(struct ftrace_ops *ops /* Copy ftrace_caller onto the trampoline memory */ ret = probe_kernel_read(trampoline, (void *)start_offset, size); if (WARN_ON(ret < 0)) { - tramp_free(trampoline); + tramp_free(trampoline, *tramp_size); return 0; } @@ -797,7 +801,7 @@ create_trampoline(struct ftrace_ops *ops /* Are we pointing to the reference? */ if (WARN_ON(memcmp(op_ptr.op, op_ref, 3) != 0)) { - tramp_free(trampoline); + tramp_free(trampoline, *tramp_size); return 0; } @@ -839,7 +843,7 @@ void arch_ftrace_update_trampoline(struc unsigned long offset; unsigned long ip; unsigned int size; - int ret; + int ret, npages; if (ops->trampoline) { /* @@ -848,11 +852,14 @@ void arch_ftrace_update_trampoline(struc */ if (!(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP)) return; + npages = PAGE_ALIGN(ops->trampoline_size) >> PAGE_SHIFT; + set_memory_rw(ops->trampoline, npages); } else { ops->trampoline = create_trampoline(ops, &size); if (!ops->trampoline) return; ops->trampoline_size = size; + npages = PAGE_ALIGN(size) >> PAGE_SHIFT; } offset = calc_trampoline_call_offset(ops->flags & FTRACE_OPS_FL_SAVE_REGS); @@ -863,6 +870,7 @@ void arch_ftrace_update_trampoline(struc /* Do a safe modify in case the trampoline is executing */ new = ftrace_call_replace(ip, (unsigned long)func); ret = update_ftrace_func(ip, new); + set_memory_ro(ops->trampoline, npages); /* The update should never fail */ WARN_ON(ret); @@ -939,7 +947,7 @@ void arch_ftrace_trampoline_free(struct if (!ops || !(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP)) return; - tramp_free((void *)ops->trampoline); + tramp_free((void *)ops->trampoline, ops->trampoline_size); ops->trampoline = 0; } ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-25 8:57 ` [PATCH V2] " Thomas Gleixner @ 2017-05-25 15:15 ` Steven Rostedt 2017-05-25 17:46 ` Luis R. Rodriguez 2017-05-26 13:37 ` Steven Rostedt 2 siblings, 0 replies; 27+ messages in thread From: Steven Rostedt @ 2017-05-25 15:15 UTC (permalink / raw) To: Thomas Gleixner Cc: Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra On Thu, 25 May 2017 10:57:51 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote: > ftrace use module_alloc() to allocate trampoline pages. The mapping of > module_alloc() is RWX, which makes sense as the memory is written to > right after allocation. But nothing makes these pages RO after > writing to them. > > Add proper set_memory_rw/ro() calls to protect the trampolines after > modification. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> OK, I pulled this in and I'm currently running it through my test suite. I'm about to board a flight, hopefully it runs smoothly and will finish by the time I get home. -- Steve ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-25 8:57 ` [PATCH V2] " Thomas Gleixner 2017-05-25 15:15 ` Steven Rostedt @ 2017-05-25 17:46 ` Luis R. Rodriguez 2017-05-25 19:51 ` Kees Cook 2017-05-26 13:37 ` Steven Rostedt 2 siblings, 1 reply; 27+ messages in thread From: Luis R. Rodriguez @ 2017-05-25 17:46 UTC (permalink / raw) To: Thomas Gleixner Cc: Steven Rostedt, Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra On Thu, May 25, 2017 at 10:57:51AM +0200, Thomas Gleixner wrote: > ftrace use module_alloc() to allocate trampoline pages. The mapping of > module_alloc() is RWX, which makes sense as the memory is written to right > after allocation. But nothing makes these pages RO after writing to them. > > Add proper set_memory_rw/ro() calls to protect the trampolines after > modification. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > arch/x86/kernel/ftrace.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > --- a/arch/x86/kernel/ftrace.c > +++ b/arch/x86/kernel/ftrace.c > @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned > { > return module_alloc(size); > } > -static inline void tramp_free(void *tramp) > +static inline void tramp_free(void *tramp, int size) > { > + int npages = PAGE_ALIGN(size) >> PAGE_SHIFT; > + > + set_memory_nx((unsigned long)tramp, npages); > + set_memory_rw((unsigned long)tramp, npages); > module_memfree(tramp); > } Can/should module_memfree() just do this for users? With Masami's fix that'd be 2 users already. Luis ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-25 17:46 ` Luis R. Rodriguez @ 2017-05-25 19:51 ` Kees Cook 2017-05-26 7:03 ` Thomas Gleixner 2017-05-26 9:49 ` Masami Hiramatsu 0 siblings, 2 replies; 27+ messages in thread From: Kees Cook @ 2017-05-25 19:51 UTC (permalink / raw) To: Luis R. Rodriguez Cc: Thomas Gleixner, Steven Rostedt, LKML, x86, Masami Hiramatsu, Peter Zijlstra On Thu, May 25, 2017 at 10:46 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote: > On Thu, May 25, 2017 at 10:57:51AM +0200, Thomas Gleixner wrote: >> ftrace use module_alloc() to allocate trampoline pages. The mapping of >> module_alloc() is RWX, which makes sense as the memory is written to right >> after allocation. But nothing makes these pages RO after writing to them. >> >> Add proper set_memory_rw/ro() calls to protect the trampolines after >> modification. >> >> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> >> --- >> arch/x86/kernel/ftrace.c | 20 ++++++++++++++------ >> 1 file changed, 14 insertions(+), 6 deletions(-) >> >> --- a/arch/x86/kernel/ftrace.c >> +++ b/arch/x86/kernel/ftrace.c >> @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned >> { >> return module_alloc(size); >> } >> -static inline void tramp_free(void *tramp) >> +static inline void tramp_free(void *tramp, int size) >> { >> + int npages = PAGE_ALIGN(size) >> PAGE_SHIFT; >> + >> + set_memory_nx((unsigned long)tramp, npages); >> + set_memory_rw((unsigned long)tramp, npages); >> module_memfree(tramp); >> } > > Can/should module_memfree() just do this for users? With Masami's fix that'd > be 2 users already. It seems like it really should. That would put it in a single place and avoid this mistake again in the future. Does module_memfree() have access to the allocation size, or does that need to get plumbed? -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-25 19:51 ` Kees Cook @ 2017-05-26 7:03 ` Thomas Gleixner 2017-05-26 9:27 ` Heiko Carstens 2017-05-26 9:49 ` Masami Hiramatsu 1 sibling, 1 reply; 27+ messages in thread From: Thomas Gleixner @ 2017-05-26 7:03 UTC (permalink / raw) To: Kees Cook Cc: Luis R. Rodriguez, Steven Rostedt, LKML, x86, Masami Hiramatsu, Peter Zijlstra On Thu, 25 May 2017, Kees Cook wrote: > On Thu, May 25, 2017 at 10:46 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote: > > On Thu, May 25, 2017 at 10:57:51AM +0200, Thomas Gleixner wrote: > >> ftrace use module_alloc() to allocate trampoline pages. The mapping of > >> module_alloc() is RWX, which makes sense as the memory is written to right > >> after allocation. But nothing makes these pages RO after writing to them. > >> > >> Add proper set_memory_rw/ro() calls to protect the trampolines after > >> modification. > >> > >> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > >> --- > >> arch/x86/kernel/ftrace.c | 20 ++++++++++++++------ > >> 1 file changed, 14 insertions(+), 6 deletions(-) > >> > >> --- a/arch/x86/kernel/ftrace.c > >> +++ b/arch/x86/kernel/ftrace.c > >> @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned > >> { > >> return module_alloc(size); > >> } > >> -static inline void tramp_free(void *tramp) > >> +static inline void tramp_free(void *tramp, int size) > >> { > >> + int npages = PAGE_ALIGN(size) >> PAGE_SHIFT; > >> + > >> + set_memory_nx((unsigned long)tramp, npages); > >> + set_memory_rw((unsigned long)tramp, npages); > >> module_memfree(tramp); > >> } > > > > Can/should module_memfree() just do this for users? With Masami's fix that'd > > be 2 users already. > > It seems like it really should. That would put it in a single place > and avoid this mistake again in the future. Does module_memfree() have > access to the allocation size, or does that need to get plumbed? No, it doesn't. But the number of instances is pretty limited. Btw, looking at BPF. It allocates memory via module_alloc() which means it's RWX. There is nothing in that BPF code which changes the permissions afterwards .... Thanks, tglx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-26 7:03 ` Thomas Gleixner @ 2017-05-26 9:27 ` Heiko Carstens 2017-05-26 9:56 ` Thomas Gleixner 2017-05-26 11:40 ` Michael Ellerman 0 siblings, 2 replies; 27+ messages in thread From: Heiko Carstens @ 2017-05-26 9:27 UTC (permalink / raw) To: Thomas Gleixner Cc: Kees Cook, Luis R. Rodriguez, Steven Rostedt, LKML, x86, Masami Hiramatsu, Peter Zijlstra, Michael Ellerman On Fri, May 26, 2017 at 09:03:13AM +0200, Thomas Gleixner wrote: > > It seems like it really should. That would put it in a single place > > and avoid this mistake again in the future. Does module_memfree() have > > access to the allocation size, or does that need to get plumbed? > > No, it doesn't. But the number of instances is pretty limited. > > Btw, looking at BPF. It allocates memory via module_alloc() which means > it's RWX. There is nothing in that BPF code which changes the permissions > afterwards .... For BPF you're probably referring to bpf_jit_binary_alloc()? Permissions are changed with bpf_jit_binary_lock_ro() within each architecure backend. Well, except for powerpc (cc'ed Michael). ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-26 9:27 ` Heiko Carstens @ 2017-05-26 9:56 ` Thomas Gleixner 2017-05-26 11:40 ` Michael Ellerman 1 sibling, 0 replies; 27+ messages in thread From: Thomas Gleixner @ 2017-05-26 9:56 UTC (permalink / raw) To: Heiko Carstens Cc: Kees Cook, Luis R. Rodriguez, Steven Rostedt, LKML, x86, Masami Hiramatsu, Peter Zijlstra, Michael Ellerman On Fri, 26 May 2017, Heiko Carstens wrote: > On Fri, May 26, 2017 at 09:03:13AM +0200, Thomas Gleixner wrote: > > > It seems like it really should. That would put it in a single place > > > and avoid this mistake again in the future. Does module_memfree() have > > > access to the allocation size, or does that need to get plumbed? > > > > No, it doesn't. But the number of instances is pretty limited. > > > > Btw, looking at BPF. It allocates memory via module_alloc() which means > > it's RWX. There is nothing in that BPF code which changes the permissions > > afterwards .... > > For BPF you're probably referring to bpf_jit_binary_alloc()? Permissions > are changed with bpf_jit_binary_lock_ro() within each architecure backend. > > Well, except for powerpc (cc'ed Michael). The problem starts elsewhere. module_alloc() should not allocate RWX memory in the first place. It should allocated RW and then the usage sites should set it to RX when the code is ready to go. Thanks, tglx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-26 9:27 ` Heiko Carstens 2017-05-26 9:56 ` Thomas Gleixner @ 2017-05-26 11:40 ` Michael Ellerman 1 sibling, 0 replies; 27+ messages in thread From: Michael Ellerman @ 2017-05-26 11:40 UTC (permalink / raw) To: Heiko Carstens, Thomas Gleixner Cc: Kees Cook, Luis R. Rodriguez, Steven Rostedt, LKML, x86, Masami Hiramatsu, Peter Zijlstra Heiko Carstens <heiko.carstens@de.ibm.com> writes: > On Fri, May 26, 2017 at 09:03:13AM +0200, Thomas Gleixner wrote: >> > It seems like it really should. That would put it in a single place >> > and avoid this mistake again in the future. Does module_memfree() have >> > access to the allocation size, or does that need to get plumbed? >> >> No, it doesn't. But the number of instances is pretty limited. >> >> Btw, looking at BPF. It allocates memory via module_alloc() which means >> it's RWX. There is nothing in that BPF code which changes the permissions >> afterwards .... > > For BPF you're probably referring to bpf_jit_binary_alloc()? Permissions > are changed with bpf_jit_binary_lock_ro() within each architecure backend. > > Well, except for powerpc (cc'ed Michael). [hangs head in shame] Thanks, we are working on this stuff (stricter RWX perms) at the moment, so will add this to the list. It's complicated somewhat by the variety of MMUs we support, but still. cheers ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-25 19:51 ` Kees Cook 2017-05-26 7:03 ` Thomas Gleixner @ 2017-05-26 9:49 ` Masami Hiramatsu 1 sibling, 0 replies; 27+ messages in thread From: Masami Hiramatsu @ 2017-05-26 9:49 UTC (permalink / raw) To: Kees Cook Cc: Luis R. Rodriguez, Thomas Gleixner, Steven Rostedt, LKML, x86, Masami Hiramatsu, Peter Zijlstra On Thu, 25 May 2017 12:51:21 -0700 Kees Cook <keescook@chromium.org> wrote: > On Thu, May 25, 2017 at 10:46 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote: > > On Thu, May 25, 2017 at 10:57:51AM +0200, Thomas Gleixner wrote: > >> ftrace use module_alloc() to allocate trampoline pages. The mapping of > >> module_alloc() is RWX, which makes sense as the memory is written to right > >> after allocation. But nothing makes these pages RO after writing to them. > >> > >> Add proper set_memory_rw/ro() calls to protect the trampolines after > >> modification. > >> > >> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > >> --- > >> arch/x86/kernel/ftrace.c | 20 ++++++++++++++------ > >> 1 file changed, 14 insertions(+), 6 deletions(-) > >> > >> --- a/arch/x86/kernel/ftrace.c > >> +++ b/arch/x86/kernel/ftrace.c > >> @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned > >> { > >> return module_alloc(size); > >> } > >> -static inline void tramp_free(void *tramp) > >> +static inline void tramp_free(void *tramp, int size) > >> { > >> + int npages = PAGE_ALIGN(size) >> PAGE_SHIFT; > >> + > >> + set_memory_nx((unsigned long)tramp, npages); > >> + set_memory_rw((unsigned long)tramp, npages); > >> module_memfree(tramp); > >> } > > > > Can/should module_memfree() just do this for users? With Masami's fix that'd > > be 2 users already. > > It seems like it really should. That would put it in a single place > and avoid this mistake again in the future. Does module_memfree() have > access to the allocation size, or does that need to get plumbed? module_memfree() is just a wrapper of vfree, so find_vm_area() will help us to get the size. Thank you, > > -Kees > > -- > Kees Cook > Pixel Security -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-25 8:57 ` [PATCH V2] " Thomas Gleixner 2017-05-25 15:15 ` Steven Rostedt 2017-05-25 17:46 ` Luis R. Rodriguez @ 2017-05-26 13:37 ` Steven Rostedt 2017-05-26 13:50 ` Thomas Gleixner 2 siblings, 1 reply; 27+ messages in thread From: Steven Rostedt @ 2017-05-26 13:37 UTC (permalink / raw) To: Thomas Gleixner Cc: Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra On Thu, 25 May 2017 10:57:51 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote: > ftrace use module_alloc() to allocate trampoline pages. The mapping of > module_alloc() is RWX, which makes sense as the memory is written to right > after allocation. But nothing makes these pages RO after writing to them. > > Add proper set_memory_rw/ro() calls to protect the trampolines after > modification. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- Unfortunately, this was triggered in my tests: ftrace: allocating 54840 entries in 215 pages Starting tracer 'function' ------------[ cut here ]------------ kernel BUG at /work/autotest/nobackup/linux-test.git/arch/x86/mm/pageattr.c:189! invalid opcode: 0000 [#1] SMP Modules linked in: CPU: 0 PID: 0 Comm: swapper Not tainted 4.12.0-rc2-test+ #3 Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014 task: ffffffffb4222500 task.stack: ffffffffb4200000 RIP: 0010:change_page_attr_set_clr+0x269/0x302 RSP: 0000:ffffffffb4203c88 EFLAGS: 00010046 RAX: 0000000000000046 RBX: 0000000000000000 RCX: 00000001b6000000 RDX: ffffffffb4203d40 RSI: 0000000000000000 RDI: ffffffffb4240d60 RBP: ffffffffb4203d18 R08: 00000001b6000000 R09: 0000000000000001 R10: ffffffffb4203aa8 R11: 0000000000000003 R12: ffffffffc029b000 R13: ffffffffb4203d40 R14: 0000000000000001 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffff9a639ea00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffff9a636b384000 CR3: 00000001ea21d000 CR4: 00000000000406b0 Call Trace: change_page_attr_clear+0x1f/0x21 set_memory_ro+0x1e/0x20 arch_ftrace_update_trampoline+0x207/0x21c ? ftrace_caller+0x64/0x64 ? 0xffffffffc029b000 ftrace_startup+0xf4/0x198 register_ftrace_function+0x26/0x3c function_trace_init+0x5e/0x73 tracer_init+0x1e/0x23 tracing_set_tracer+0x127/0x15a register_tracer+0x19b/0x1bc init_function_trace+0x90/0x92 early_trace_init+0x236/0x2b3 start_kernel+0x200/0x3f5 x86_64_start_reservations+0x29/0x2b x86_64_start_kernel+0x17c/0x18f secondary_startup_64+0x9f/0x9f ? secondary_startup_64+0x9f/0x9f Code: 89 df e8 79 f4 ff ff 48 85 c0 74 12 f6 00 01 74 0d be 00 10 00 00 48 89 df e8 84 e8 ff ff 49 ff c4 eb a4 9c 58 0f ba e0 09 72 02 <0f> 0b 49 8d 84 24 ff 0f 00 00 48 25 00 f0 ff ff 49 39 c4 74 02 RIP: change_page_attr_set_clr+0x269/0x302 RSP: ffffffffb4203c88 ---[ end trace 418d67f4f812a298 ]--- Kernel panic - not syncing: Attempted to kill the idle task! ---[ end Kernel panic - not syncing: Attempted to kill the idle task! It appears that if you enable function tracing at boot up, calling set_memory_ro() with interrupts disabled can cause this. As pageattr.c at line 189 has: BUG_ON(irqs_disabled()); in cpa_flush_range() -- Steve ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-26 13:37 ` Steven Rostedt @ 2017-05-26 13:50 ` Thomas Gleixner 2017-05-26 13:58 ` Steven Rostedt 0 siblings, 1 reply; 27+ messages in thread From: Thomas Gleixner @ 2017-05-26 13:50 UTC (permalink / raw) To: Steven Rostedt Cc: Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra On Fri, 26 May 2017, Steven Rostedt wrote: > On Thu, 25 May 2017 10:57:51 +0200 (CEST) > Thomas Gleixner <tglx@linutronix.de> wrote: > > > ftrace use module_alloc() to allocate trampoline pages. The mapping of > > module_alloc() is RWX, which makes sense as the memory is written to right > > after allocation. But nothing makes these pages RO after writing to them. > > > > Add proper set_memory_rw/ro() calls to protect the trampolines after > > modification. > > > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > > --- > > Unfortunately, this was triggered in my tests: > > ftrace: allocating 54840 entries in 215 pages > Starting tracer 'function' > ------------[ cut here ]------------ > kernel BUG at /work/autotest/nobackup/linux-test.git/arch/x86/mm/pageattr.c:189! > > It appears that if you enable function tracing at boot up, calling > set_memory_ro() with interrupts disabled can cause this. As pageattr.c > at line 189 has: > > BUG_ON(irqs_disabled()); That's very early boot, right? So interrupts have to be disabled. So this wants to be: BUG_ON(irqs_disabled() && !early_boot_irqs_disabled); Thanks, tglx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-26 13:50 ` Thomas Gleixner @ 2017-05-26 13:58 ` Steven Rostedt 0 siblings, 0 replies; 27+ messages in thread From: Steven Rostedt @ 2017-05-26 13:58 UTC (permalink / raw) To: Thomas Gleixner Cc: Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra On Fri, 26 May 2017 15:50:38 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote: > That's very early boot, right? So interrupts have to be disabled. > > So this wants to be: > > BUG_ON(irqs_disabled() && !early_boot_irqs_disabled); > I was thinking the same thing. I'll add a patch and retest. Thanks! -- Steve ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-24 22:25 ` Steven Rostedt 2017-05-24 23:18 ` Luis R. Rodriguez 2017-05-25 6:25 ` Thomas Gleixner @ 2017-05-25 9:09 ` Masami Hiramatsu 2017-05-25 10:34 ` Masami Hiramatsu 2 siblings, 1 reply; 27+ messages in thread From: Masami Hiramatsu @ 2017-05-25 9:09 UTC (permalink / raw) To: Steven Rostedt Cc: Thomas Gleixner, Kees Cook, LKML, x86, Masami Hiramatsu, Luis R. Rodriguez, Peter Zijlstra On Wed, 24 May 2017 18:25:47 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Wed, 24 May 2017 21:13:27 +0200 (CEST) > Thomas Gleixner <tglx@linutronix.de> wrote: > > > > > Oops: 0003 [#1] SMP > > > Modules linked in: > > > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42 > > > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 > > > 02/22/2014 task: ffff8802153a8000 task.stack: ffffc90000c74000 > > > RIP: 0010:new_slab+0x1e8/0x2b4 > > > RSP: 0000:ffffc90000c77b28 EFLAGS: 00010282 > > > RAX: 0000000040040000 RBX: ffff880216003f00 RCX: ffff880214f5c058 > > > RDX: 0000000000000000 RSI: ffff880214f5c000 RDI: ffff880216003f00 > > > RBP: ffffc90000c77b70 R08: 000000000000002a R09: 0000000000000000 > > > R10: 00000000000201e2 R11: 0000000000020190 R12: ffff880214f5c000 > > > R13: 000000000000002e R14: 0000000000000001 R15: ffffea000853d700 > > > FS: 0000000000000000(0000) GS:ffff88021eb80000(0000) > > > knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: > > > 0000000080050033 CR2: ffff880214f5c000 CR3: 000000000221d000 CR4: > > > 00000000001406e0 Call Trace: > > > ? interleave_nodes+0x29/0x40 > > > ___slab_alloc+0x2e8/0x49e > > > > That does not make any sense, but I'm digging into it. > > The trampolines uses the module allocation, and it appears, that needs > to become rw before freeing again. Oops, I also have to fix kprobes side, because it has same issue. Thanks! > > I applied this patch, and it appears to fix the bug for me. > > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > > -- Steve > > diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c > index 663a35d..5e93a9a 100644 > --- a/arch/x86/kernel/ftrace.c > +++ b/arch/x86/kernel/ftrace.c > @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned long size) > { > return module_alloc(size); > } > -static inline void tramp_free(void *tramp) > +static inline void tramp_free(void *tramp, int size) > { > + int npages; > + > + npages = PAGE_ALIGN(size) >> PAGE_SHIFT; > + set_memory_rw((unsigned long)tramp, npages); > module_memfree(tramp); > } > #else > @@ -699,7 +703,7 @@ static inline void *alloc_tramp(unsigned long size) > { > return NULL; > } > -static inline void tramp_free(void *tramp) { } > +static inline void tramp_free(void *tramp, int size) { } > #endif > > /* Defined as markers to the end of the ftrace default trampolines */ > @@ -771,7 +775,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size) > /* Copy ftrace_caller onto the trampoline memory */ > ret = probe_kernel_read(trampoline, (void *)start_offset, size); > if (WARN_ON(ret < 0)) { > - tramp_free(trampoline); > + tramp_free(trampoline, *tramp_size); > return 0; > } > > @@ -797,7 +801,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size) > > /* Are we pointing to the reference? */ > if (WARN_ON(memcmp(op_ptr.op, op_ref, 3) != 0)) { > - tramp_free(trampoline); > + tramp_free(trampoline, *tramp_size); > return 0; > } > > @@ -943,7 +947,7 @@ void arch_ftrace_trampoline_free(struct ftrace_ops *ops) > if (!ops || !(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP)) > return; > > - tramp_free((void *)ops->trampoline); > + tramp_free((void *)ops->trampoline, ops->trampoline_size); > ops->trampoline = 0; > } > -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-25 9:09 ` [PATCH] " Masami Hiramatsu @ 2017-05-25 10:34 ` Masami Hiramatsu 2017-05-25 15:18 ` Steven Rostedt 0 siblings, 1 reply; 27+ messages in thread From: Masami Hiramatsu @ 2017-05-25 10:34 UTC (permalink / raw) To: Masami Hiramatsu Cc: Steven Rostedt, Thomas Gleixner, Kees Cook, LKML, x86, Luis R. Rodriguez, Peter Zijlstra On Thu, 25 May 2017 18:09:10 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > On Wed, 24 May 2017 18:25:47 -0400 > Steven Rostedt <rostedt@goodmis.org> wrote: > > > On Wed, 24 May 2017 21:13:27 +0200 (CEST) > > Thomas Gleixner <tglx@linutronix.de> wrote: > > > > > > > > Oops: 0003 [#1] SMP > > > > Modules linked in: > > > > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42 > > > > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 > > > > 02/22/2014 task: ffff8802153a8000 task.stack: ffffc90000c74000 > > > > RIP: 0010:new_slab+0x1e8/0x2b4 > > > > RSP: 0000:ffffc90000c77b28 EFLAGS: 00010282 > > > > RAX: 0000000040040000 RBX: ffff880216003f00 RCX: ffff880214f5c058 > > > > RDX: 0000000000000000 RSI: ffff880214f5c000 RDI: ffff880216003f00 > > > > RBP: ffffc90000c77b70 R08: 000000000000002a R09: 0000000000000000 > > > > R10: 00000000000201e2 R11: 0000000000020190 R12: ffff880214f5c000 > > > > R13: 000000000000002e R14: 0000000000000001 R15: ffffea000853d700 > > > > FS: 0000000000000000(0000) GS:ffff88021eb80000(0000) > > > > knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: > > > > 0000000080050033 CR2: ffff880214f5c000 CR3: 000000000221d000 CR4: > > > > 00000000001406e0 Call Trace: > > > > ? interleave_nodes+0x29/0x40 > > > > ___slab_alloc+0x2e8/0x49e > > > > > > That does not make any sense, but I'm digging into it. > > > > The trampolines uses the module allocation, and it appears, that needs > > to become rw before freeing again. > > Oops, I also have to fix kprobes side, because it has same issue. OK, I've ensured that following command hits same BUG. grep t /proc/kallsyms | cut -f3 -d" " | grep -v .*\\..* | \ head -n 256 | while read i; do echo p $i+5 ; done > tracing/kprobe_events echo 1 > tracing/events/kprobes/enable echo 0 > tracing/events/kprobes/enable echo > tracing/kprobe_events sleep 5 I'll send a bugfix asap. Thank, -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-25 10:34 ` Masami Hiramatsu @ 2017-05-25 15:18 ` Steven Rostedt 2017-05-26 1:34 ` Masami Hiramatsu 0 siblings, 1 reply; 27+ messages in thread From: Steven Rostedt @ 2017-05-25 15:18 UTC (permalink / raw) To: Masami Hiramatsu Cc: Thomas Gleixner, Kees Cook, LKML, x86, Luis R. Rodriguez, Peter Zijlstra On Thu, 25 May 2017 19:34:43 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > OK, I've ensured that following command hits same BUG. > > grep t /proc/kallsyms | cut -f3 -d" " | grep -v .*\\..* | \ > head -n 256 | while read i; do echo p $i+5 ; done > > tracing/kprobe_events > > echo 1 > tracing/events/kprobes/enable > echo 0 > tracing/events/kprobes/enable > echo > tracing/kprobe_events > sleep 5 Hi Masami, Can you add a selftest that checks this too? Thanks! -- Steve > > I'll send a bugfix asap. > > Thank, > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX 2017-05-25 15:18 ` Steven Rostedt @ 2017-05-26 1:34 ` Masami Hiramatsu 0 siblings, 0 replies; 27+ messages in thread From: Masami Hiramatsu @ 2017-05-26 1:34 UTC (permalink / raw) To: Steven Rostedt Cc: Thomas Gleixner, Kees Cook, LKML, x86, Luis R. Rodriguez, Peter Zijlstra On Thu, 25 May 2017 11:18:02 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Thu, 25 May 2017 19:34:43 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > > OK, I've ensured that following command hits same BUG. > > > > grep t /proc/kallsyms | cut -f3 -d" " | grep -v .*\\..* | \ > > head -n 256 | while read i; do echo p $i+5 ; done > > > tracing/kprobe_events > > > > echo 1 > tracing/events/kprobes/enable > > echo 0 > tracing/events/kprobes/enable > > echo > tracing/kprobe_events > > sleep 5 > > Hi Masami, > > Can you add a selftest that checks this too? Yes, however, it should be included after my fix is accepted, or it can cause a kernel panic... Thanks, -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2017-05-26 13:58 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-24 13:47 [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX Thomas Gleixner 2017-05-24 14:33 ` Masami Hiramatsu 2017-05-24 15:04 ` Steven Rostedt 2017-05-24 17:47 ` Steven Rostedt 2017-05-24 18:16 ` Luis R. Rodriguez 2017-05-24 18:53 ` Thomas Gleixner 2017-05-24 19:34 ` Luis R. Rodriguez 2017-05-24 19:13 ` Thomas Gleixner 2017-05-24 22:25 ` Steven Rostedt 2017-05-24 23:18 ` Luis R. Rodriguez 2017-05-25 6:25 ` Thomas Gleixner 2017-05-25 8:57 ` [PATCH V2] " Thomas Gleixner 2017-05-25 15:15 ` Steven Rostedt 2017-05-25 17:46 ` Luis R. Rodriguez 2017-05-25 19:51 ` Kees Cook 2017-05-26 7:03 ` Thomas Gleixner 2017-05-26 9:27 ` Heiko Carstens 2017-05-26 9:56 ` Thomas Gleixner 2017-05-26 11:40 ` Michael Ellerman 2017-05-26 9:49 ` Masami Hiramatsu 2017-05-26 13:37 ` Steven Rostedt 2017-05-26 13:50 ` Thomas Gleixner 2017-05-26 13:58 ` Steven Rostedt 2017-05-25 9:09 ` [PATCH] " Masami Hiramatsu 2017-05-25 10:34 ` Masami Hiramatsu 2017-05-25 15:18 ` Steven Rostedt 2017-05-26 1:34 ` Masami Hiramatsu
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.