linux-kernel - Re: [PATCH -v6 13/13] futex: futex_lock_pi() vs PREEMPT_RT

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: <20170410090818.ztb2fxzb6oy3ee7o@hirez.programming.kicks-ass.net>
Date:   Mon, 10 Apr 2017 11:08:18 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Darren Hart <dvhart@...radead.org>
Cc:     tglx@...utronix.de, mingo@...nel.org, juri.lelli@....com,
        rostedt@...dmis.org, xlpang@...hat.com, bigeasy@...utronix.de,
        linux-kernel@...r.kernel.org, mathieu.desnoyers@...icios.com,
        jdesfossez@...icios.com, bristot@...hat.com
Subject: Re: [PATCH -v6 13/13] futex: futex_lock_pi() vs PREEMPT_RT_FULL

On Fri, Apr 07, 2017 at 07:26:10PM -0700, Darren Hart wrote:
> Peter, I presume there will be a v7 with the u32 change and hopefully a couple
> text updates?

Well, tglx already committed these here patches, so no -v7. What I can
do however is do a follow up patch that fixes some of the in-code things
you mentioned.

Something like the below is what I had lying about from your earlier
emails; I've not looked to see if there's anything else from your later
emails.

---
Subject: futex: Small misc fixes..
From: Peter Zijlstra <peterz@...radead.org>
Date: Fri Apr 7 09:04:07 CEST 2017

Feedback from Darren's review.

Reported-by: Darren Hart (VMWare) <dvhart@...radead.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
---
 kernel/futex.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -1025,7 +1025,8 @@ static int attach_to_pi_state(u32 __user
 			      struct futex_pi_state **ps)
 {
 	pid_t pid = uval & FUTEX_TID_MASK;
-	int ret, uval2;
+	u32 uval2;
+	int ret;
 
 	/*
 	 * Userspace might have messed up non-PI and PI futexes [3]
@@ -1441,6 +1442,11 @@ static int wake_futex_pi(u32 __user *uad
 	if (ret)
 		goto out_unlock;
 
+	/*
+	 * This is a point of no return; once we modify the uval there is no
+	 * going back and subsequent operations must not fail.
+	 */
+
 	raw_spin_lock(&pi_state->owner->pi_lock);
 	WARN_ON(list_empty(&pi_state->list));
 	list_del_init(&pi_state->list);
@@ -1452,9 +1458,6 @@ static int wake_futex_pi(u32 __user *uad
 	pi_state->owner = new_owner;
 	raw_spin_unlock(&new_owner->pi_lock);
 
-	/*
-	 * We've updated the uservalue, this unlock cannot fail.
-	 */
 	postunlock = __rt_mutex_futex_unlock(&pi_state->pi_mutex, &wake_q);
 
 out_unlock: