[PATCH RFC] __futex_pivot_hash() race

[PATCH RFC] __futex_pivot_hash() race

[Chris Mason]

This is really just AI questioning how we synchronize mmph->batches and
mmph->hash.  It looks real to me, and the repro with printks does fire,
so hopefully I've got things right.

The patch and description were also AI, meaning this is more bug
report in patch form.  Testing was only done against the one repro.

(I may or may not have successfully switched over my git config to
mason@kernel.org, lets see...)

[PATCH] futex: sample poll cookie after publishing new hash

[Chris Mason]

futex_ref_drop() may only skip its grace period when one has already
elapsed since the current private hash was published:

    kernel/futex/core.c:futex_ref_drop
        if (poll_state_synchronize_rcu(mm->futex.phash.batches)) {
                /*
                 * There was a grace-period, we can begin now.
                 */
                __futex_ref_atomic_begin(fph);
                return;
        }

The cookie it polls is sampled one statement before that publication:

    kernel/futex/core.c:__futex_pivot_hash
        new->state = FR_PERCPU;
        scoped_guard(rcu) {
                mmph->batches = get_state_synchronize_rcu();
                rcu_assign_pointer(mmph->hash, new);
        }
        kvfree_rcu(fph, rcu);

get_state_synchronize_rcu() anchors its guarantee at the snapshot, so
the cookie is cleared by the first grace period that starts from there
on, including one starting between the two stores which never waited
for a reader that loaded the old hash after it began.

    CPU 0 (resize)                    CPU 1 (futex_hash)
    ==============                    ==================
    __futex_pivot_hash()
      batches = get_state_...()
                                      grace period starts
                                      guard(rcu)
                                      fph = old hash
      rcu_assign_pointer(hash, new)
      kvfree_rcu(old hash)
    futex_hash_allocate()
      futex_ref_drop(new hash)
        poll_state_...() -> true
        __futex_ref_atomic_begin()
          atomic = LONG_MAX
                                      futex_ref_get(old hash) -> true
                                      spin_lock(&fph->queues[i].lock)

The reference count lives in the mm and has just been biased for the
new generation, so the stalled reader pins and then locks the retired
hash that is already queued for free, and its later put is charged
against the live generation.

Fix by sampling the cookie after rcu_assign_pointer() publishes the
new hash. Drop the surrounding scoped_guard(rcu) while at it: it only
delayed completion of the prematurely anchored grace-period and serves
no purpose once the cookie is sampled after publication. The writer
side is serialized by mm->futex.phash.lock and neither
rcu_assign_pointer() nor kvfree_rcu() requires a read-side section.

Fixes: 56180dd20c19 ("futex: Use RCU-based per-CPU reference counting instead of rcuref_t")
Assisted-by: kres:claude-opus-5
Signed-off-by: Chris Mason <mason@kernel.org>
---
 kernel/futex/core.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index 128c5752f225..05619eb8329c 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -209,10 +209,14 @@ static bool __futex_pivot_hash(struct mm_struct *mm, struct futex_private_hash *
 		futex_rehash_private(fph, new);
 	}
 	new->state = FR_PERCPU;
-	scoped_guard(rcu) {
-		mmph->batches = get_state_synchronize_rcu();
-		rcu_assign_pointer(mmph->hash, new);
-	}
+	rcu_assign_pointer(mmph->hash, new);
+	/*
+	 * Pairs with futex_ref_drop(): ->batches must be sampled at or after
+	 * the rcu_assign_pointer() above, so any grace-period satisfying
+	 * poll_state_synchronize_rcu() provably started once no reader could
+	 * still load the retired fph. Both stores are done under mmph->lock.
+	 */
+	mmph->batches = get_state_synchronize_rcu();
 	kvfree_rcu(fph, rcu);
 	return true;
 }
-- 
2.53.0-Meta

Re: [PATCH] futex: sample poll cookie after publishing new hash

[Peter Zijlstra]

On Mon, Aug 17, 2026 at 06:01:42PM -0700, Chris Mason wrote:
> futex_ref_drop() may only skip its grace period when one has already
> elapsed since the current private hash was published:
> 
>     kernel/futex/core.c:futex_ref_drop
>         if (poll_state_synchronize_rcu(mm->futex.phash.batches)) {
>                 /*
>                  * There was a grace-period, we can begin now.
>                  */
>                 __futex_ref_atomic_begin(fph);
>                 return;
>         }
> 
> The cookie it polls is sampled one statement before that publication:
> 
>     kernel/futex/core.c:__futex_pivot_hash
>         new->state = FR_PERCPU;
>         scoped_guard(rcu) {
>                 mmph->batches = get_state_synchronize_rcu();
>                 rcu_assign_pointer(mmph->hash, new);
>         }
>         kvfree_rcu(fph, rcu);
> 
> get_state_synchronize_rcu() anchors its guarantee at the snapshot, so
> the cookie is cleared by the first grace period that starts from there
> on, including one starting between the two stores which never waited
> for a reader that loaded the old hash after it began.
> 
>     CPU 0 (resize)                    CPU 1 (futex_hash)
>     ==============                    ==================
>     __futex_pivot_hash()
>       batches = get_state_...()
>                                       grace period starts
>                                       guard(rcu)
>                                       fph = old hash
>       rcu_assign_pointer(hash, new)
>       kvfree_rcu(old hash)
>     futex_hash_allocate()
>       futex_ref_drop(new hash)
>         poll_state_...() -> true
>         __futex_ref_atomic_begin()
>           atomic = LONG_MAX
>                                       futex_ref_get(old hash) -> true
>                                       spin_lock(&fph->queues[i].lock)
> 
> The reference count lives in the mm and has just been biased for the
> new generation, so the stalled reader pins and then locks the retired
> hash that is already queued for free, and its later put is charged
> against the live generation.
> 
> Fix by sampling the cookie after rcu_assign_pointer() publishes the
> new hash. Drop the surrounding scoped_guard(rcu) while at it: it only
> delayed completion of the prematurely anchored grace-period and serves
> no purpose once the cookie is sampled after publication. The writer
> side is serialized by mm->futex.phash.lock and neither
> rcu_assign_pointer() nor kvfree_rcu() requires a read-side section.

God, how I hate reading AI output :-(

Anyway, the thinking was that by holding rcu_read_lock(), the current
RCU-GP cannot change and the cookie and assignment are effectively
'atomic'.

Paul?

> Fixes: 56180dd20c19 ("futex: Use RCU-based per-CPU reference counting instead of rcuref_t")
> Assisted-by: kres:claude-opus-5
> Signed-off-by: Chris Mason <mason@kernel.org>
> ---
>  kernel/futex/core.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/futex/core.c b/kernel/futex/core.c
> index 128c5752f225..05619eb8329c 100644
> --- a/kernel/futex/core.c
> +++ b/kernel/futex/core.c
> @@ -209,10 +209,14 @@ static bool __futex_pivot_hash(struct mm_struct *mm, struct futex_private_hash *
>  		futex_rehash_private(fph, new);
>  	}
>  	new->state = FR_PERCPU;
> -	scoped_guard(rcu) {
> -		mmph->batches = get_state_synchronize_rcu();
> -		rcu_assign_pointer(mmph->hash, new);
> -	}
> +	rcu_assign_pointer(mmph->hash, new);
> +	/*
> +	 * Pairs with futex_ref_drop(): ->batches must be sampled at or after
> +	 * the rcu_assign_pointer() above, so any grace-period satisfying
> +	 * poll_state_synchronize_rcu() provably started once no reader could
> +	 * still load the retired fph. Both stores are done under mmph->lock.
> +	 */
> +	mmph->batches = get_state_synchronize_rcu();
>  	kvfree_rcu(fph, rcu);
>  	return true;
>  }
> -- 
> 2.53.0-Meta
>