From: Todd Kjos <tkjos@android.com> To: tkjos@google.com, gregkh@linuxfoundation.org, arve@android.com, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, maco@google.com Cc: joel@joelfernandes.org, kernel-team@android.com Subject: [PATCH] binder: fix possible UAF when freeing buffer Date: Wed, 12 Jun 2019 13:29:27 -0700 [thread overview] Message-ID: <20190612202927.54518-1-tkjos@google.com> (raw) There is a race between the binder driver cleaning up a completed transaction via binder_free_transaction() and a user calling binder_ioctl(BC_FREE_BUFFER) to release a buffer. It doesn't matter which is first but they need to be protected against running concurrently which can result in a UAF. Signed-off-by: Todd Kjos <tkjos@google.com> --- drivers/android/binder.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 748ac489ef7eb..bc26b5511f0a9 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -1941,8 +1941,18 @@ static void binder_free_txn_fixups(struct binder_transaction *t) static void binder_free_transaction(struct binder_transaction *t) { - if (t->buffer) - t->buffer->transaction = NULL; + struct binder_proc *target_proc = t->to_proc; + + if (target_proc) { + binder_inner_proc_lock(target_proc); + if (t->buffer) + t->buffer->transaction = NULL; + binder_inner_proc_unlock(target_proc); + } + /* + * If the transaction has no target_proc, then + * t->buffer->transaction has already been cleared. + */ binder_free_txn_fixups(t); kfree(t); binder_stats_deleted(BINDER_STAT_TRANSACTION); @@ -3551,10 +3561,12 @@ static void binder_transaction(struct binder_proc *proc, static void binder_free_buf(struct binder_proc *proc, struct binder_buffer *buffer) { + binder_inner_proc_lock(proc); if (buffer->transaction) { buffer->transaction->buffer = NULL; buffer->transaction = NULL; } + binder_inner_proc_unlock(proc); if (buffer->async_transaction && buffer->target_node) { struct binder_node *buf_node; struct binder_work *w; -- 2.22.0.rc2.383.gf4fbbf30c2-goog
next reply other threads:[~2019-06-12 20:29 UTC|newest] Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-12 20:29 Todd Kjos [this message] 2019-06-13 5:41 ` Greg KH 2019-06-13 15:43 ` Todd Kjos
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190612202927.54518-1-tkjos@google.com \ --to=tkjos@android.com \ --cc=arve@android.com \ --cc=devel@driverdev.osuosl.org \ --cc=gregkh@linuxfoundation.org \ --cc=joel@joelfernandes.org \ --cc=kernel-team@android.com \ --cc=linux-kernel@vger.kernel.org \ --cc=maco@google.com \ --cc=tkjos@google.com \ --subject='Re: [PATCH] binder: fix possible UAF when freeing buffer' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).