--- /dev/null
+From bdc1c5fac982845a58d28690cdb56db8c88a530d Mon Sep 17 00:00:00 2001
+From: Carlos Llamas <cmllamas@google.com>
+Date: Fri, 5 May 2023 20:30:20 +0000
+Subject: binder: fix UAF caused by faulty buffer cleanup
+
+From: Carlos Llamas <cmllamas@google.com>
+
+commit bdc1c5fac982845a58d28690cdb56db8c88a530d upstream.
+
+In binder_transaction_buffer_release() the 'failed_at' offset indicates
+the number of objects to clean up. However, this function was changed by
+commit 44d8047f1d87 ("binder: use standard functions to allocate fds"),
+to release all the objects in the buffer when 'failed_at' is zero.
+
+This introduced an issue when a transaction buffer is released without
+any objects having been processed so far. In this case, 'failed_at' is
+indeed zero yet it is misinterpreted as releasing the entire buffer.
+
+This leads to use-after-free errors where nodes are incorrectly freed
+and subsequently accessed. Such is the case in the following KASAN
+report:
+
+ ==================================================================
+ BUG: KASAN: slab-use-after-free in binder_thread_read+0xc40/0x1f30
+ Read of size 8 at addr ffff4faf037cfc58 by task poc/474
+
+ CPU: 6 PID: 474 Comm: poc Not tainted 6.3.0-12570-g7df047b3f0aa #5
+ Hardware name: linux,dummy-virt (DT)
+ Call trace:
+ dump_backtrace+0x94/0xec
+ show_stack+0x18/0x24
+ dump_stack_lvl+0x48/0x60
+ print_report+0xf8/0x5b8
+ kasan_report+0xb8/0xfc
+ __asan_load8+0x9c/0xb8
+ binder_thread_read+0xc40/0x1f30
+ binder_ioctl+0xd9c/0x1768
+ __arm64_sys_ioctl+0xd4/0x118
+ invoke_syscall+0x60/0x188
+ [...]
+
+ Allocated by task 474:
+ kasan_save_stack+0x3c/0x64
+ kasan_set_track+0x2c/0x40
+ kasan_save_alloc_info+0x24/0x34
+ __kasan_kmalloc+0xb8/0xbc
+ kmalloc_trace+0x48/0x5c
+ binder_new_node+0x3c/0x3a4
+ binder_transaction+0x2b58/0x36f0
+ binder_thread_write+0x8e0/0x1b78
+ binder_ioctl+0x14a0/0x1768
+ __arm64_sys_ioctl+0xd4/0x118
+ invoke_syscall+0x60/0x188
+ [...]
+
+ Freed by task 475:
+ kasan_save_stack+0x3c/0x64
+ kasan_set_track+0x2c/0x40
+ kasan_save_free_info+0x38/0x5c
+ __kasan_slab_free+0xe8/0x154
+ __kmem_cache_free+0x128/0x2bc
+ kfree+0x58/0x70
+ binder_dec_node_tmpref+0x178/0x1fc
+ binder_transaction_buffer_release+0x430/0x628
+ binder_transaction+0x1954/0x36f0
+ binder_thread_write+0x8e0/0x1b78
+ binder_ioctl+0x14a0/0x1768
+ __arm64_sys_ioctl+0xd4/0x118
+ invoke_syscall+0x60/0x188
+ [...]
+ ==================================================================
+
+In order to avoid these issues, let's always calculate the intended
+'failed_at' offset beforehand. This is renamed and wrapped in a helper
+function to make it clear and convenient.
+
+Fixes: 32e9f56a96d8 ("binder: don't detect sender/target during buffer cleanup")
+Reported-by: Zi Fan Tan <zifantan@google.com>
+Cc: stable@vger.kernel.org
+Signed-off-by: Carlos Llamas <cmllamas@google.com>
+Acked-by: Todd Kjos <tkjos@google.com>
+Link: https://lore.kernel.org/r/20230505203020.4101154-1-cmllamas@google.com
+[cmllamas: resolve trivial conflict due to missing commit 9864bb4801331]
+Signed-off-by: Carlos Llamas <cmllamas@google.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ drivers/android/binder.c | 26 ++++++++++++++++++++------
+ 1 file changed, 20 insertions(+), 6 deletions(-)
+
+--- a/drivers/android/binder.c
++++ b/drivers/android/binder.c
+@@ -2270,24 +2270,23 @@ static void binder_deferred_fd_close(int
+ static void binder_transaction_buffer_release(struct binder_proc *proc,
+ struct binder_thread *thread,
+ struct binder_buffer *buffer,
+- binder_size_t failed_at,
++ binder_size_t off_end_offset,
+ bool is_failure)
+ {
+ int debug_id = buffer->debug_id;
+- binder_size_t off_start_offset, buffer_offset, off_end_offset;
++ binder_size_t off_start_offset, buffer_offset;
+
+ binder_debug(BINDER_DEBUG_TRANSACTION,
+ "%d buffer release %d, size %zd-%zd, failed at %llx\n",
+ proc->pid, buffer->debug_id,
+ buffer->data_size, buffer->offsets_size,
+- (unsigned long long)failed_at);
++ (unsigned long long)off_end_offset);
+
+ if (buffer->target_node)
+ binder_dec_node(buffer->target_node, 1, 0);
+
+ off_start_offset = ALIGN(buffer->data_size, sizeof(void *));
+- off_end_offset = is_failure && failed_at ? failed_at :
+- off_start_offset + buffer->offsets_size;
++
+ for (buffer_offset = off_start_offset; buffer_offset < off_end_offset;
+ buffer_offset += sizeof(binder_size_t)) {
+ struct binder_object_header *hdr;
+@@ -2447,6 +2446,21 @@ static void binder_transaction_buffer_re
+ }
+ }
+
++/* Clean up all the objects in the buffer */
++static inline void binder_release_entire_buffer(struct binder_proc *proc,
++ struct binder_thread *thread,
++ struct binder_buffer *buffer,
++ bool is_failure)
++{
++ binder_size_t off_end_offset;
++
++ off_end_offset = ALIGN(buffer->data_size, sizeof(void *));
++ off_end_offset += buffer->offsets_size;
++
++ binder_transaction_buffer_release(proc, thread, buffer,
++ off_end_offset, is_failure);
++}
++
+ static int binder_translate_binder(struct flat_binder_object *fp,
+ struct binder_transaction *t,
+ struct binder_thread *thread)
+@@ -3930,7 +3944,7 @@ binder_free_buf(struct binder_proc *proc
+ binder_node_inner_unlock(buf_node);
+ }
+ trace_binder_transaction_buffer_release(buffer);
+- binder_transaction_buffer_release(proc, thread, buffer, 0, is_failure);
++ binder_release_entire_buffer(proc, thread, buffer, is_failure);
+ binder_alloc_free_buf(&proc->alloc, buffer);
+ }
+
projects / thirdparty / kernel / stable-queue.git / commitdiff
