linux-kernel - Re: [PATCH v4 3/3] rcu: Use _full() API to debug synchronize

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] [day] [month] [year] [list]

Message-ID: <Z9Au3RWVt9ERrea6@pc636>
Date: Tue, 11 Mar 2025 13:38:53 +0100
From: Uladzislau Rezki <urezki@...il.com>
To: Joel Fernandes <joelagnelf@...dia.com>
Cc: "Uladzislau Rezki (Sony)" <urezki@...il.com>,
	"Paul E . McKenney" <paulmck@...nel.org>,
	Boqun Feng <boqun.feng@...il.com>, RCU <rcu@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Frederic Weisbecker <frederic@...nel.org>,
	Cheung Wall <zzqq0103.hey@...il.com>,
	Neeraj upadhyay <Neeraj.Upadhyay@....com>,
	Joel Fernandes <joel@...lfernandes.org>,
	Oleksiy Avramchenko <oleksiy.avramchenko@...y.com>
Subject: Re: [PATCH v4 3/3] rcu: Use _full() API to debug synchronize_rcu()

Hello, Joel!

> Hi Uladzislau,
> 
> On Thu, Feb 27, 2025 at 02:16:13PM +0100, Uladzislau Rezki (Sony) wrote:
> > Switch for using of get_state_synchronize_rcu_full() and
> > poll_state_synchronize_rcu_full() pair to debug a normal
> > synchronize_rcu() call.
> > 
> > Just using "not" full APIs to identify if a grace period is
> > passed or not might lead to a false-positive kernel splat.
> > 
> > It can happen, because get_state_synchronize_rcu() compresses
> > both normal and expedited states into one single unsigned long
> > value, so a poll_state_synchronize_rcu() can miss GP-completion
> > when synchronize_rcu()/synchronize_rcu_expedited() concurrently
> > run.
> 
> Agreed, I provided a scenario below but let me know if I missed anything.
> 
> > To address this, switch to poll_state_synchronize_rcu_full() and
> > get_state_synchronize_rcu_full() APIs, which use separate variables
> > for expedited and normal states.
> 
> Reviewed-by: Joel Fernandes <joelagnelf@...dia.com>
> 
> For completeness and just to clarify how this may happen, firstly as noted:
> rcu_poll_gp_seq_start/end() is called for both begin/end of normal and exp
> GPs thus compressing the use of the rcu_state.gp_seq_polled counter for
> both normal and exp GPs.
> 
> Then if we intersperse synchronize_rcu() with synchronize_rcu_expedited(),
> something like the following may happen.
> 
> CPU 0					CPU 1
> 
> 					synchronize_rcu_expedited()
> 					// -> rcu_poll_gp_seq_start()
> 					// This does rcu_seq_start on the
> 					// gp_seq_polled and
> 					// notes the started gp_seq_polled
> 					// (say its 5)
> synchronize_rcu()
>  -> synchronize_rcu_normal()
>   -> rs.head.func =
>      get_state_synchronize_rcu();
>      // saves the value 12
>  
> 
>  -> rcu_gp_init()
>   -> rcu_poll_gp_seq_start()
>   // rcu_seq_start does nothing
>   // but notes the pre-started
>   // gp_seq_polled (value 5)
> 
> -> rcu_gp_cleanup()
>   // -> rcu_poll_gp_seq_end()
>   // ends the gp_seq_polled since it
>   // matches prior saved gp_seq_polled (5)
>   // new gp_seq_polled is 8.
> 
>                         /*  NORMAL GP COMPLETES  */
> 
> rcu_gp_cleanup()
>  -> rcu_sr_normal_gp_cleanup()
>    -> rcu_sr_normal_complete()
>      -> poll_state_synchronize_rcu()
>        -> returns FALSE because gp_seq_polled is still 8.
>        -> Warning (false positive)
> 
> 
Thank you for clarification, this is good for archive :)

--
Uladzislau Rezki