linux-kernel - Re: [RFC PATCH 1/4] hazptr: Add initial implementation of hazard pointers

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: <55ea84c8-92fd-4268-9732-6fac3a0e78b7@huaweicloud.com>
Date: Wed, 25 Sep 2024 14:19:47 +0200
From: Jonas Oberhauser <jonas.oberhauser@...weicloud.com>
To: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 Boqun Feng <boqun.feng@...il.com>
Cc: linux-kernel@...r.kernel.org, rcu@...r.kernel.org, linux-mm@...ck.org,
 lkmm@...ts.linux.dev, "Paul E. McKenney" <paulmck@...nel.org>,
 Frederic Weisbecker <frederic@...nel.org>,
 Neeraj Upadhyay <neeraj.upadhyay@...nel.org>,
 Joel Fernandes <joel@...lfernandes.org>,
 Josh Triplett <josh@...htriplett.org>, Uladzislau Rezki <urezki@...il.com>,
 Steven Rostedt <rostedt@...dmis.org>, Lai Jiangshan
 <jiangshanlai@...il.com>, Zqiang <qiang.zhang1211@...il.com>,
 Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Will Deacon <will@...nel.org>, Waiman Long <longman@...hat.com>,
 Mark Rutland <mark.rutland@....com>, Thomas Gleixner <tglx@...utronix.de>,
 Kent Overstreet <kent.overstreet@...il.com>,
 Linus Torvalds <torvalds@...ux-foundation.org>,
 Vlastimil Babka <vbabka@...e.cz>, maged.michael@...il.com,
 Neeraj Upadhyay <neeraj.upadhyay@....com>
Subject: Re: [RFC PATCH 1/4] hazptr: Add initial implementation of hazard
 pointers

Am 9/25/2024 um 1:59 PM schrieb Mathieu Desnoyers:
> On 2024-09-25 12:45, Boqun Feng wrote:
>> On Wed, Sep 25, 2024 at 12:11:52PM +0200, Jonas Oberhauser wrote:
>>>
>>>
>>> Am 9/25/2024 um 12:02 PM schrieb Boqun Feng:
>>>> Hi Jonas,
>>>>
>>>> Of
>>>> course, if we are really worried about compilers being too "smart"
>>>
>>> Ah, I see you know me better and better...
>>>
>>>> we can always do the comparison in asm code, then compilers don't know
>>>> anything of the equality between 'ptr' and 'head - head_offset'.
>>> Yes, but then a simple compiler barrier between the comparison and 
>>> returning
>>> ptr would also do the trick, right? And maybe easier on the eyes.
>>>
>>
>> The thing about putting a compiler barrier is that it will prevent all
>> compiler reorderings, and some of the reordering may contribute to
>> better codegen. (I know in this case, we have a smp_mb(), but still
>> compilers can move unrelated code upto the second load for optimization
>> purpose). Asm comparison is cheaper in this way. But TBH, compilers
>> should provide a way to compare pointer values without using the result
>> for pointer equality proof, if "convert to unsigned long" doesn't work,
>> some other ways should work.
>>
> 
> Based on Documentation/RCU/rcu_dereference.rst :
> 
> -       Be very careful about comparing pointers obtained from
>          rcu_dereference() against non-NULL values.  As Linus Torvalds
>          explained, if the two pointers are equal, the compiler could
>          substitute the pointer you are comparing against for the pointer
>          obtained from rcu_dereference().  For example::
> 
>                  p = rcu_dereference(gp);
>                  if (p == &default_struct)
>                          do_default(p->a);
> 
>          Because the compiler now knows that the value of "p" is exactly
>          the address of the variable "default_struct", it is free to
>          transform this code into the following::
> 
>                  p = rcu_dereference(gp);
>                  if (p == &default_struct)
>                          do_default(default_struct.a);
> 
>          On ARM and Power hardware, the load from "default_struct.a"
>          can now be speculated, such that it might happen before the
>          rcu_dereference().  This could result in bugs due to misordering.
> 
> So I am not only concerned about compiler proofs here, as it appears
> that the speculation done by the CPU can also cause issues on some
> architectures.

No, this is only possible in this example because of the compiler first 
doing some other optimizations (like what I mentioned on Boqun's 
original patch).

If you can ensure that the instruction sequence corresponds to more or less

t = load p // again
// on alpha: dep fence
...

*t

then you can be sure that there is an address dependency which orders 
the access. This is guaranteed by LKMM, or if you don't trust LKMM, also 
by Arm, Power, Alpha etc.

The extra dep fence on alpha is automatically inserted if you use 
READ_ONCE as boqun did (and I assumed your uatomic_load or whatever is 
doing the same thing, but I didn't check).

Given that the hazard-pointer-protected object presumably is not a 
single static non-freeable object, but some dynamically allocated 
object, it is pretty much impossible for the compiler to guess the 
address like in the example you shared above.

Note that inside the if, the code after transform is 
do_default(default_struct.a);
which is an address that is known to the hardware before it loads from gp.

That would not be the case here (if the compiler optimization is ruled out).

jonas