Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753869AbdF2TKA (ORCPT ); Thu, 29 Jun 2017 15:10:00 -0400 Received: from mail-pg0-f46.google.com ([74.125.83.46]:36131 "EHLO mail-pg0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753107AbdF2TDU (ORCPT ); Thu, 29 Jun 2017 15:03:20 -0400 From: Todd Kjos X-Google-Original-From: Todd Kjos To: gregkh@linuxfoundation.org, arve@android.com, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, maco@google.com, tkjos@google.com Subject: [PATCH 14/37] binder: avoid race conditions when enqueuing txn Date: Thu, 29 Jun 2017 12:01:48 -0700 Message-Id: <20170629190211.16927-15-tkjos@google.com> X-Mailer: git-send-email 2.13.2.725.g09c95d1e9-goog In-Reply-To: <20170629190211.16927-1-tkjos@google.com> References: <20170629190211.16927-1-tkjos@google.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1737 Lines: 48 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. Signed-off-by: Todd Kjos --- drivers/android/binder.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index f17d1dfa5b02..71faf548482d 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -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); -- 2.13.2.725.g09c95d1e9-goog