Re: [PATCH v2 01/33] ftrace: Unpoison ftrace_regs in ftrace_ops_list_func()

From: Ilya Leoshkevich <iii@linux.ibm.com>
To: Steven Rostedt <rostedt@goodmis.org>,
	Alexander Potapenko <glider@google.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Christoph Lameter <cl@linux.com>,
	David Rientjes <rientjes@google.com>,
	Heiko Carstens <hca@linux.ibm.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Marco Elver <elver@google.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Pekka Enberg <penberg@kernel.org>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Christian Borntraeger <borntraeger@linux.ibm.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Hyeonggon Yoo <42.hyeyoo@gmail.com>,
	kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, linux-s390@vger.kernel.org,
	linux-trace-kernel@vger.kernel.org,
	Mark Rutland <mark.rutland@arm.com>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	Sven Schnelle <svens@linux.ibm.com>
Subject: Re: [PATCH v2 01/33] ftrace: Unpoison ftrace_regs in ftrace_ops_list_func()
Date: Tue, 12 Dec 2023 03:05:25 +0100	[thread overview]
Message-ID: <3897a38ef97742f7f51fb4c84c5ddeb4e36dae79.camel@linux.ibm.com> (raw)
In-Reply-To: <20231208093133.62aae274@gandalf.local.home>

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