|
Jiri Wiesner |
c8b2fc |
From 90d758896787048fa3d4209309d4800f3920e66f Mon Sep 17 00:00:00 2001
|
|
Jiri Wiesner |
c8b2fc |
From: Alexey Izbyshev <izbyshev@ispras.ru>
|
|
Jiri Wiesner |
c8b2fc |
Date: Sat, 12 Nov 2022 00:54:39 +0300
|
|
Jiri Wiesner |
c8b2fc |
Subject: [PATCH] futex: Resend potentially swallowed owner death notification
|
|
Jiri Wiesner |
c8b2fc |
Git-commit: 90d758896787048fa3d4209309d4800f3920e66f
|
|
Jiri Wiesner |
c8b2fc |
Patch-mainline: v6.2-rc1
|
|
Jiri Wiesner |
c8b2fc |
References: git-fixes
|
|
Jiri Wiesner |
c8b2fc |
|
|
Jiri Wiesner |
c8b2fc |
Commit ca16d5bee598 ("futex: Prevent robust futex exit race") addressed
|
|
Jiri Wiesner |
c8b2fc |
two cases when tasks waiting on a robust non-PI futex remained blocked
|
|
Jiri Wiesner |
c8b2fc |
despite the futex not being owned anymore:
|
|
Jiri Wiesner |
c8b2fc |
|
|
Jiri Wiesner |
c8b2fc |
* if the owner died after writing zero to the futex word, but before
|
|
Jiri Wiesner |
c8b2fc |
waking up a waiter
|
|
Jiri Wiesner |
c8b2fc |
|
|
Jiri Wiesner |
c8b2fc |
* if a task waiting on the futex was woken up, but died before updating
|
|
Jiri Wiesner |
c8b2fc |
the futex word (effectively swallowing the notification without acting
|
|
Jiri Wiesner |
c8b2fc |
on it)
|
|
Jiri Wiesner |
c8b2fc |
|
|
Jiri Wiesner |
c8b2fc |
In the second case, the task could be woken up either by the previous
|
|
Jiri Wiesner |
c8b2fc |
owner (after the futex word was reset to zero) or by the kernel (after
|
|
Jiri Wiesner |
c8b2fc |
the OWNER_DIED bit was set and the TID part of the futex word was reset
|
|
Jiri Wiesner |
c8b2fc |
to zero) if the previous owner died without the resetting the futex.
|
|
Jiri Wiesner |
c8b2fc |
|
|
Jiri Wiesner |
c8b2fc |
Because the referenced commit wakes up a potential waiter only if the
|
|
Jiri Wiesner |
c8b2fc |
whole futex word is zero, the latter subcase remains unaddressed.
|
|
Jiri Wiesner |
c8b2fc |
|
|
Jiri Wiesner |
c8b2fc |
Fix this by looking only at the TID part of the futex when deciding
|
|
Jiri Wiesner |
c8b2fc |
whether a wake up is needed.
|
|
Jiri Wiesner |
c8b2fc |
|
|
Jiri Wiesner |
c8b2fc |
Fixes: ca16d5bee598 ("futex: Prevent robust futex exit race")
|
|
Jiri Wiesner |
c8b2fc |
Signed-off-by: Alexey Izbyshev <izbyshev@ispras.ru>
|
|
Jiri Wiesner |
c8b2fc |
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
|
|
Jiri Wiesner |
c8b2fc |
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
|
|
Jiri Wiesner |
c8b2fc |
Link: https://lore.kernel.org/r/20221111215439.248185-1-izbyshev@ispras.ru
|
|
Jiri Wiesner |
c8b2fc |
Signed-off-by: Jiri Wiesner <jwiesner@suse.de>
|
|
Jiri Wiesner |
c8b2fc |
---
|
|
Jiri Wiesner |
c8b2fc |
kernel/futex/core.c | 26 +++++++++++++++++---------
|
|
Jiri Wiesner |
c8b2fc |
1 file changed, 17 insertions(+), 9 deletions(-)
|
|
Jiri Wiesner |
c8b2fc |
|
|
Jiri Wiesner |
c8b2fc |
diff --git a/kernel/futex/core.c b/kernel/futex/core.c
|
|
Jiri Wiesner |
c8b2fc |
index b22ef1efe751..514e4582b863 100644
|
|
Jiri Wiesner |
c8b2fc |
--- a/kernel/futex/core.c
|
|
Jiri Wiesner |
c8b2fc |
+++ b/kernel/futex/core.c
|
|
Jiri Wiesner |
c8b2fc |
@@ -638,6 +638,7 @@ static int handle_futex_death(u32 __user *uaddr, struct task_struct *curr,
|
|
Jiri Wiesner |
c8b2fc |
bool pi, bool pending_op)
|
|
Jiri Wiesner |
c8b2fc |
{
|
|
Jiri Wiesner |
c8b2fc |
u32 uval, nval, mval;
|
|
Jiri Wiesner |
c8b2fc |
+ pid_t owner;
|
|
Jiri Wiesner |
c8b2fc |
int err;
|
|
Jiri Wiesner |
c8b2fc |
|
|
Jiri Wiesner |
c8b2fc |
/* Futex address must be 32bit aligned */
|
|
Jiri Wiesner |
c8b2fc |
@@ -659,6 +660,10 @@ static int handle_futex_death(u32 __user *uaddr, struct task_struct *curr,
|
|
Jiri Wiesner |
c8b2fc |
* 2. A woken up waiter is killed before it can acquire the
|
|
Jiri Wiesner |
c8b2fc |
* futex in user space.
|
|
Jiri Wiesner |
c8b2fc |
*
|
|
Jiri Wiesner |
c8b2fc |
+ * In the second case, the wake up notification could be generated
|
|
Jiri Wiesner |
c8b2fc |
+ * by the unlock path in user space after setting the futex value
|
|
Jiri Wiesner |
c8b2fc |
+ * to zero or by the kernel after setting the OWNER_DIED bit below.
|
|
Jiri Wiesner |
c8b2fc |
+ *
|
|
Jiri Wiesner |
c8b2fc |
* In both cases the TID validation below prevents a wakeup of
|
|
Jiri Wiesner |
c8b2fc |
* potential waiters which can cause these waiters to block
|
|
Jiri Wiesner |
c8b2fc |
* forever.
|
|
Jiri Wiesner |
c8b2fc |
@@ -667,24 +672,27 @@ static int handle_futex_death(u32 __user *uaddr, struct task_struct *curr,
|
|
Jiri Wiesner |
c8b2fc |
*
|
|
Jiri Wiesner |
c8b2fc |
* 1) task->robust_list->list_op_pending != NULL
|
|
Jiri Wiesner |
c8b2fc |
* @pending_op == true
|
|
Jiri Wiesner |
c8b2fc |
- * 2) User space futex value == 0
|
|
Jiri Wiesner |
c8b2fc |
+ * 2) The owner part of user space futex value == 0
|
|
Jiri Wiesner |
c8b2fc |
* 3) Regular futex: @pi == false
|
|
Jiri Wiesner |
c8b2fc |
*
|
|
Jiri Wiesner |
c8b2fc |
* If these conditions are met, it is safe to attempt waking up a
|
|
Jiri Wiesner |
c8b2fc |
* potential waiter without touching the user space futex value and
|
|
Jiri Wiesner |
c8b2fc |
- * trying to set the OWNER_DIED bit. The user space futex value is
|
|
Jiri Wiesner |
c8b2fc |
- * uncontended and the rest of the user space mutex state is
|
|
Jiri Wiesner |
c8b2fc |
- * consistent, so a woken waiter will just take over the
|
|
Jiri Wiesner |
c8b2fc |
- * uncontended futex. Setting the OWNER_DIED bit would create
|
|
Jiri Wiesner |
c8b2fc |
- * inconsistent state and malfunction of the user space owner died
|
|
Jiri Wiesner |
c8b2fc |
- * handling.
|
|
Jiri Wiesner |
c8b2fc |
+ * trying to set the OWNER_DIED bit. If the futex value is zero,
|
|
Jiri Wiesner |
c8b2fc |
+ * the rest of the user space mutex state is consistent, so a woken
|
|
Jiri Wiesner |
c8b2fc |
+ * waiter will just take over the uncontended futex. Setting the
|
|
Jiri Wiesner |
c8b2fc |
+ * OWNER_DIED bit would create inconsistent state and malfunction
|
|
Jiri Wiesner |
c8b2fc |
+ * of the user space owner died handling. Otherwise, the OWNER_DIED
|
|
Jiri Wiesner |
c8b2fc |
+ * bit is already set, and the woken waiter is expected to deal with
|
|
Jiri Wiesner |
c8b2fc |
+ * this.
|
|
Jiri Wiesner |
c8b2fc |
*/
|
|
Jiri Wiesner |
c8b2fc |
- if (pending_op && !pi && !uval) {
|
|
Jiri Wiesner |
c8b2fc |
+ owner = uval & FUTEX_TID_MASK;
|
|
Jiri Wiesner |
c8b2fc |
+
|
|
Jiri Wiesner |
c8b2fc |
+ if (pending_op && !pi && !owner) {
|
|
Jiri Wiesner |
c8b2fc |
futex_wake(uaddr, 1, 1, FUTEX_BITSET_MATCH_ANY);
|
|
Jiri Wiesner |
c8b2fc |
return 0;
|
|
Jiri Wiesner |
c8b2fc |
}
|
|
Jiri Wiesner |
c8b2fc |
|
|
Jiri Wiesner |
c8b2fc |
- if ((uval & FUTEX_TID_MASK) != task_pid_vnr(curr))
|
|
Jiri Wiesner |
c8b2fc |
+ if (owner != task_pid_vnr(curr))
|
|
Jiri Wiesner |
c8b2fc |
return 0;
|
|
Jiri Wiesner |
c8b2fc |
|
|
Jiri Wiesner |
c8b2fc |
/*
|
|
Jiri Wiesner |
c8b2fc |
--
|
|
Jiri Wiesner |
c8b2fc |
2.35.3
|
|
Jiri Wiesner |
c8b2fc |
|