linux-kernel - Re: [RFC PATCH 1/1] hpref: Hazard Pointers with Reference Counter

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: <07c9285f-44a1-486a-8390-0c63cefae35a@huaweicloud.com>
Date: Wed, 25 Sep 2024 12:06:24 +0200
From: Jonas Oberhauser <jonas.oberhauser@...weicloud.com>
To: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 Boqun Feng <boqun.feng@...il.com>, "Paul E. McKenney" <paulmck@...nel.org>
Cc: linux-kernel@...r.kernel.org, Will Deacon <will@...nel.org>,
 Peter Zijlstra <peterz@...radead.org>, Alan Stern
 <stern@...land.harvard.edu>, John Stultz <jstultz@...gle.com>,
 Linus Torvalds <torvalds@...ux-foundation.org>,
 Frederic Weisbecker <frederic@...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>,
 Ingo Molnar <mingo@...hat.com>, Waiman Long <longman@...hat.com>,
 Mark Rutland <mark.rutland@....com>, Thomas Gleixner <tglx@...utronix.de>,
 Vlastimil Babka <vbabka@...e.cz>, maged.michael@...il.com,
 Mateusz Guzik <mjguzik@...il.com>, rcu@...r.kernel.org, linux-mm@...ck.org,
 lkmm@...ts.linux.dev
Subject: Re: [RFC PATCH 1/1] hpref: Hazard Pointers with Reference Counter

Am 9/25/2024 um 8:35 AM schrieb Mathieu Desnoyers:
> On 2024-09-25 07:57, Jonas Oberhauser wrote:
>> Hi Mathieu,

>> I haven't read your code in detail but it seems to me you have an ABA 
>> bug: as I explained elsewhere, you could read the same pointer after 
>> ABA but you don't synchronize with the newer store that gave you 
>> node2, leaving you to speculatively read stale values through *ctx->hp.
>> (I am assuming here that ctx->hp is essentially an out parameter used 
>> to let the caller know which node got protected).
> 
> The following change should fix it:
> 
>       cmm_barrier();
> -    node2 = uatomic_load(node_p, CMM_RELAXED);    /* Load A */
> +    node2 = rcu_dereference(*node_p);    /* Load A */
> 

I don't think this fixes it, because IIRC rcu_dereference relies on the 
address dependency (which we don't have here) to provide ordering.

I would recommend either:

-    ctx->hp = node;
+    ctx->hp = node2;

which fixes the problem under the perhaps too weak assumption that the 
compiler doesn't use its knowledge that node==node2 to just undo this 
fix, or more strictly,

+    ctx->hp = READ_ONCE(node2);

which I believe makes sure that the value of node2 is used.

Alternatively you could always use an acquire load.

Best wishes,

   jonas