linux-kernel - Re: [PATCH 1/2] docs: rcu: Add cautionary note on plain-accesses to requirements

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: <DFBE4F45-BF8B-4267-93C7-B085A88BE356@gmail.com>
Date:   Sat, 5 Aug 2023 00:33:03 +0800
From:   Alan Huang <mmpgouride@...il.com>
To:     "Paul E. McKenney" <paulmck@...nel.org>
Cc:     Joel Fernandes <joel@...lfernandes.org>,
        linux-kernel@...r.kernel.org, rcu@...r.kernel.org,
        Will Deacon <will@...nel.org>,
        Frederic Weisbecker <frederic@...nel.org>,
        Neeraj Upadhyay <quic_neeraju@...cinc.com>,
        Josh Triplett <josh@...htriplett.org>,
        Boqun Feng <boqun.feng@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Zqiang <qiang.zhang1211@...il.com>,
        Jonathan Corbet <corbet@....net>
Subject: Re: [PATCH 1/2] docs: rcu: Add cautionary note on plain-accesses to
 requirements


>> Yes, a write-write data race where the value is the same is also fine.
>> 
>> But they are still data race, if the compiler is within its right to do anything it likes (due to data race),
>> we still need WRITE_ONCE() in these cases, though it’s semantically safe.
>> 
>> IIUC, even with _ONCE(), the compiler is within its right do anything according to the standard (at least before the upcoming C23), because the standard doesn’t consider a volatile access to be atomic.
> 
> Volatile accesses are not specified very well in the standard.  However,
> as a practical matter, compilers that wish to be able to device drivers
> (whether in kernels or userspace applications) must compile those volatile
> accesses in such a way to allow reliable device drivers to be written.
> 
>> However, the kernel consider the volatile access to be atomic, right?
> 
> The compiler must therefore act as if a volatile access to an aligned
> machine-word size location is atomic.  To see this, consider accesses
> to memory that is shared by a device driver and that device's firmware,
> both of which are written in either C or C++.

I learned these things a few months ago. But still thank you!

The real problem is that there may be a data race at line 5, so Joel is correct that the compiler
can cache the value loaded from line 5 according to the standard given that the standard says that
a data race result in undefined behavior, so the compiler might be allowed to do anything. But from the
perspective of the kernel, line 5 is likely a diagnostic read, so it’s fine without READ_ONCE() and the
compiler is not allowed to cache the value.

This situation is like the volatile access.

Am I missing something?

> 
> Does that help?
> 
> Thanx, Paul
> 
>> BTW, line 5 in the example is likely to be optimized away. And yes, the compiler can cache the value loaded from line 5 from the perspective of undefined behavior, even if I believe it would be a compiler bug from the perspective of kernel.
>> 
>>> result will not change the semantics of the program. But otherwise,
>>> that's bad.
>>> 
>>> [1] https://lwn.net/Articles/793253/#Store%20Tearing
>>> 
>>> thanks,
>>> 
>>> - Joel
>>> 
>>> 
>>>> 
>>>>> 
>>>>> Thanks.
>>>>> 
>>>>> 
>>>>> 
>>>>>> 
>>>>>>> +plain accesses of a memory location with rcu_dereference() of the same memory
>>>>>>> +location, in code involved in a data race.
>>>>>>> +
>>>>>>> In short, updaters use rcu_assign_pointer() and readers use
>>>>>>> rcu_dereference(), and these two RCU API elements work together to
>>>>>>> ensure that readers have a consistent view of newly added data elements.
>>>>>>> --
>>>>>>> 2.41.0.585.gd2178a4bd4-goog