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: <55975a55-302f-4c45-bfcc-192a8a1242e9@huaweicloud.com>
Date: Fri, 20 Sep 2024 09:41:15 +0200
From: Jonas Oberhauser <jonas.oberhauser@...weicloud.com>
To: Boqun Feng <boqun.feng@...il.com>, linux-kernel@...r.kernel.org,
 rcu@...r.kernel.org, linux-mm@...ck.org, lkmm@...r.kernel.org
Cc: "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>,
 Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 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/17/2024 um 4:33 PM schrieb Boqun Feng:
> +static inline void *__hazptr_tryprotect(hazptr_t *hzp,
> +					void *const *p,
> +					unsigned long head_offset)
> +{
> +	void *ptr;
> +	struct callback_head *head;
> +
> +	ptr = READ_ONCE(*p);
> +
> +	if (ptr == NULL)
> +		return NULL;
> +
> +	head = (struct callback_head *)(ptr + head_offset);
> +
> +	WRITE_ONCE(*hzp, head);
> +	smp_mb();
> +
> +	ptr = READ_ONCE(*p); // read again
> +
> +	if (ptr + head_offset != head) { // pointer changed
> +		WRITE_ONCE(*hzp, NULL);  // reset hazard pointer
> +		return NULL;
> +	} else
> +		return ptr;
> +}

There is a subtle potential for ABA issues here.

If the compiler replaces 'return ptr;' with 'return head - 
head_offset;', then you do not have an address dependency from the 
second read.

In this case, in ABA, the first read can read from a stale store, then 
the second read reads the same value from a newer store but only 
establishes control-dependency based synchronization with that store; 
any reads from *ptr could be speculatively executed before doing the 
second ptr = READ_ONCE(*p).

Therefore you could read the object state before it is properly 
reinitialized by the second store.

I'm not sure what the most efficient fix is or if you just want to 
gamble that "the compiler will never do that".
I guess either READ_ONCE(ptr) or a compiler barrier before return ptr 
might do it?

Have fun,
    jonas