From aefd2d632eb0914f22fc6149f52eb0c6c148ce35 Mon Sep 17 00:00:00 2001 From: Todd Kjos Date: Wed, 30 Oct 2019 12:36:47 -0700 Subject: [PATCH] ANDROID: binder: fix sleeping from invalid function caused by RT inheritance When changing a thread's scheduling priority, binder calls sched_setscheduler_nocheck() while holding the node lock and proc inner lock. This was safe until v5.3 when a change was introduced where cpuset_read_lock() is called in this path which can sleep: commit 710da3c8ea7df ("sched/core: Prevent race condition between cpuset and __sched_setscheduler()"). Refactored binder_proc_transaction() to avoid holding a lock when calling binder_transaction_priority(). Bug: 143627611 Change-Id: I405c76b4813777905090ccc33e4f048b37700068 Fixes: e00eb41c0c7be ("ANDROID: binder: add support for RT prio inheritance.") Signed-off-by: Todd Kjos --- drivers/android/binder.c | 61 ++++++++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 12 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 1face45e0e49..bc431dbf94f5 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2911,19 +2911,50 @@ static bool binder_proc_transaction(struct binder_transaction *t, struct binder_priority node_prio; bool oneway = !!(t->flags & TF_ONE_WAY); bool pending_async = false; + bool retry = false; BUG_ON(!node); - binder_node_lock(node); + +set_thread_prio: node_prio.prio = node->min_priority; node_prio.sched_policy = node->sched_policy; + if (thread) { + /* + * Priority must be set outside of lock, but must be + * done before enqueuing the transaction. + */ + binder_transaction_priority(thread->task, t, node_prio, + node->inherit_rt); + } + +retry_after_prio_restore: + binder_node_lock(node); if (oneway) { - BUG_ON(thread); + BUG_ON(!retry && thread); if (node->has_async_transaction) { pending_async = true; } else { node->has_async_transaction = true; } + if (thread && pending_async) { + /* + * The node state has changed since we selected + * the thread. Return the thread to the + * waiting_threads list. We have to drop + * the node lock to restore priority so we + * have to re-check the node state. + */ + binder_node_unlock(node); + binder_restore_priority(thread->task, + proc->default_priority); + binder_inner_proc_lock(proc); + list_add(&thread->waiting_thread_node, + &proc->waiting_threads); + binder_inner_proc_unlock(proc); + thread = NULL; + goto retry_after_prio_restore; + } } binder_inner_proc_lock(proc); @@ -2934,19 +2965,25 @@ static bool binder_proc_transaction(struct binder_transaction *t, return false; } - if (!thread && !pending_async) + if (!thread && !pending_async) { thread = binder_select_thread_ilocked(proc); - - if (thread) { - binder_transaction_priority(thread->task, t, node_prio, - node->inherit_rt); - binder_enqueue_thread_work_ilocked(thread, &t->work); - } else if (!pending_async) { - binder_enqueue_work_ilocked(&t->work, &proc->todo); - } else { - binder_enqueue_work_ilocked(&t->work, &node->async_todo); + if (thread) { + if (oneway) + node->has_async_transaction = false; + binder_inner_proc_unlock(proc); + binder_node_unlock(node); + retry = true; + goto set_thread_prio; + } } + if (thread) + binder_enqueue_thread_work_ilocked(thread, &t->work); + else if (!pending_async) + binder_enqueue_work_ilocked(&t->work, &proc->todo); + else + binder_enqueue_work_ilocked(&t->work, &node->async_todo); + if (!pending_async) binder_wakeup_thread_ilocked(proc, thread, !oneway /* sync */);