[PATCH] hazptr: Fix two-phase hazptr_synchronize race with detach

[PATCH] hazptr: Fix two-phase hazptr_synchronize race with detach

[Mathieu Desnoyers]

Boqun Feng pointed out that a detach happening concurrently with
hazptr_synchronize can miss a slot. Indeed, scanning the per-CPU
slots needs to be done *before* scanning the overflow lists. Fix the
implementation accordingly.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Reported-by: Boqun Feng <boqun@kernel.org>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Boqun Feng <boqun@kernel.org>
Cc: Bradley Morgan <brads@mainlining.org>
Cc: Gary Guo <gary@garyguo.net>
Cc: <rcu@vger.kernel.org>
Cc: <lkmm@lists.linux.dev>
---
 kernel/hazptr.c | 43 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/kernel/hazptr.c b/kernel/hazptr.c
index d3d1050d92cf..ccc19843edbd 100644
--- a/kernel/hazptr.c
+++ b/kernel/hazptr.c
@@ -24,6 +24,9 @@ static DEFINE_MUTEX(hazptr_wildcard_lock);	/* Protect the wildcard flip. */
 void *hazptr_wildcard = (void *) 1UL;
 EXPORT_SYMBOL_GPL(hazptr_wildcard);
 
+/* The current overflow list phase. */
+static unsigned int hazptr_overflow_list_phase;
+
 struct hazptr_overflow_list {
 	raw_spinlock_t lock;		/* Lock protecting overflow list and list generation. */
 	struct hlist_head head;		/* Overflow list head. */
@@ -53,6 +56,12 @@ void *flip_wildcard(void *wildcard)
 	return ((unsigned long) wildcard == 1UL) ? (void *) 2UL : (void *) 1UL;
 }
 
+static
+unsigned int flip_list_phase(unsigned int phase)
+{
+	return 1 - phase;
+}
+
 static
 bool is_wildcard(void *addr)
 {
@@ -178,15 +187,12 @@ void hazptr_synchronize_cpu_slots(int cpu, void *addr, void *scan_wildcard)
 }
 
 static
-void hazptr_scan_period(void *addr, void *scan_wildcard)
+void hazptr_scan_cpu_slots_period(void *addr, void *scan_wildcard)
 {
-	unsigned int scan_idx = (unsigned long) scan_wildcard - 1;
 	int cpu;
 
 	/* Scan all CPUs slots. */
 	for_each_possible_cpu(cpu) {
-		struct hazptr_overflow_list_flip *overflow_list_flip = per_cpu_ptr(&percpu_overflow_list_flip, cpu);
-
 		/*
 		 * Scan CPU slots.
 		 * Forward progress against recurring wildcards is guaranteed
@@ -199,6 +205,17 @@ void hazptr_scan_period(void *addr, void *scan_wildcard)
 		 * to acquire that same hazard pointer value.
 		 */
 		hazptr_synchronize_cpu_slots(cpu, addr, scan_wildcard);
+	}
+}
+
+static
+void hazptr_scan_overflow_list_period(void *addr, unsigned int scan_idx)
+{
+	int cpu;
+
+	/* Scan all CPUs overflow lists. */
+	for_each_possible_cpu(cpu) {
+		struct hazptr_overflow_list_flip *overflow_list_flip = per_cpu_ptr(&percpu_overflow_list_flip, cpu);
 
 		/*
 		 * Scan backup slots in percpu overflow lists.
@@ -218,6 +235,7 @@ void hazptr_scan_period(void *addr, void *scan_wildcard)
  */
 void hazptr_synchronize(void *addr)
 {
+	unsigned int scan_list_phase;
 	void *scan_wildcard;
 
 	/*
@@ -236,10 +254,21 @@ void hazptr_synchronize(void *addr)
 	smp_mb();
 
 	guard(mutex)(&hazptr_wildcard_lock);
+
+	/* Scan per-CPU slots. */
 	scan_wildcard = flip_wildcard(hazptr_wildcard);
-	hazptr_scan_period(addr, scan_wildcard);
-	WRITE_ONCE(hazptr_wildcard, scan_wildcard);	/* Flip the current wildcard. */
-	hazptr_scan_period(addr, flip_wildcard(scan_wildcard));
+	hazptr_scan_cpu_slots_period(addr, scan_wildcard);
+	WRITE_ONCE(hazptr_wildcard, scan_wildcard);			/* Flip the current wildcard. */
+	hazptr_scan_cpu_slots_period(addr, flip_wildcard(scan_wildcard));
+
+	/*
+	 * Scan overflow lists *after* scanning per-CPU slots. See
+	 * hazptr_promote_to_backup_slot() for scan ordering requirement.
+	 */
+	scan_list_phase = flip_list_phase(hazptr_overflow_list_phase);
+	hazptr_scan_overflow_list_period(addr, scan_list_phase);
+	WRITE_ONCE(hazptr_overflow_list_phase, scan_list_phase);	/* Flip the current list phase. */
+	hazptr_scan_overflow_list_period(addr, flip_list_phase(scan_list_phase));
 }
 EXPORT_SYMBOL_GPL(hazptr_synchronize);
 
-- 
2.43.0

Re: [PATCH] hazptr: Fix two-phase hazptr_synchronize race with detach

[Bradley Morgan]

On 25 September 2026 20:49:42 BST, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>Boqun Feng pointed out that a detach happening concurrently with
>hazptr_synchronize can miss a slot. Indeed, scanning the per-CPU
>slots needs to be done *before* scanning the overflow lists. Fix the
>implementation accordingly.
>
>Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>Reported-by: Boqun Feng <boqun@kernel.org>
>Cc: Paul E. McKenney <paulmck@kernel.org>
>Cc: Boqun Feng <boqun@kernel.org>
>Cc: Bradley Morgan <brads@mainlining.org>
>Cc: Gary Guo <gary@garyguo.net>
>Cc: <rcu@vger.kernel.org>
>Cc: <lkmm@lists.linux.dev>
>---
> kernel/hazptr.c | 43 ++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 36 insertions(+), 7 deletions(-)
>
>diff --git a/kernel/hazptr.c b/kernel/hazptr.c
>index d3d1050d92cf..ccc19843edbd 100644
>--- a/kernel/hazptr.c
>+++ b/kernel/hazptr.c
>@@ -24,6 +24,9 @@ static DEFINE_MUTEX(hazptr_wildcard_lock);	/* Protect the wildcard flip. */
> void *hazptr_wildcard = (void *) 1UL;
> EXPORT_SYMBOL_GPL(hazptr_wildcard);
> 
>+/* The current overflow list phase. */
>+static unsigned int hazptr_overflow_list_phase;
>+
> struct hazptr_overflow_list {
> 	raw_spinlock_t lock;		/* Lock protecting overflow list and list generation. */
> 	struct hlist_head head;		/* Overflow list head. */
>@@ -53,6 +56,12 @@ void *flip_wildcard(void *wildcard)
> 	return ((unsigned long) wildcard == 1UL) ? (void *) 2UL : (void *) 1UL;
> }
> 
>+static
>+unsigned int flip_list_phase(unsigned int phase)
>+{
>+	return 1 - phase;
>+}
>+
> static
> bool is_wildcard(void *addr)
> {
>@@ -178,15 +187,12 @@ void hazptr_synchronize_cpu_slots(int cpu, void *addr, void *scan_wildcard)
> }
> 
> static
>-void hazptr_scan_period(void *addr, void *scan_wildcard)
>+void hazptr_scan_cpu_slots_period(void *addr, void *scan_wildcard)
> {
>-	unsigned int scan_idx = (unsigned long) scan_wildcard - 1;
> 	int cpu;
> 
> 	/* Scan all CPUs slots. */
> 	for_each_possible_cpu(cpu) {
>-		struct hazptr_overflow_list_flip *overflow_list_flip = per_cpu_ptr(&percpu_overflow_list_flip, cpu);
>-
> 		/*
> 		 * Scan CPU slots.
> 		 * Forward progress against recurring wildcards is guaranteed
>@@ -199,6 +205,17 @@ void hazptr_scan_period(void *addr, void *scan_wildcard)
> 		 * to acquire that same hazard pointer value.
> 		 */
> 		hazptr_synchronize_cpu_slots(cpu, addr, scan_wildcard);
>+	}
>+}
>+
>+static
>+void hazptr_scan_overflow_list_period(void *addr, unsigned int scan_idx)
>+{
>+	int cpu;
>+
>+	/* Scan all CPUs overflow lists. */
>+	for_each_possible_cpu(cpu) {
>+		struct hazptr_overflow_list_flip *overflow_list_flip = per_cpu_ptr(&percpu_overflow_list_flip, cpu);
> 
> 		/*
> 		 * Scan backup slots in percpu overflow lists.
>@@ -218,6 +235,7 @@ void hazptr_scan_period(void *addr, void *scan_wildcard)
>  */
> void hazptr_synchronize(void *addr)
> {
>+	unsigned int scan_list_phase;
> 	void *scan_wildcard;
> 
> 	/*
>@@ -236,10 +254,21 @@ void hazptr_synchronize(void *addr)
> 	smp_mb();
> 
> 	guard(mutex)(&hazptr_wildcard_lock);
>+
>+	/* Scan per-CPU slots. */
> 	scan_wildcard = flip_wildcard(hazptr_wildcard);
>-	hazptr_scan_period(addr, scan_wildcard);
>-	WRITE_ONCE(hazptr_wildcard, scan_wildcard);	/* Flip the current wildcard. */
>-	hazptr_scan_period(addr, flip_wildcard(scan_wildcard));
>+	hazptr_scan_cpu_slots_period(addr, scan_wildcard);
>+	WRITE_ONCE(hazptr_wildcard, scan_wildcard);			/* Flip the current wildcard. */
>+	hazptr_scan_cpu_slots_period(addr, flip_wildcard(scan_wildcard));
>+
>+	/*
>+	 * Scan overflow lists *after* scanning per-CPU slots. See
>+	 * hazptr_promote_to_backup_slot() for scan ordering requirement.
>+	 */
>+	scan_list_phase = flip_list_phase(hazptr_overflow_list_phase);
>+	hazptr_scan_overflow_list_period(addr, scan_list_phase);
>+	WRITE_ONCE(hazptr_overflow_list_phase, scan_list_phase);	/* Flip the current list phase. */
>+	hazptr_scan_overflow_list_period(addr, flip_list_phase(scan_list_phase));
> }
> EXPORT_SYMBOL_GPL(hazptr_synchronize);
> 
>
Nice!

Reviewed-by: Bradley Morgan <brads@mainlining.org>


--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/

Re: [PATCH] hazptr: Fix two-phase hazptr_synchronize race with detach

[Mathieu Desnoyers]

On 2026-09-25 15:49, Mathieu Desnoyers wrote:
> Boqun Feng pointed out that a detach happening concurrently with
> hazptr_synchronize can miss a slot. Indeed, scanning the per-CPU
> slots needs to be done *before* scanning the overflow lists. Fix the
> implementation accordingly.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

Self-NACK. I did not migrate the detach code to move it to the new
hazptr_overflow_list_phase. Sorry about the noise. I'll prepare an
updated version.

Thanks,

Mathieu


> Reported-by: Boqun Feng <boqun@kernel.org>
> Cc: Paul E. McKenney <paulmck@kernel.org>
> Cc: Boqun Feng <boqun@kernel.org>
> Cc: Bradley Morgan <brads@mainlining.org>
> Cc: Gary Guo <gary@garyguo.net>
> Cc: <rcu@vger.kernel.org>
> Cc: <lkmm@lists.linux.dev>
> ---
>   kernel/hazptr.c | 43 ++++++++++++++++++++++++++++++++++++-------
>   1 file changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/hazptr.c b/kernel/hazptr.c
> index d3d1050d92cf..ccc19843edbd 100644
> --- a/kernel/hazptr.c
> +++ b/kernel/hazptr.c
> @@ -24,6 +24,9 @@ static DEFINE_MUTEX(hazptr_wildcard_lock);	/* Protect the wildcard flip. */
>   void *hazptr_wildcard = (void *) 1UL;
>   EXPORT_SYMBOL_GPL(hazptr_wildcard);
>   
> +/* The current overflow list phase. */
> +static unsigned int hazptr_overflow_list_phase;
> +
>   struct hazptr_overflow_list {
>   	raw_spinlock_t lock;		/* Lock protecting overflow list and list generation. */
>   	struct hlist_head head;		/* Overflow list head. */
> @@ -53,6 +56,12 @@ void *flip_wildcard(void *wildcard)
>   	return ((unsigned long) wildcard == 1UL) ? (void *) 2UL : (void *) 1UL;
>   }
>   
> +static
> +unsigned int flip_list_phase(unsigned int phase)
> +{
> +	return 1 - phase;
> +}
> +
>   static
>   bool is_wildcard(void *addr)
>   {
> @@ -178,15 +187,12 @@ void hazptr_synchronize_cpu_slots(int cpu, void *addr, void *scan_wildcard)
>   }
>   
>   static
> -void hazptr_scan_period(void *addr, void *scan_wildcard)
> +void hazptr_scan_cpu_slots_period(void *addr, void *scan_wildcard)
>   {
> -	unsigned int scan_idx = (unsigned long) scan_wildcard - 1;
>   	int cpu;
>   
>   	/* Scan all CPUs slots. */
>   	for_each_possible_cpu(cpu) {
> -		struct hazptr_overflow_list_flip *overflow_list_flip = per_cpu_ptr(&percpu_overflow_list_flip, cpu);
> -
>   		/*
>   		 * Scan CPU slots.
>   		 * Forward progress against recurring wildcards is guaranteed
> @@ -199,6 +205,17 @@ void hazptr_scan_period(void *addr, void *scan_wildcard)
>   		 * to acquire that same hazard pointer value.
>   		 */
>   		hazptr_synchronize_cpu_slots(cpu, addr, scan_wildcard);
> +	}
> +}
> +
> +static
> +void hazptr_scan_overflow_list_period(void *addr, unsigned int scan_idx)
> +{
> +	int cpu;
> +
> +	/* Scan all CPUs overflow lists. */
> +	for_each_possible_cpu(cpu) {
> +		struct hazptr_overflow_list_flip *overflow_list_flip = per_cpu_ptr(&percpu_overflow_list_flip, cpu);
>   
>   		/*
>   		 * Scan backup slots in percpu overflow lists.
> @@ -218,6 +235,7 @@ void hazptr_scan_period(void *addr, void *scan_wildcard)
>    */
>   void hazptr_synchronize(void *addr)
>   {
> +	unsigned int scan_list_phase;
>   	void *scan_wildcard;
>   
>   	/*
> @@ -236,10 +254,21 @@ void hazptr_synchronize(void *addr)
>   	smp_mb();
>   
>   	guard(mutex)(&hazptr_wildcard_lock);
> +
> +	/* Scan per-CPU slots. */
>   	scan_wildcard = flip_wildcard(hazptr_wildcard);
> -	hazptr_scan_period(addr, scan_wildcard);
> -	WRITE_ONCE(hazptr_wildcard, scan_wildcard);	/* Flip the current wildcard. */
> -	hazptr_scan_period(addr, flip_wildcard(scan_wildcard));
> +	hazptr_scan_cpu_slots_period(addr, scan_wildcard);
> +	WRITE_ONCE(hazptr_wildcard, scan_wildcard);			/* Flip the current wildcard. */
> +	hazptr_scan_cpu_slots_period(addr, flip_wildcard(scan_wildcard));
> +
> +	/*
> +	 * Scan overflow lists *after* scanning per-CPU slots. See
> +	 * hazptr_promote_to_backup_slot() for scan ordering requirement.
> +	 */
> +	scan_list_phase = flip_list_phase(hazptr_overflow_list_phase);
> +	hazptr_scan_overflow_list_period(addr, scan_list_phase);
> +	WRITE_ONCE(hazptr_overflow_list_phase, scan_list_phase);	/* Flip the current list phase. */
> +	hazptr_scan_overflow_list_period(addr, flip_list_phase(scan_list_phase));
>   }
>   EXPORT_SYMBOL_GPL(hazptr_synchronize);
>   


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Re: [PATCH] hazptr: Fix two-phase hazptr_synchronize race with detach

[Bradley Morgan]

On 25 September 2026 20:52:45 BST, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>On 2026-09-25 15:49, Mathieu Desnoyers wrote:
>> Boqun Feng pointed out that a detach happening concurrently with
>> hazptr_synchronize can miss a slot. Indeed, scanning the per-CPU
>> slots needs to be done *before* scanning the overflow lists. Fix the
>> implementation accordingly.
>> 
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>
>Self-NACK. I did not migrate the detach code to move it to the new
>hazptr_overflow_list_phase. Sorry about the noise. I'll prepare an
>updated version.
>
>Thanks,
>
>Mathieu

1: Didn't know you could self nak
2: I must've been dumb during review then, in respin I'll think heavier about it.

>
>
>> Reported-by: Boqun Feng <boqun@kernel.org>
>> Cc: Paul E. McKenney <paulmck@kernel.org>
>> Cc: Boqun Feng <boqun@kernel.org>
>> Cc: Bradley Morgan <brads@mainlining.org>
>> Cc: Gary Guo <gary@garyguo.net>
>> Cc: <rcu@vger.kernel.org>
>> Cc: <lkmm@lists.linux.dev>
>> ---
>>   kernel/hazptr.c | 43 ++++++++++++++++++++++++++++++++++++-------
>>   1 file changed, 36 insertions(+), 7 deletions(-)
>> 
>> diff --git a/kernel/hazptr.c b/kernel/hazptr.c
>> index d3d1050d92cf..ccc19843edbd 100644
>> --- a/kernel/hazptr.c
>> +++ b/kernel/hazptr.c
>> @@ -24,6 +24,9 @@ static DEFINE_MUTEX(hazptr_wildcard_lock);	/* Protect the wildcard flip. */
>>   void *hazptr_wildcard = (void *) 1UL;
>>   EXPORT_SYMBOL_GPL(hazptr_wildcard);
>>   +/* The current overflow list phase. */
>> +static unsigned int hazptr_overflow_list_phase;
>> +
>>   struct hazptr_overflow_list {
>>   	raw_spinlock_t lock;		/* Lock protecting overflow list and list generation. */
>>   	struct hlist_head head;		/* Overflow list head. */
>> @@ -53,6 +56,12 @@ void *flip_wildcard(void *wildcard)
>>   	return ((unsigned long) wildcard == 1UL) ? (void *) 2UL : (void *) 1UL;
>>   }
>>   +static
>> +unsigned int flip_list_phase(unsigned int phase)
>> +{
>> +	return 1 - phase;
>> +}
>> +
>>   static
>>   bool is_wildcard(void *addr)
>>   {
>> @@ -178,15 +187,12 @@ void hazptr_synchronize_cpu_slots(int cpu, void
>*addr, void *scan_wildcard)
>>   }
>>     static
>> -void hazptr_scan_period(void *addr, void *scan_wildcard)
>> +void hazptr_scan_cpu_slots_period(void *addr, void *scan_wildcard)
>>   {
>> -	unsigned int scan_idx = (unsigned long) scan_wildcard - 1;
>>   	int cpu;
>>     	/* Scan all CPUs slots. */
>>   	for_each_possible_cpu(cpu) {
>> -		struct hazptr_overflow_list_flip *overflow_list_flip = per_cpu_ptr(&percpu_overflow_list_flip, cpu);
>> -
>>   		/*
>>   		 * Scan CPU slots.
>>   		 * Forward progress against recurring wildcards is guaranteed
>> @@ -199,6 +205,17 @@ void hazptr_scan_period(void *addr, void
>*scan_wildcard)
>>   		 * to acquire that same hazard pointer value.
>>   		 */
>>   		hazptr_synchronize_cpu_slots(cpu, addr, scan_wildcard);
>> +	}
>> +}
>> +
>> +static
>> +void hazptr_scan_overflow_list_period(void *addr, unsigned int
>scan_idx)
>> +{
>> +	int cpu;
>> +
>> +	/* Scan all CPUs overflow lists. */
>> +	for_each_possible_cpu(cpu) {
>> +		struct hazptr_overflow_list_flip *overflow_list_flip = per_cpu_ptr(&percpu_overflow_list_flip, cpu);
>>     		/*
>>   		 * Scan backup slots in percpu overflow lists.
>> @@ -218,6 +235,7 @@ void hazptr_scan_period(void *addr, void
>*scan_wildcard)
>>    */
>>   void hazptr_synchronize(void *addr)
>>   {
>> +	unsigned int scan_list_phase;
>>   	void *scan_wildcard;
>>     	/*
>> @@ -236,10 +254,21 @@ void hazptr_synchronize(void *addr)
>>   	smp_mb();
>>     	guard(mutex)(&hazptr_wildcard_lock);
>> +
>> +	/* Scan per-CPU slots. */
>>   	scan_wildcard = flip_wildcard(hazptr_wildcard);
>> -	hazptr_scan_period(addr, scan_wildcard);
>> -	WRITE_ONCE(hazptr_wildcard, scan_wildcard);	/* Flip the current wildcard. */
>> -	hazptr_scan_period(addr, flip_wildcard(scan_wildcard));
>> +	hazptr_scan_cpu_slots_period(addr, scan_wildcard);
>> +	WRITE_ONCE(hazptr_wildcard, scan_wildcard);			/* Flip the current wildcard. */
>> +	hazptr_scan_cpu_slots_period(addr, flip_wildcard(scan_wildcard));
>> +
>> +	/*
>> +	 * Scan overflow lists *after* scanning per-CPU slots. See
>> +	 * hazptr_promote_to_backup_slot() for scan ordering requirement.
>> +	 */
>> +	scan_list_phase = flip_list_phase(hazptr_overflow_list_phase);
>> +	hazptr_scan_overflow_list_period(addr, scan_list_phase);
>> +	WRITE_ONCE(hazptr_overflow_list_phase, scan_list_phase);	/* Flip the current list phase. */
>> +	hazptr_scan_overflow_list_period(addr, flip_list_phase(scan_list_phase));
>>   }
>>   EXPORT_SYMBOL_GPL(hazptr_synchronize);
>>   
>
>
>

--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/