linux-kernel - Re: [PATCH] rcu,ftrace: Fix ftrace recursion

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200929105416.757c47f0@gandalf.local.home>
Date:   Tue, 29 Sep 2020 10:54:16 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     "Paul E. McKenney" <paulmck@...nel.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>, kim.phillips@....com
Subject: Re: [PATCH] rcu,ftrace: Fix ftrace recursion

On Tue, 29 Sep 2020 07:41:06 -0700
"Paul E. McKenney" <paulmck@...nel.org> wrote:

> On Tue, Sep 29, 2020 at 10:36:20AM -0400, Steven Rostedt wrote:
> > On Tue, 29 Sep 2020 13:33:40 +0200
> > Peter Zijlstra <peterz@...radead.org> wrote:
> >   
> > > Kim reported that perf-ftrace made his box unhappy. It turns out that
> > > commit:
> > > 
> > >   ff5c4f5cad33 ("rcu/tree: Mark the idle relevant functions noinstr")
> > > 
> > > removed one too many notrace. Probably due to there not being a helpful
> > > comment.
> > > 
> > > Reinstate the notrace and add a comment to avoid loosing it again.
> > > 
> > > Fixes: ff5c4f5cad33 ("rcu/tree: Mark the idle relevant functions noinstr")
> > > Reported-by: Kim Phillips <kim.phillips@....com>
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> > > ---
> > >  kernel/rcu/tree.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > index ee5e595501e8..33020d84ec6b 100644
> > > --- a/kernel/rcu/tree.c
> > > +++ b/kernel/rcu/tree.c
> > > @@ -1098,8 +1098,11 @@ noinstr bool __rcu_is_watching(void)
> > >   * CPU can safely enter RCU read-side critical sections.  In other words,
> > >   * if the current CPU is not in its idle loop or is in an interrupt or
> > >   * NMI handler, return true.
> > > + *
> > > + * Must be notrace because __ftrace_ops_list_func() / ftrace_ops_assist_func()
> > > + * will call this (for every function) outside of recursion protection.
> > >   */
> > > -bool rcu_is_watching(void)
> > > +notrace bool rcu_is_watching(void)
> > >  {
> > >  	bool ret;
> > >    
> > 
> > I think the patch I suggested is more suitable.  
> 
> OK, I will let you guys fight it out.  ;-)
> 

Well, I think we should actually apply both, but the comment needs to be
updated, as it will no longer be outside recursion. And the comment is
wrong now as well, as its only outside recursion protection for the
assist_func(). 

But it does prevent it from being always called for perf.

 * Make notrace because it can be called by the internal functions of
 * ftrace, and making this notrace removes unnecessary recursion calls.


-- Steve