binder: fix possible UAF when freeing buffer

commit a370003cc301d4361bae20c9ef615f89bf8d1e8a upstream

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>
Cc: stable <stable@vger.kernel.org> # 4.14 4.19
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Change-Id: I1b9c9bdc52df8ddbc5fe7c6d8308f1068265f8ae

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 1fe5f0c6f392bb79971d89fb7865ab321067f743..1cae7d2015275932e9c0ceb6c1c7d01a3d08b7b6 100644 (file)
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2128,8 +2128,18 @@ static struct binder_thread *binder_get_txn_from_and_acq_inner(
 
 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.
+        */
        kfree(t);
        binder_stats_deleted(BINDER_STAT_TRANSACTION);
 }
@@ -3705,10 +3715,12 @@ int binder_thread_write(struct binder_proc *proc,
                                     buffer->debug_id,
                                     buffer->transaction ? "active" : "finished");
 
+                       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;