linux-kernel - Re: [PATCH RFC 3/5] sched/cpufreq: Fix incorrect RCU API usage

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: <20190221171311.GA118415@google.com>
Date:   Thu, 21 Feb 2019 12:13:11 -0500
From:   Joel Fernandes <joel@...lfernandes.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     "Paul E. McKenney" <paulmck@...ux.ibm.com>,
        linux-kernel@...r.kernel.org, Alexei Starovoitov <ast@...nel.org>,
        Christian Brauner <christian@...uner.io>,
        Daniel Borkmann <daniel@...earbox.net>,
        David Ahern <dsahern@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Ido Schimmel <idosch@...lanox.com>,
        Ingo Molnar <mingo@...hat.com>,
        "moderated list:INTEL ETHERNET DRIVERS" 
        <intel-wired-lan@...ts.osuosl.org>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        John Fastabend <john.fastabend@...il.com>,
        Josh Triplett <josh@...htriplett.org>, keescook@...omium.org,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Martin KaFai Lau <kafai@...com>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        netdev@...r.kernel.org, rcu@...r.kernel.org,
        Song Liu <songliubraving@...com>,
        Steven Rostedt <rostedt@...dmis.org>,
        xdp-newbies@...r.kernel.org, Yonghong Song <yhs@...com>
Subject: Re: [PATCH RFC 3/5] sched/cpufreq: Fix incorrect RCU API usage

On Thu, Feb 21, 2019 at 05:11:44PM +0100, Peter Zijlstra wrote:
> On Thu, Feb 21, 2019 at 07:52:18AM -0800, Paul E. McKenney wrote:
> > On Thu, Feb 21, 2019 at 04:31:17PM +0100, Peter Zijlstra wrote:
> > > On Thu, Feb 21, 2019 at 10:21:39AM -0500, Joel Fernandes wrote:
> > > > On Thu, Feb 21, 2019 at 10:18:05AM +0100, Peter Zijlstra wrote:
> > > > > On Thu, Feb 21, 2019 at 12:49:40AM -0500, Joel Fernandes (Google) wrote:
> > > > > > @@ -34,8 +34,12 @@ void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
> > > > > >  	if (WARN_ON(!data || !func))
> > > > > >  		return;
> > > > > >  
> > > > > > -	if (WARN_ON(per_cpu(cpufreq_update_util_data, cpu)))
> > > > > > +	rcu_read_lock();
> > > > > > +	if (WARN_ON(rcu_dereference(per_cpu(cpufreq_update_util_data, cpu)))) {
> > > > > > +		rcu_read_unlock();
> > > > > >  		return;
> > > > > > +	}
> > > > > > +	rcu_read_unlock();
> > > > > >  
> > > > > >  	data->func = func;
> > > > > >  	rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
> 
> > For whatever it is worth, in that case it could use rcu_access_pointer().
> > And this primitive does not do the lockdep check for being within an RCU
> > read-side critical section.  As Peter says, if there is no dereferencing,
> > there can be no use-after-free bug, so the RCU read-side critical is
> > not needed.
> 
> On top of that, I suspect this is under the write-side lock (we're doing
> assignment after all).

Yes it is under a policy->rwsem, just confirmed that.

And indeed rcu_read_lock() is not needed here in this patch, thanks for
pointing that out. Sometimes the word "dereference" plays some visual tricks
and in this case tempted me to add an RCU reader section ;-) Assuming you
guys are in agreement with usage of rcu_access_pointer() here to keep sparse
happy, I'll rework the patch accordingly and resubmit with that.

thanks!

 - Joel