| Message ID | 20231121220155.1217090-2-iii@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | kmsan: Enable on s390 | expand |
On Tue, 21 Nov 2023 23:00:55 +0100 Ilya Leoshkevich <iii@linux.ibm.com> wrote: > Architectures use assembly code to initialize ftrace_regs and call > ftrace_ops_list_func(). Therefore, from the KMSAN's point of view, > ftrace_regs is poisoned on ftrace_ops_list_func entry(). This causes > KMSAN warnings when running the ftrace testsuite. > > Fix by trusting the architecture-specific assembly code and always > unpoisoning ftrace_regs in ftrace_ops_list_func. You must be very trusting to trust architecture-specific assembly code ;-) Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org> -- Steve > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > kernel/trace/ftrace.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 8de8bec5f366..dfb8b26966aa 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -7399,6 +7399,7 @@ __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip, > void arch_ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct ftrace_regs *fregs) > { > + kmsan_unpoison_memory(fregs, sizeof(*fregs)); > __ftrace_ops_list_func(ip, parent_ip, NULL, fregs); > } > #else
On Tue, Nov 21, 2023 at 11:02 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > Architectures use assembly code to initialize ftrace_regs and call > ftrace_ops_list_func(). Therefore, from the KMSAN's point of view, > ftrace_regs is poisoned on ftrace_ops_list_func entry(). This causes > KMSAN warnings when running the ftrace testsuite. I couldn't reproduce these warnings on x86, hope you really need this change on s390 :) > Fix by trusting the architecture-specific assembly code and always > unpoisoning ftrace_regs in ftrace_ops_list_func. > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> Reviewed-by: Alexander Potapenko <glider@google.com>
On Fri, 8 Dec 2023 15:16:10 +0100 Alexander Potapenko <glider@google.com> wrote: > On Tue, Nov 21, 2023 at 11:02 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > > > Architectures use assembly code to initialize ftrace_regs and call > > ftrace_ops_list_func(). Therefore, from the KMSAN's point of view, > > ftrace_regs is poisoned on ftrace_ops_list_func entry(). This causes > > KMSAN warnings when running the ftrace testsuite. > > I couldn't reproduce these warnings on x86, hope you really need this > change on s390 :) On x86, ftrace_regs sits on the stack. And IIUC, s390 doesn't have the same concept of a "stack" as other architectures. Perhaps that's the reason s390 needs this? -- Steve
On Fri, 2023-12-08 at 09:31 -0500, Steven Rostedt wrote: > On Fri, 8 Dec 2023 15:16:10 +0100 > Alexander Potapenko <glider@google.com> wrote: > > > On Tue, Nov 21, 2023 at 11:02 PM Ilya Leoshkevich > > <iii@linux.ibm.com> wrote: > > > > > > Architectures use assembly code to initialize ftrace_regs and > > > call > > > ftrace_ops_list_func(). Therefore, from the KMSAN's point of > > > view, > > > ftrace_regs is poisoned on ftrace_ops_list_func entry(). This > > > causes > > > KMSAN warnings when running the ftrace testsuite. > > > > I couldn't reproduce these warnings on x86, hope you really need > > this > > change on s390 :) I just double-checked, and it's still needed. Without it, I get: [ 4.140184] ===================================================== [ 4.140416] BUG: KMSAN: uninit-value in arch_ftrace_ops_list_func+0x8e6/0x14b0 [ 4.140484] arch_ftrace_ops_list_func+0x8e6/0x14b0 [ 4.140546] ftrace_graph_caller+0x0/0x34 [ 4.140614] read_tod_clock+0x6/0x1e0 [ 4.140671] ktime_get+0x3a4/0x670 [ 4.140727] clockevents_program_event+0x1c8/0xb10 [ 4.140785] tick_program_event+0x11e/0x230 [ 4.140842] hrtimer_interrupt+0x118a/0x1d10 [ 4.140898] do_IRQ+0x108/0x150 [ 4.140959] do_irq_async+0xfc/0x270 [ 4.141021] do_ext_irq+0x98/0x120 [ 4.141080] ext_int_handler+0xc4/0xf0 [ 4.141141] _raw_spin_unlock_irqrestore+0xfa/0x190 [ 4.141207] _raw_spin_unlock_irqrestore+0xf6/0x190 [ 4.141271] s390_kernel_write+0x218/0x250 [ 4.141328] ftrace_make_call+0x362/0x4a0 [ 4.141386] __ftrace_replace_code+0xb44/0xbd0 [ 4.141442] ftrace_replace_code+0x1d8/0x440 [ 4.141497] ftrace_modify_all_code+0xfe/0x510 [ 4.141555] ftrace_startup+0x4f0/0xcf0 [ 4.141609] register_ftrace_function+0x1316/0x1440 [ 4.141670] function_trace_init+0x2c0/0x3d0 [ 4.141732] tracer_init+0x282/0x370 [ 4.141789] trace_selftest_startup_function+0x104/0x19d0 [ 4.141857] run_tracer_selftest+0x7c8/0xab0 [ 4.141918] init_trace_selftests+0x200/0x820 [ 4.141977] do_one_initcall+0x35e/0x1090 [ 4.142032] do_initcall_level+0x276/0x660 [ 4.142095] do_initcalls+0x16a/0x2d0 [ 4.142153] kernel_init_freeable+0x632/0x960 [ 4.142216] kernel_init+0x36/0x1810 [ 4.142277] __ret_from_fork+0xc0/0x180 [ 4.142333] ret_from_fork+0xa/0x30 [ 4.142431] Local variable agg.tmp.i.i created at: 02:06:55 [30/1836] [ 4.142476] timekeeping_advance+0x79a/0x2870 [ 4.142394] [ 4.142431] Local variable agg.tmp.i.i created at: [ 4.142476] timekeeping_advance+0x79a/0x2870 [ 4.142534] [ 4.142573] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 6.7.0-rc4-g7657d31dc545 #4 [ 4.142638] Hardware name: IBM 3931 A01 704 (KVM/Linux) [ 4.142686] ===================================================== [ 4.142734] Kernel panic - not syncing: kmsan.panic set ... [ 4.142734] ===================================================== > On x86, ftrace_regs sits on the stack. And IIUC, s390 doesn't have > the same > concept of a "stack" as other architectures. Perhaps that's the > reason s390 > needs this? It's not that different on s390x. There is indeed no architecture- mandated stack pointer and no push/pop, but other than that it's fairly normal. Linux uses %r15 as a stack pointer. On s390x ftrace_regs is allocated on stack by mcount.S. From what I can see, Intel's ftrace_64.S does the same thing, so I don't immediately see why uninit-value is not detected on Intel, even though I think it should. > -- Steve
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 8de8bec5f366..dfb8b26966aa 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -7399,6 +7399,7 @@ __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip, void arch_ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op, struct ftrace_regs *fregs) { + kmsan_unpoison_memory(fregs, sizeof(*fregs)); __ftrace_ops_list_func(ip, parent_ip, NULL, fregs); } #else
Architectures use assembly code to initialize ftrace_regs and call ftrace_ops_list_func(). Therefore, from the KMSAN's point of view, ftrace_regs is poisoned on ftrace_ops_list_func entry(). This causes KMSAN warnings when running the ftrace testsuite. Fix by trusting the architecture-specific assembly code and always unpoisoning ftrace_regs in ftrace_ops_list_func. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- kernel/trace/ftrace.c | 1 + 1 file changed, 1 insertion(+)