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 <[email protected]>
---
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
--
2.41.0
On Tue, 21 Nov 2023 23:00:55 +0100
Ilya Leoshkevich <[email protected]> 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) <[email protected]>
-- Steve
>
> Signed-off-by: Ilya Leoshkevich <[email protected]>
> ---
> 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 <[email protected]> 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 <[email protected]>
Reviewed-by: Alexander Potapenko <[email protected]>
On Fri, 8 Dec 2023 15:16:10 +0100
Alexander Potapenko <[email protected]> wrote:
> On Tue, Nov 21, 2023 at 11:02 PM Ilya Leoshkevich <[email protected]> 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 <[email protected]> wrote:
>
> > On Tue, Nov 21, 2023 at 11:02 PM Ilya Leoshkevich
> > <[email protected]> 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