linux-kernel - [PATCH] rtmutex: Fix compare of waiter prio and task prio

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: <20131121125212.142f2786@gandalf.local.home>
Date:	Thu, 21 Nov 2013 12:52:12 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Juri Lelli <juri.lelli@...il.com>
Cc:	peterz@...radead.org, tglx@...utronix.de, mingo@...hat.com,
	oleg@...hat.com, fweisbec@...il.com, darren@...art.com,
	johan.eker@...csson.com, p.faure@...tech.ch,
	linux-kernel@...r.kernel.org, claudio@...dence.eu.com,
	michael@...rulasolutions.com, fchecconi@...il.com,
	tommaso.cucinotta@...up.it, nicola.manica@...i.unitn.it,
	luca.abeni@...tn.it, dhaval.giani@...il.com, hgu1972@...il.com,
	paulmck@...ux.vnet.ibm.com, raistlin@...ux.it,
	insop.song@...il.com, liming.wang@...driver.com, jkacur@...hat.com,
	harald.gustafsson@...csson.com, vincent.guittot@...aro.org,
	bruce.ashfield@...driver.com
Subject: [PATCH] rtmutex: Fix compare of waiter prio and task prio

The conversion of the rt_mutex from using plist to rbtree eliminated
the use of the waiter->list_entry.prio, and instead used directly the
waiter->task->prio.

The problem with this is that the priority inheritance code relies on
the prio of the waiter being stored is different from the task's prio.
The change didn't take into account waiter->task == task, which makes
the compares of:

	if (waiter->task->prio == task->prio)

rather pointless, since they will always be the same:

	task->pi_blocked_on = waiter;
	waiter->task = task;

When deadlock detection is not being used (for internal users of
rt_mutex_lock(); things other than futex), the code relies on
the prio associated to the waiter being different than the prio
associated to the task.

Another use case where this is critical, is when a task that is
blocked on an rt_mutex has its priority increased by a separate task.
Then the compare in rt_mutex_adjust_pi() (called from
sched_setscheduler()), returns without doing anything. This is because
it checks if the priority of the task is different than the priority of
its waiter.

The simple solution is to add a prio member to the rt_mutex_waiter
structure that associates the priority to the waiter that is separate
from the task.

I created a test program that tests this case:

  http://rostedt.homelinux.com/code/pi_mutex_test.c

(too big to include in a change log) I'll work on getting this test
into other projects like LTP and the kernel (perf test?)

Signed-off-by: Steven Rostedt <rostedt@...dmis.org>

Index: linux-rt.git/kernel/rtmutex.c
===================================================================
--- linux-rt.git.orig/kernel/rtmutex.c
+++ linux-rt.git/kernel/rtmutex.c
@@ -197,7 +197,7 @@ int rt_mutex_getprio(struct task_struct
 	if (likely(!task_has_pi_waiters(task)))
 		return task->normal_prio;
 
-	return min(task_top_pi_waiter(task)->task->prio,
+	return min(task_top_pi_waiter(task)->prio,
 		   task->normal_prio);
 }
 
@@ -336,7 +336,7 @@ static int rt_mutex_adjust_prio_chain(st
 	 * When deadlock detection is off then we check, if further
 	 * priority adjustment is necessary.
 	 */
-	if (!detect_deadlock && waiter->task->prio == task->prio)
+	if (!detect_deadlock && waiter->prio == task->prio)
 		goto out_unlock_pi;
 
 	lock = waiter->lock;
@@ -358,7 +358,7 @@ static int rt_mutex_adjust_prio_chain(st
 
 	/* Requeue the waiter */
 	rt_mutex_dequeue(lock, waiter);
-	waiter->task->prio = task->prio;
+	waiter->prio = task->prio;
 	rt_mutex_enqueue(lock, waiter);
 
 	/* Release the task */
@@ -456,7 +456,7 @@ static int try_to_take_rt_mutex(struct r
 	 * 3) it is top waiter
 	 */
 	if (rt_mutex_has_waiters(lock)) {
-		if (task->prio >= rt_mutex_top_waiter(lock)->task->prio) {
+		if (task->prio >= rt_mutex_top_waiter(lock)->prio) {
 			if (!waiter || waiter != rt_mutex_top_waiter(lock))
 				return 0;
 		}
@@ -516,7 +516,8 @@ static int task_blocks_on_rt_mutex(struc
 	__rt_mutex_adjust_prio(task);
 	waiter->task = task;
 	waiter->lock = lock;
-	
+	waiter->prio = task->prio;
+
 	/* Get the top priority waiter on the lock */
 	if (rt_mutex_has_waiters(lock))
 		top_waiter = rt_mutex_top_waiter(lock);
@@ -661,7 +662,7 @@ void rt_mutex_adjust_pi(struct task_stru
 	raw_spin_lock_irqsave(&task->pi_lock, flags);
 
 	waiter = task->pi_blocked_on;
-	if (!waiter || (waiter->task->prio == task->prio &&
+	if (!waiter || (waiter->prio == task->prio &&
 			!dl_prio(task->prio))) {
 		raw_spin_unlock_irqrestore(&task->pi_lock, flags);
 		return;
Index: linux-rt.git/kernel/rtmutex_common.h
===================================================================
--- linux-rt.git.orig/kernel/rtmutex_common.h
+++ linux-rt.git/kernel/rtmutex_common.h
@@ -54,6 +54,7 @@ struct rt_mutex_waiter {
 	struct pid		*deadlock_task_pid;
 	struct rt_mutex		*deadlock_lock;
 #endif
+	int			prio;
 };
 
 /*
--
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/