FROMLIST: binder: avoid race conditions when enqueuing txn
authorTodd Kjos <tkjos@google.com>
Tue, 9 May 2017 15:31:32 +0000 (08:31 -0700)
committerTodd Kjos <tkjos@google.com>
Thu, 13 Jul 2017 15:34:19 +0000 (08:34 -0700)
(from https://patchwork.kernel.org/patch/9817813/)

Currently, the transaction complete work item is queued
after the transaction. This means that it is possible
for the transaction to be handled and a reply to be
enqueued in the current thread before the transaction
complete is enqueued, which violates the protocol
with userspace who may not expect the transaction
complete. Fixed by always enqueing the transaction
complete first.

Also, once the transaction is enqueued, it is unsafe
to access since it might be freed. Currently,
t->flags is accessed to determine whether a sync
wake is needed. Changed to access tr->flags
instead.

Change-Id: I247f25a66cfeac8a1fcb2ad65c6053d51cafe4f3
Signed-off-by: Todd Kjos <tkjos@google.com>
drivers/android/binder.c

index ae64dc46b6f9e43bf439ac79d0e95ef2d3da1077..0cd37910ff83c058ebd787460d04186080729462 100644 (file)
@@ -1799,6 +1799,9 @@ static void binder_transaction(struct binder_proc *proc,
                        goto err_bad_object_type;
                }
        }
+       tcomplete->type = BINDER_WORK_TRANSACTION_COMPLETE;
+       list_add_tail(&tcomplete->entry, &thread->todo);
+
        if (reply) {
                BUG_ON(t->buffer->async_transaction != 0);
                binder_pop_transaction(target_thread, in_reply_to);
@@ -1818,10 +1821,8 @@ static void binder_transaction(struct binder_proc *proc,
        }
        t->work.type = BINDER_WORK_TRANSACTION;
        list_add_tail(&t->work.entry, target_list);
-       tcomplete->type = BINDER_WORK_TRANSACTION_COMPLETE;
-       list_add_tail(&tcomplete->entry, &thread->todo);
        if (target_wait) {
-               if (reply || !(t->flags & TF_ONE_WAY))
+               if (reply || !(tr->flags & TF_ONE_WAY))
                        wake_up_interruptible_sync(target_wait);
                else
                        wake_up_interruptible(target_wait);