mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Suleiman Souhlal <suleiman@google.com>
To: linux-kernel@vger.kernel.org
Cc: "Suleiman Souhlal" <suleiman@google.com>,
	"Thomas Gleixner" <tglx@kernel.org>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Darren Hart" <dvhart@infradead.org>,
	"Davidlohr Bueso" <dave@stgolabs.net>,
	"André Almeida" <andrealmeid@igalia.com>,
	"Juri Lelli" <juri.lelli@redhat.com>,
	"Vincent Guittot" <vincent.guittot@linaro.org>,
	"Dietmar Eggemann" <dietmar.eggemann@arm.com>,
	"Steven Rostedt" <rostedt@goodmis.org>,
	"Ben Segall" <bsegall@google.com>, "Mel Gorman" <mgorman@suse.de>,
	"Valentin Schneider" <vschneid@redhat.com>,
	"K Prateek Nayak" <kprateek.nayak@amd.com>,
	"zhidao su" <soolaugust@gmail.com>,
	"John Stultz" <jstultz@google.com>,
	"Qais Yousef" <qyousef@google.com>,
	ssouhlal@FreeBSD.org
Subject: [RFC PATCH 11/12] futex: Allow userspace stealing for PING futexes.
Date: Thu, 17 Sep 2026 04:33:35 +0000	[thread overview]
Message-ID: <20260917043339.2093426-12-suleiman@google.com> (raw)
In-Reply-To: <20260917043339.2093426-1-suleiman@google.com>

Similarly to how the kernel part of PING locking can steal the futex
from the top waiter, it is also possible for userspace to also take
advantage of this and try to steal without going to the kernel.

When a new (contending) locker notices that the lock has been stolen
from userspace, the ownership of the ping_state and the ping_mutex
are fixed up to the real owner.

TODO: Verify that the case where the ping_state owner exits with the
  futex having been user stolen is handled correctly.
  In other words, when ping_state->owner = task getting killed,
  ping_mutex.owner = NULL and uval = real owner (set by userspace).
  Right now, it seems like we might be doing the wrong thing in such
  cases. exit_ping_state_list() needs to detect such situations
  (by walking the uvals?) and fixup the ownerships.

Signed-off-by: Suleiman Souhlal <suleiman@google.com>
---
 kernel/futex/core.c  |  8 ++++++++
 kernel/futex/futex.h |  2 ++
 kernel/futex/pi.c    | 12 ++++++++++--
 kernel/futex/ping.c  | 38 +++++++++++++++++++++++++++++++++++---
 4 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index 56c7e2d5faac..023531c4b45a 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -1445,6 +1445,14 @@ static void exit_ping_state_list(struct task_struct *curr)
 	while (!list_empty(head)) {
 		next = head->next;
 		ping_state = list_entry(next, struct futex_pi_state, list);
+		/*
+		 * XXX In the case when we are ping_state owner but
+		 * the futex is actually owned by someone who stole it
+		 * from userspace, is setting ping_state.owner = NULL here
+		 * enough? Probably not, otherwise the ping_state could leak
+		 * if they also exit before unlocking or someone else
+		 * fixing up the ownership.
+		 */
 		if (1) {
 			CLASS(hbr, hbr)(&key);
 			auto hb = hbr.hb;
diff --git a/kernel/futex/futex.h b/kernel/futex/futex.h
index 113289779dd1..922d130c5b44 100644
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -413,6 +413,8 @@ extern int attach_to_pi_owner(u32 __user *uaddr, u32 uval, union futex_key *key,
 			      bool ping);
 extern void get_ping_state(struct futex_pi_state *ping_state);
 extern void put_ping_state(struct futex_pi_state *ping_state);
+extern int fixup_ping_owner_after_user_steal(struct futex_pi_state *ping_state,
+				      u32 __user *uaddr, u32 uval);
 
 /*
  * Express the locking dependencies for lockdep:
diff --git a/kernel/futex/pi.c b/kernel/futex/pi.c
index e7e6e347f97d..dfd2da5188b0 100644
--- a/kernel/futex/pi.c
+++ b/kernel/futex/pi.c
@@ -348,8 +348,16 @@ int attach_to_pi_state(u32 __user *uaddr, u32 uval,
 	 * state exists then the owner TID must be the same as the
 	 * user space TID. [9/10]
 	 */
-	if (pid != task_pid_vnr(pi_state->owner))
-		goto out_einval;
+	if (pid != task_pid_vnr(pi_state->owner)) {
+		if (!ping) {
+			goto out_einval;
+		} else {
+			ret = fixup_ping_owner_after_user_steal(pi_state,
+								uaddr, uval);
+			if (ret)
+				goto out_error;
+		}
+	}
 
 out_attach:
 	if (!ping) {
diff --git a/kernel/futex/ping.c b/kernel/futex/ping.c
index e5765b2d3834..4cef1c46b4c6 100644
--- a/kernel/futex/ping.c
+++ b/kernel/futex/ping.c
@@ -96,6 +96,22 @@ put_ping_state(struct futex_pi_state *ping_state)
 	}
 }
 
+int fixup_ping_owner_after_user_steal(struct futex_pi_state *ping_state,
+				      u32 __user *uaddr, u32 uval)
+{
+	struct task_struct *p;
+
+	p = find_get_task_by_vpid(uval & FUTEX_TID_MASK);
+	if (p == NULL)
+		return pi_handle_exit_race(uaddr, uval);
+	if (unlikely(p->flags & PF_KTHREAD))
+		return -EPERM;
+	ping_state_update_owner(ping_state, p);
+	WRITE_ONCE(ping_state->ping_mutex.owner, p);
+
+	return 0;
+}
+
 static void futex_unqueue_ping(struct futex_q *q)
 {
 	if (!plist_node_empty(&q->list))
@@ -149,6 +165,7 @@ static int futex_trylock_ping_state(u32 __user *uaddr,
 				    bool handoff)
 {
 	struct task_struct *owner;
+	pid_t pid;
 	u32 uval, new, newtid;
 	int ret;
 
@@ -163,8 +180,17 @@ static int futex_trylock_ping_state(u32 __user *uaddr,
 
 		WARN_ON_ONCE(ping_state->handoff || ping_state->pickup);
 
-		if (uval & FUTEX_TID_MASK) {
-			ret = -EAGAIN;
+		/*
+		 * No owner but a userspace TID means that it got stolen
+		 * from userspace.
+		 * Fix up the ownership.
+		 */
+		pid = uval & FUTEX_TID_MASK;
+		if (pid) {
+			ret = fixup_ping_owner_after_user_steal(ping_state,
+								uaddr, uval);
+			if (ret == 0)
+				ret = -EAGAIN;
 			goto err;
 		}
 		new = newtid | FUTEX_WAITERS;
@@ -203,7 +229,7 @@ static int futex_trylock_ping_state(u32 __user *uaddr,
 	case -EINVAL:
 		break;
 	default:
-		WARN_ON(1);
+		break;
 	}
 	return ret;
 }
@@ -619,6 +645,12 @@ int futex_unlock_ping(u32 __user *uaddr, unsigned int flags)
 		goto out_unlock;
 	raw_spin_lock_irq(&ping_state->ping_mutex.wait_lock);
 
+	/*
+	 * If we're unlocking a lock we stole from userspace,
+	 * it's possible that we don't own the ping_state or the
+	 * ping_mutex. But we'll give them to the next task here anyway.
+	 */
+
 	next = ping_current_proxy_donor(ping_state);
 	get_ping_state(ping_state);
 	/* Leave it queued, it gets unqueued on the lock side */
-- 
2.55.0.1082.g2b9226bbc0-goog


  parent reply	other threads:[~2026-09-17  4:34 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17  4:33 [RFC PATCH 00/12] FUTEX_PING: A stealable futex using Proxy Execution Suleiman Souhlal
2026-09-17  4:33 ` [RFC PATCH 01/12] sched: Abstract task_struct->blocked_on by locking primitive Suleiman Souhlal
2026-09-17  4:33 ` [RFC PATCH 02/12] futex: Switch PI futex to use p->pi_futex_lock instead of p->pi_lock Suleiman Souhlal
2026-09-17 15:38   ` Peter Zijlstra
2026-09-17  4:33 ` [RFC PATCH 03/12] futex: Add "ping" parameter to pi_state management functions and export them Suleiman Souhlal
2026-09-17  4:33 ` [RFC PATCH 04/12] futex: Introduce stealable PI futex, FUTEX_*_PING Suleiman Souhlal
2026-09-17  4:33 ` [RFC PATCH 05/12] futex: Implement exit_ping_state_list() Suleiman Souhlal
2026-09-17  4:33 ` [RFC PATCH 06/12] futex: Address aborting from futex_lock_ping() while owning ping_state Suleiman Souhlal
2026-09-17  4:33 ` [RFC PATCH 07/12] futex: Make FUTEX_*_PING use Proxy Execution Suleiman Souhlal
2026-09-17 13:18   ` Jihan LIN
2026-09-17 14:39     ` K Prateek Nayak
2026-09-17 15:36       ` Peter Zijlstra
2026-09-17  4:33 ` [RFC PATCH 08/12] futex: Implement PING futex handoff Suleiman Souhlal
2026-09-17  4:33 ` [RFC PATCH 09/12] futex: Wake up donor in PING futex unlock Suleiman Souhlal
2026-09-17  4:33 ` [RFC PATCH 10/12] futex: Optimistic spinning for PING futexes Suleiman Souhlal
2026-09-17  4:33 ` Suleiman Souhlal [this message]
2026-09-17  4:33 ` [RFC PATCH 12/12] tools/testing/futex: Add ping_bench, a tool for benchmarking futexes Suleiman Souhlal
2026-09-17  8:58 ` [RFC PATCH 00/12] FUTEX_PING: A stealable futex using Proxy Execution Peter Zijlstra
2026-09-17 17:53   ` John Stultz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260917043339.2093426-12-suleiman@google.com \
    --to=suleiman@google.com \
    --cc=andrealmeid@igalia.com \
    --cc=bsegall@google.com \
    --cc=dave@stgolabs.net \
    --cc=dietmar.eggemann@arm.com \
    --cc=dvhart@infradead.org \
    --cc=jstultz@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=qyousef@google.com \
    --cc=rostedt@goodmis.org \
    --cc=soolaugust@gmail.com \
    --cc=ssouhlal@FreeBSD.org \
    --cc=tglx@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®