linux-kernel - [PATCH] sched: remove unreliable pointer in mutex_spin_on

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-next>] [day] [month] [year] [list]

Message-ID: <BANLkTim9cr6H9fTik8-yQ4E0j1yFAOt-7g@mail.gmail.com>
Date:	Wed, 8 Jun 2011 20:49:53 +0800
From:	Hillf Danton <dhillf@...il.com>
To:	LKML <linux-kernel@...r.kernel.org>
Cc:	Ingo Molnar <mingo@...e.hu>, Peter Zijlstra <peterz@...radead.org>
Subject: [PATCH] sched: remove unreliable pointer in mutex_spin_on_owner()

The dereference of unreliable owner pointer is unnecessary in owner_running(),
though under RCU protection, because the true result is only determined by
checking the validity of lock owner, as the comment says, due to likely heavy
lock contention, which has little to do with whether owner->on_cpu is false.

If owner->on_cpu is really false, only the owner_running loop is shortened,
but also returns incorrect result, since the lock owner is not changed, though
maybe changed soon.

Signed-off-by: Hillf Danton <dhillf@...il.com>
---
 kernel/sched.c |   36 +-----------------------------------
 1 files changed, 1 insertions(+), 35 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index fd18f39..2c32616 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -4293,52 +4293,18 @@ EXPORT_SYMBOL(schedule);

 #ifdef CONFIG_MUTEX_SPIN_ON_OWNER

-static inline bool owner_running(struct mutex *lock, struct task_struct *owner)
-{
-	bool ret = false;
-
-	rcu_read_lock();
-	if (lock->owner != owner)
-		goto fail;
-
-	/*
-	 * Ensure we emit the owner->on_cpu, dereference _after_ checking
-	 * lock->owner still matches owner, if that fails, owner might
-	 * point to free()d memory, if it still matches, the rcu_read_lock()
-	 * ensures the memory stays valid.
-	 */
-	barrier();
-
-	ret = owner->on_cpu;
-fail:
-	rcu_read_unlock();
-
-	return ret;
-}
-
-/*
- * Look out! "owner" is an entirely speculative pointer
- * access and not reliable.
- */
 int mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner)
 {
 	if (!sched_feat(OWNER_SPIN))
 		return 0;

-	while (owner_running(lock, owner)) {
+	while (lock->owner != NULL) {
 		if (need_resched())
 			return 0;

 		arch_mutex_cpu_relax();
 	}

-	/*
-	 * If the owner changed to another task there is likely
-	 * heavy contention, stop spinning.
-	 */
-	if (lock->owner)
-		return 0;
-
 	return 1;
 }
 #endif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/