Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1723964

[PATCH v2 12/13] ANDROID: binder: don't queue async transactions to thread.

Show all headers | View raw

This can cause issues with processes using the poll()
interface:

1) client sends two oneway transactions
2) the second one gets queued on async_todo
   (because the server didn't handle the first one
    yet)
3) server returns from poll(), picks up the
   first transaction and does transaction work
4) server is done with the transaction, sends
   BC_FREE_BUFFER, and the second transaction gets
   moved to thread->todo
5) libbinder's handlePolledCommands() only handles
   the commands in the current data buffer, so
   doesn't see the new transaction
6) the server continues running and issues a new
   outgoing transaction. Now, it suddenly finds
   the incoming oneway transaction on its thread
   todo, and returns that to userspace.
7) userspace does not expect this to happen; it
   may be holding a lock while making the outgoing
   transaction, and if handling the incoming
   trasnaction requires taking the same lock,
   userspace will deadlock.

By queueing the async transaction to the proc
workqueue, we make sure it's only picked up when
a thread is ready for proc work.

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/android/binder.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 2d23f8699d40..7768ba280177 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -3570,11 +3570,13 @@ static int binder_thread_write(struct binder_proc *proc,
 				BUG_ON(buf_node->proc != proc);
 				w = binder_dequeue_work_head_ilocked(
 						&buf_node->async_todo);
-				if (!w)
+				if (!w) {
 					buf_node->has_async_transaction = 0;
-				else
+				} else {
 					binder_enqueue_work_ilocked(
-							w, &thread->todo);
+							w, &proc->todo);
+					binder_wakeup_proc_ilocked(proc);
+				}
 				binder_node_inner_unlock(buf_node);
 			}
 			trace_binder_transaction_buffer_release(buffer);
-- 
2.14.1.581.gf28d330327-goog

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v2 00/13] ANDROID: binder: RT priority inheritance and small fixes. Martijn Coenen <maco@android.com> - 2017-08-31 10:10 +0200
  [PATCH v2 12/13] ANDROID: binder: don't queue async transactions to thread. Martijn Coenen <maco@android.com> - 2017-08-31 10:10 +0200
  [PATCH v2 08/13] ANDROID: binder: don't check prio permissions on restore. Martijn Coenen <maco@android.com> - 2017-08-31 10:20 +0200
  [PATCH v2 01/13] ANDROID: binder: remove proc waitqueue Martijn Coenen <maco@android.com> - 2017-08-31 10:20 +0200
  [PATCH v2 09/13] ANDROID: binder: Don't BUG_ON(!spin_is_locked()). Martijn Coenen <maco@android.com> - 2017-08-31 10:20 +0200
  [PATCH v2 02/13] ANDROID: binder: push new transactions to waiting threads. Martijn Coenen <maco@android.com> - 2017-08-31 10:20 +0200

csiph-web