linux-kernel - Re: Litmus test for question from Al Viro

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, 2 Oct 2020 22:35:45 -0400
From:   Jon Masters <jcm@...masters.org>
To:     Alan Stern <stern@...land.harvard.edu>,
        "Paul E. McKenney" <paulmck@...nel.org>
Cc:     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,
        dlustig@...dia.com, joel@...lfernandes.org,
        viro@...iv.linux.org.uk, linux-kernel@...r.kernel.org,
        linux-arch@...r.kernel.org
Subject: Re: Litmus test for question from Al Viro

On 10/1/20 12:15 PM, Alan Stern wrote:
> On Wed, Sep 30, 2020 at 09:51:16PM -0700, Paul E. McKenney wrote:
>> Hello!
>>
>> Al Viro posted the following query:
>>
>> ------------------------------------------------------------------------
>>
>> <viro> fun question regarding barriers, if you have time for that
>> <viro>         V->A = V->B = 1;
>> <viro>
>> <viro> CPU1:
>> <viro>         to_free = NULL
>> <viro>         spin_lock(&LOCK)
>> <viro>         if (!smp_load_acquire(&V->B))
>> <viro>                 to_free = V
>> <viro>         V->A = 0
>> <viro>         spin_unlock(&LOCK)
>> <viro>         kfree(to_free)
>> <viro>
>> <viro> CPU2:
>> <viro>         to_free = V;
>> <viro>         if (READ_ONCE(V->A)) {
>> <viro>                 spin_lock(&LOCK)
>> <viro>                 if (V->A)
>> <viro>                         to_free = NULL
>> <viro>                 smp_store_release(&V->B, 0);
>> <viro>                 spin_unlock(&LOCK)
>> <viro>         }
>> <viro>         kfree(to_free);
>> <viro> 1) is it guaranteed that V will be freed exactly once and that
>> 	  no accesses to *V will happen after freeing it?
>> <viro> 2) do we need smp_store_release() there?  I.e. will anything
>> 	  break if it's replaced with plain V->B = 0?
> 
> Here are my answers to Al's questions:
> 
> 1) It is guaranteed that V will be freed exactly once.  It is not
> guaranteed that no accesses to *V will occur after it is freed, because
> the test contains a data race.  CPU1's plain "V->A = 0" write races with
> CPU2's READ_ONCE; if the plain write were replaced with
> "WRITE_ONCE(V->A, 0)" then the guarantee would hold.  Equally well,
> CPU1's smp_load_acquire could be replaced with a plain read while the
> plain write is replaced with smp_store_release.
> 
> 2) The smp_store_release in CPU2 is not needed.  Replacing it with a
> plain V->B = 0 will not break anything.

This was my interpretation also. I made the mistake of reading this 
right before trying to go to bed the other night and ended up tweeting 
at Paul that I'd regret it if he gave me scary dreams. Thought about it 
and read your write up and it is still exactly how I see it.

Jon.

-- 
Computer Architect