linux-kernel - Re: [PATCH v12 10/21] futex: Introduce futex_q_lockptr

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: <0cd48028-8eed-49c1-b4b8-798037457c8e@igalia.com>
Date: Thu, 8 May 2025 16:06:28 -0300
From: André Almeida <andrealmeid@...lia.com>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc: Darren Hart <dvhart@...radead.org>, Davidlohr Bueso <dave@...olabs.net>,
 Ingo Molnar <mingo@...hat.com>, Juri Lelli <juri.lelli@...hat.com>,
 Peter Zijlstra <peterz@...radead.org>, Thomas Gleixner <tglx@...utronix.de>,
 Valentin Schneider <vschneid@...hat.com>, Waiman Long <longman@...hat.com>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v12 10/21] futex: Introduce futex_q_lockptr_lock()

Em 16/04/2025 13:29, Sebastian Andrzej Siewior escreveu:
> futex_lock_pi() and __fixup_pi_state_owner() acquire the
> futex_q::lock_ptr without holding a reference assuming the previously
> obtained hash bucket and the assigned lock_ptr are still valid. This
> isn't the case once the private hash can be resized and becomes invalid
> after the reference drop.
> 
> Introduce futex_q_lockptr_lock() to lock the hash bucket recorded in
> futex_q::lock_ptr. The lock pointer is read in a RCU section to ensure
> that it does not go away if the hash bucket has been replaced and the
> old pointer has been observed. After locking the pointer needs to be
> compared to check if it changed. If so then the hash bucket has been
> replaced and the user has been moved to the new one and lock_ptr has
> been updated. The lock operation needs to be redone in this case.
> 
> The locked hash bucket is not returned.
> 
> A special case is an early return in futex_lock_pi() (due to signal or
> timeout) and a successful futex_wait_requeue_pi(). In both cases a valid
> futex_q::lock_ptr is expected (and its matching hash bucket) but since
> the waiter has been removed from the hash this can no longer be
> guaranteed. Therefore before the waiter is removed and a reference is
> acquired which is later dropped by the waiter to avoid a resize.
> 
> Add futex_q_lockptr_lock() and use it.
> Acquire an additional reference in requeue_pi_wake_futex() and
> futex_unlock_pi() while the futex_q is removed, denote this extra
> reference in futex_q::drop_hb_ref and let the waiter drop the reference
> in this case.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> ---
>   kernel/futex/core.c    | 25 +++++++++++++++++++++++++
>   kernel/futex/futex.h   |  3 ++-
>   kernel/futex/pi.c      | 15 +++++++++++++--
>   kernel/futex/requeue.c | 16 +++++++++++++---
>   4 files changed, 53 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/futex/core.c b/kernel/futex/core.c
> index 5e70cb8eb2507..1443a98dfa7fa 100644
> --- a/kernel/futex/core.c
> +++ b/kernel/futex/core.c
> @@ -134,6 +134,13 @@ struct futex_hash_bucket *futex_hash(union futex_key *key)
>   	return &futex_queues[hash & futex_hashmask];
>   }
>   
> +/**
> + * futex_hash_get - Get an additional reference for the local hash.
> + * @hb:                    ptr to the private local hash.
> + *
> + * Obtain an additional reference for the already obtained hash bucket. The
> + * caller must already own an reference.
> + */

This comment should come with patch 6 (that creates the function) or 
patch 14 (that implements the function).

>   void futex_hash_get(struct futex_hash_bucket *hb) { }
>   void futex_hash_put(struct futex_hash_bucket *hb) { }
>   
> @@ -615,6 +622,24 @@ int futex_unqueue(struct futex_q *q)
>   	return ret;
>   }
>   
> +void futex_q_lockptr_lock(struct futex_q *q)
> +{
> +	spinlock_t *lock_ptr;
> +
> +	/*
> +	 * See futex_unqueue() why lock_ptr can change.
> +	 */
> +	guard(rcu)();
> +retry:
> +	lock_ptr = READ_ONCE(q->lock_ptr);
> +	spin_lock(lock_ptr);
> +
> +	if (unlikely(lock_ptr != q->lock_ptr)) {
> +		spin_unlock(lock_ptr);
> +		goto retry;
> +	}
> +}
> +
>   /*
>    * PI futexes can not be requeued and must remove themselves from the hash
>    * bucket. The hash bucket lock (i.e. lock_ptr) is held.
> diff --git a/kernel/futex/futex.h b/kernel/futex/futex.h
> index bc76e366f9a77..26e69333cb745 100644
> --- a/kernel/futex/futex.h
> +++ b/kernel/futex/futex.h
> @@ -183,6 +183,7 @@ struct futex_q {
>   	union futex_key *requeue_pi_key;
>   	u32 bitset;
>   	atomic_t requeue_state;
> +	bool drop_hb_ref;

This new member needs a comment:

  * @drop_hb_ref:	True if an extra reference was acquired by a pi 
operation, and needs an extra put()