Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752112AbbBQEqn (ORCPT ); Mon, 16 Feb 2015 23:46:43 -0500 Received: from smtp2.provo.novell.com ([137.65.250.81]:41022 "EHLO smtp2.provo.novell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751559AbbBQEqm (ORCPT ); Mon, 16 Feb 2015 23:46:42 -0500 Message-ID: <1424148397.2046.101.camel@stgolabs.net> Subject: [PATCH] futex: Robustify wake_futex() From: Davidlohr Bueso To: tglx@linutronix.de, mingo@kernel.org Cc: peterz@infradead.org, dvhart@linux.intel.com, linux-kernel@vger.kernel.org, dave@stgolabs.net Date: Mon, 16 Feb 2015 20:46:37 -0800 Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.9 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3692 Lines: 128 Current code assumes that wake_futex() will never fail, thus we are rather sloppy when incrementing the return value in wake related calls, accounting for the newly woken task. Of course this will never occur, thus not a problem. This bug is as real as the need for the redundant pi checks in wake_futex(). These redundant checks are fine and past discussion indicates that they will stay. However, it does introduce this mismatch, thus it is better to robustify the function and avoid any assumptions that could bite us in the arse the future. Signed-off-by: Davidlohr Bueso --- This got my attention this while looking at Micheal's futex manpage changes. The downside of this patch is that the return error from wake_futex() is even more redundant now... however it still seems like the right thing to do. kernel/futex.c | 45 +++++++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/kernel/futex.c b/kernel/futex.c index 2a5e383..a01843a 100644 --- a/kernel/futex.c +++ b/kernel/futex.c @@ -1091,13 +1091,16 @@ static void __unqueue_futex(struct futex_q *q) /* * The hash bucket lock must be held when this is called. * Afterwards, the futex_q must not be accessed. + * + * Returns true only when the task was woken, otherwise false. */ -static void wake_futex(struct futex_q *q) +static bool wake_futex(struct futex_q *q) { struct task_struct *p = q->task; - if (WARN(q->pi_state || q->rt_waiter, "refusing to wake PI futex\n")) - return; + if (unlikely(WARN(q->pi_state || q->rt_waiter, + "refusing to wake PI futex\n"))) + return false; /* * We set q->lock_ptr = NULL _before_ we wake up the task. If @@ -1120,6 +1123,7 @@ static void wake_futex(struct futex_q *q) wake_up_state(p, TASK_NORMAL); put_task_struct(p); + return true; } static int wake_futex_pi(u32 __user *uaddr, u32 uval, struct futex_q *this) @@ -1244,7 +1248,10 @@ futex_wake(u32 __user *uaddr, unsigned int flags, int nr_wake, u32 bitset) if (!(this->bitset & bitset)) continue; - wake_futex(this); + if (!wake_futex(this)) { + ret = -EINVAL; + break; + } if (++ret >= nr_wake) break; } @@ -1320,7 +1327,11 @@ retry_private: ret = -EINVAL; goto out_unlock; } - wake_futex(this); + + if (!wake_futex(this)) { + ret = -EINVAL; + goto out_unlock; + } if (++ret >= nr_wake) break; } @@ -1334,7 +1345,11 @@ retry_private: ret = -EINVAL; goto out_unlock; } - wake_futex(this); + + if (!wake_futex(this)) { + ret = -EINVAL; + goto out_unlock; + } if (++op_ret >= nr_wake2) break; } @@ -1674,13 +1689,19 @@ retry_private: } /* - * Wake nr_wake waiters. For requeue_pi, if we acquired the - * lock, we already woke the top_waiter. If not, it will be - * woken by futex_unlock_pi(). + * For requeue_pi, if we acquired the lock, we already woke + * the top_waiter. If not, it will be woken by futex_unlock_pi. + * + * The regular (non-pi) case is much simpler: Wake the top + * waiter (next in line) and repeat. */ - if (++task_count <= nr_wake && !requeue_pi) { - wake_futex(this); - continue; + if (!requeue_pi) { + if (!wake_futex(this)) { + ret = -EINVAL; + break; + } + if (++task_count <= nr_wake) + continue; } /* Ensure we requeue to the expected futex for requeue_pi. */ -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/