linux-kernel - Re: [PATCH memory-model 2/4] tools/memory-model: Add example for heuristic lockless reads

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]

Date:   Fri, 23 Jul 2021 10:30:10 -0700
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Alan Stern <stern@...land.harvard.edu>
Cc:     linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
        kernel-team@...com, mingo@...nel.org, parri.andrea@...il.com,
        will@...nel.org, peterz@...radead.org, boqun.feng@...il.com,
        npiggin@...il.com, dhowells@...hat.com, j.alglave@....ac.uk,
        luc.maranget@...ia.fr, akiyks@...il.com,
        Manfred Spraul <manfred@...orfullife.com>
Subject: Re: [PATCH memory-model 2/4] tools/memory-model: Add example for
 heuristic lockless reads

On Fri, Jul 23, 2021 at 12:59:47PM -0400, Alan Stern wrote:
> On Fri, Jul 23, 2021 at 09:24:31AM -0700, Paul E. McKenney wrote:
> > On Thu, Jul 22, 2021 at 10:08:46PM -0400, Alan Stern wrote:
> 
> > > > +	void do_something_locked(struct foo *fp)
> > > > +	{
> > > > +		bool gf = true;
> > > > +
> > > > +		/* IMPORTANT: Heuristic plus spin_lock()! */
> > > > +		if (!data_race(global_flag)) {
> > > > +			spin_lock(&fp->f_lock);
> > > > +			if (!smp_load_acquire(&global_flag)) {
> 
> > > > +	void begin_global(void)
> > > > +	{
> > > > +		int i;
> > > > +
> > > > +		spin_lock(&global_lock);
> > > > +		WRITE_ONCE(global_flag, true);
> > > 
> > > Why does this need to be WRITE_ONCE?  It still races with the first read 
> > > of global_flag above.
> > 
> > But also with the smp_load_acquire() of global_flag, right?
> 
> What I'm curious about is why, given these two races, you notate one of 
> them by changing a normal write to WRITE_ONCE and you notate the other 
> by changing a normal read to a data_race() read.  Why not handle them 
> both the same way?

Because the code can tolerate the first read returning complete nonsense,
but needs the value from the second read to be exact at that point in
time.  (If the value changes immediately after being read, the fact that
->f_lock is held prevents begin_global() from completing.)

							Thanx, Paul