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: <20250310015507.GA3993297@joelnvbox>
Date: Sun, 9 Mar 2025 21:55:07 -0400
From: Joel Fernandes <joelagnelf@...dia.com>
To: "Uladzislau Rezki (Sony)" <urezki@...il.com>
Cc: "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()

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)



thanks,

  - Joel