linux-kernel - Re: [PATCH] futex: Avoid reusing outdated pi

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: <20acc902-a34b-4289-ae1a-8abf211b11df@kernel.org>
Date: Tue, 16 Jan 2024 14:58:29 +0100
From: Jiri Slaby <jirislaby@...nel.org>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
 Zijlstra <peterz@...radead.org>, Thomas Gleixner <tglx@...utronix.de>
Cc: linux-kernel@...r.kernel.org, boqun.feng@...il.com, bristot@...hat.com,
 bsegall@...gle.com, dietmar.eggemann@....com, jstultz@...gle.com,
 juri.lelli@...hat.com, longman@...hat.com, mgorman@...e.de,
 mingo@...hat.com, rostedt@...dmis.org, swood@...hat.com,
 vincent.guittot@...aro.org, vschneid@...hat.com, will@...nel.org
Subject: Re: [PATCH] futex: Avoid reusing outdated pi_state.

On 16. 01. 24, 14:08, Sebastian Andrzej Siewior wrote:
> Jiri Slaby reported a futex state inconsistency resulting in -EINVAL
> during a lock operation for a PI futex. A requirement is that the lock
> process is interrupted by a timeout or signal:
> 
> T1		T2
> *owns* futex
> 		futex_lock_pi()
>              	*create PI state, attach to it, queue RT waiter*
> 		rt_mutex_wait_proxy_lock() /* -ETIMEDOUT */
> 		rt_mutex_cleanup_proxy_lock()
> 		  remove_waiter()
> 
> futex_unlock_pi()
> spin_lock(&hb->lock);
> top_waiter = futex_top_waiter(hb, &key);
> /* top_waiter is NULL, do_uncontended */
> spin_unlock(&hb->lock);
> 
> To spice things up, player T3 and T4 enter the game:
> 
> 	T3 			T4
> 	*acquires futex in userland*
> 				futex_lock_pi()
> 				futex_q_lock(&q);
> 				futex_lock_pi_atomic()
> 				top_waiter = futex_top_waiter(hb, key);
> 				/* top_waiter is from T1, still */
> 				attach_to_pi_state()
> 				/* Here -EINVAL is returned because uval
> 				 *  points to T3 but pi_state says T1.
> 				 */
> 
> We must not unlock the futex for userland as long as there is still a
> state pending in kernel. It can be used by further futex_lock_pi()
> caller (as it has been observed by futex_unlock_pi()). The caller will
> observe an outdated state of the futex because it was not removed during
> unlock operation in kernel.
> 
> The lock can not be handed over to T1 because it already gave up and
> stared to clean up.
> All futex_q entries point to the same pi_state and the pi_mutex has no
> waiters. A waiter can not be enqueued because hb->lock +
> pi_mutex.wait_lock is acquired (by the unlock operation) and the same
> ordering is used by futex_lock_pi() during locking.
> 
> Remove all futex_q entries from the hb list which point to the futex if
> no waiter has been observed. This closes the race window by removing all
> pointer to the previous in-kernel state.
> The leaving futex_lock_pi() caller can clean up the pi-state once it
> acquires hb->lock. The following futex_lock_pi() caller will create a
> new in-kernel state.
> The optional removal from hb->chain is only needed if the futex was not
> acquired because it might have been done by the unlock path with
> hb->lock acquired.
> 
> Fixes: fbeb558b0dd0d ("futex/pi: Fix recursive rt_mutex waiter state")
> Reported-by: Jiri Slaby <jirislaby@...nel.org>

Tested-by: Jiri Slaby <jirislaby@...nel.org>

> Closes: c737a604-d441-49c6-a5cd-ef01e9f2a454@...nel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>

thanks,
-- 
js
suse labs