linux-kernel - Re: [PATCH 3/3] srcu: Fix broken node geometry after early ssp init

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: <20210402100221.GA134532@lothringen>
Date:   Fri, 2 Apr 2021 12:02:21 +0200
From:   Frederic Weisbecker <frederic@...nel.org>
To:     "Paul E. McKenney" <paulmck@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Uladzislau Rezki <urezki@...il.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Neeraj Upadhyay <neeraju@...eaurora.org>,
        Josh Triplett <josh@...htriplett.org>,
        Joel Fernandes <joel@...lfernandes.org>
Subject: Re: [PATCH 3/3] srcu: Fix broken node geometry after early ssp init

On Thu, Apr 01, 2021 at 06:12:41PM -0700, Paul E. McKenney wrote:
> On Fri, Apr 02, 2021 at 01:47:04AM +0200, Frederic Weisbecker wrote:
gg> >  void __init srcu_init(void)
> >  {
> > -	struct srcu_struct *ssp;
> > +	static struct srcu_data __initdata old_sdp;
> > +	static struct srcu_node __initdata old_mynode;
> > +	struct srcu_struct *ssp, *tmp;
> >  
> >  	srcu_init_done = true;
> > -	while (!list_empty(&srcu_boot_list)) {
> > -		ssp = list_first_entry(&srcu_boot_list, struct srcu_struct,
> > -				      work.work.entry);
> > +	/*
> > +	 * ssp's that got initialized before rcu_init_geometry() have a stale
> > +	 * node hierarchy. Rebuild their node trees and propagate states from
> > +	 * pruned leaf nodes.
> > +	 */
> > +	list_for_each_entry_safe(ssp, tmp, &srcu_early_init_list, early_init) {
> > +		struct srcu_data *sdp = this_cpu_ptr(ssp->sda);
> > +
> > +		list_del(&ssp->early_init);
> > +		old_sdp = *sdp;
> > +		old_mynode = *sdp->mynode;
> > +		init_srcu_struct_nodes(ssp);
> > +		restore_srcu_sdp(sdp, &old_sdp, &old_mynode);
> 
> Why not just pull all of the pre-initialization callbacks onto a local
> list (re-initialization the rcu_segcblist structures if necessary),
> then iterate through that list re-invoking call_srcu() on each?
> There shouldn't be that many pre-initialization call_srcu() invocations,
> and if someday there are, it would not be hard to make __call_srcu()
> take lists of callbacks and a count instead of a single callback, correct?
> 
> Or am I once again missing something basic?

So that would work for callbacks based started grace periods but not for the
others. So if anybody calls start_poll_synchronize_rcu() early, simply requeuing
the callbacks won't help. Even worse, a blind reinitialization of the ssp would
lose the started grace period and a future poll_state_synchronize_rcu() might
never succeed.

Arguably that's a quite a corner case and I don't expect anyone to call
start_poll_synchronize_srcu() so early but who knows. An alternative is to
forbid it and warn if used before srcu is initialized.

Thanks.