Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Message-ID: <3897a38ef97742f7f51fb4c84c5ddeb4e36dae79.camel@linux.ibm.com>
Subject: 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>
Date:   Tue, 12 Dec 2023 03:05:25 +0100
In-Reply-To: <20231208093133.62aae274@gandalf.local.home>
References: <20231121220155.1217090-1-iii@linux.ibm.com>
         <20231121220155.1217090-2-iii@linux.ibm.com>
         <CAG_fn=WHf0t=-OJL0031D+X7cX_D25G7TG0TqROsT34QcEnqsw@mail.gmail.com>
         <20231208093133.62aae274@gandalf.local.home>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

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:
>=20
> > On Tue, Nov 21, 2023 at 11:02=E2=80=AFPM Ilya Leoshkevich
> > <iii@linux.ibm.com> wrote:
> > >=20
> > > 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.=C2=A0=20
> >=20
> > 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] =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D =20
[    4.140416] BUG: KMSAN: uninit-value in
arch_ftrace_ops_list_func+0x8e6/0x14b0           =20
[    4.140484]  arch_ftrace_ops_list_func+0x8e6/0x14b0               =20
[    4.140546]  ftrace_graph_caller+0x0/0x34                         =20
[    4.140614]  read_tod_clock+0x6/0x1e0                             =20
[    4.140671]  ktime_get+0x3a4/0x670                                =20
[    4.140727]  clockevents_program_event+0x1c8/0xb10                =20
[    4.140785]  tick_program_event+0x11e/0x230                       =20
[    4.140842]  hrtimer_interrupt+0x118a/0x1d10                      =20
[    4.140898]  do_IRQ+0x108/0x150                                   =20
[    4.140959]  do_irq_async+0xfc/0x270                              =20
[    4.141021]  do_ext_irq+0x98/0x120                                =20
[    4.141080]  ext_int_handler+0xc4/0xf0                            =20
[    4.141141]  _raw_spin_unlock_irqrestore+0xfa/0x190               =20
[    4.141207]  _raw_spin_unlock_irqrestore+0xf6/0x190               =20
[    4.141271]  s390_kernel_write+0x218/0x250                        =20
[    4.141328]  ftrace_make_call+0x362/0x4a0                         =20
[    4.141386]  __ftrace_replace_code+0xb44/0xbd0                    =20
[    4.141442]  ftrace_replace_code+0x1d8/0x440                      =20
[    4.141497]  ftrace_modify_all_code+0xfe/0x510                    =20
[    4.141555]  ftrace_startup+0x4f0/0xcf0                           =20
[    4.141609]  register_ftrace_function+0x1316/0x1440               =20
[    4.141670]  function_trace_init+0x2c0/0x3d0                      =20
[    4.141732]  tracer_init+0x282/0x370                              =20
[    4.141789]  trace_selftest_startup_function+0x104/0x19d0         =20
[    4.141857]  run_tracer_selftest+0x7c8/0xab0                      =20
[    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:                =20
02:06:55 [30/1836]
[    4.142476]  timekeeping_advance+0x79a/0x2870
[    4.142394]=20
[    4.142431] Local variable agg.tmp.i.i created at:
[    4.142476]  timekeeping_advance+0x79a/0x2870
[    4.142534]=20
[    4.142573] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W     =20
6.7.0-rc4-g7657d31dc545 #4
[    4.142638] Hardware name: IBM 3931 A01 704 (KVM/Linux)
[    4.142686] =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D
[    4.142734] Kernel panic - not syncing: kmsan.panic set ...
[    4.142734] =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D

> 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