From 04841d3e70dd128ff5f10b0874142b2ff92fdaaf Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 3 Dec 2018 19:02:37 +0100 Subject: [PATCH] 4.14-stable patches added patches: binder-fix-race-that-allows-malicious-free-of-live-buffer.patch --- ...allows-malicious-free-of-live-buffer.patch | 117 ++++++++++++++++++ queue-4.14/series | 1 + 2 files changed, 118 insertions(+) create mode 100644 queue-4.14/binder-fix-race-that-allows-malicious-free-of-live-buffer.patch diff --git a/queue-4.14/binder-fix-race-that-allows-malicious-free-of-live-buffer.patch b/queue-4.14/binder-fix-race-that-allows-malicious-free-of-live-buffer.patch new file mode 100644 index 00000000000..d912f2b697c --- /dev/null +++ b/queue-4.14/binder-fix-race-that-allows-malicious-free-of-live-buffer.patch @@ -0,0 +1,117 @@ +From 7bada55ab50697861eee6bb7d60b41e68a961a9c Mon Sep 17 00:00:00 2001 +From: Todd Kjos +Date: Tue, 6 Nov 2018 15:55:32 -0800 +Subject: binder: fix race that allows malicious free of live buffer +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +From: Todd Kjos + +commit 7bada55ab50697861eee6bb7d60b41e68a961a9c upstream. + +Malicious code can attempt to free buffers using the BC_FREE_BUFFER +ioctl to binder. There are protections against a user freeing a buffer +while in use by the kernel, however there was a window where +BC_FREE_BUFFER could be used to free a recently allocated buffer that +was not completely initialized. This resulted in a use-after-free +detected by KASAN with a malicious test program. + +This window is closed by setting the buffer's allow_user_free attribute +to 0 when the buffer is allocated or when the user has previously freed +it instead of waiting for the caller to set it. The problem was that +when the struct buffer was recycled, allow_user_free was stale and set +to 1 allowing a free to go through. + +Signed-off-by: Todd Kjos +Acked-by: Arve Hjønnevåg +Cc: stable # 4.14 +Signed-off-by: Greg Kroah-Hartman + + +--- + drivers/android/binder.c | 21 ++++++++++++--------- + drivers/android/binder_alloc.c | 14 ++++++-------- + drivers/android/binder_alloc.h | 3 +-- + 3 files changed, 19 insertions(+), 19 deletions(-) + +--- a/drivers/android/binder.c ++++ b/drivers/android/binder.c +@@ -2918,7 +2918,6 @@ static void binder_transaction(struct bi + t->buffer = NULL; + goto err_binder_alloc_buf_failed; + } +- t->buffer->allow_user_free = 0; + t->buffer->debug_id = t->debug_id; + t->buffer->transaction = t; + t->buffer->target_node = target_node; +@@ -3407,14 +3406,18 @@ static int binder_thread_write(struct bi + + buffer = binder_alloc_prepare_to_free(&proc->alloc, + data_ptr); +- if (buffer == NULL) { +- binder_user_error("%d:%d BC_FREE_BUFFER u%016llx no match\n", +- proc->pid, thread->pid, (u64)data_ptr); +- break; +- } +- if (!buffer->allow_user_free) { +- binder_user_error("%d:%d BC_FREE_BUFFER u%016llx matched unreturned buffer\n", +- proc->pid, thread->pid, (u64)data_ptr); ++ if (IS_ERR_OR_NULL(buffer)) { ++ if (PTR_ERR(buffer) == -EPERM) { ++ binder_user_error( ++ "%d:%d BC_FREE_BUFFER u%016llx matched unreturned or currently freeing buffer\n", ++ proc->pid, thread->pid, ++ (u64)data_ptr); ++ } else { ++ binder_user_error( ++ "%d:%d BC_FREE_BUFFER u%016llx no match\n", ++ proc->pid, thread->pid, ++ (u64)data_ptr); ++ } + break; + } + binder_debug(BINDER_DEBUG_FREE_BUFFER, +--- a/drivers/android/binder_alloc.c ++++ b/drivers/android/binder_alloc.c +@@ -149,14 +149,12 @@ static struct binder_buffer *binder_allo + else { + /* + * Guard against user threads attempting to +- * free the buffer twice ++ * free the buffer when in use by kernel or ++ * after it's already been freed. + */ +- if (buffer->free_in_progress) { +- pr_err("%d:%d FREE_BUFFER u%016llx user freed buffer twice\n", +- alloc->pid, current->pid, (u64)user_ptr); +- return NULL; +- } +- buffer->free_in_progress = 1; ++ if (!buffer->allow_user_free) ++ return ERR_PTR(-EPERM); ++ buffer->allow_user_free = 0; + return buffer; + } + } +@@ -486,7 +484,7 @@ struct binder_buffer *binder_alloc_new_b + + rb_erase(best_fit, &alloc->free_buffers); + buffer->free = 0; +- buffer->free_in_progress = 0; ++ buffer->allow_user_free = 0; + binder_insert_allocated_buffer_locked(alloc, buffer); + binder_alloc_debug(BINDER_DEBUG_BUFFER_ALLOC, + "%d: binder_alloc_buf size %zd got %pK\n", +--- a/drivers/android/binder_alloc.h ++++ b/drivers/android/binder_alloc.h +@@ -50,8 +50,7 @@ struct binder_buffer { + unsigned free:1; + unsigned allow_user_free:1; + unsigned async_transaction:1; +- unsigned free_in_progress:1; +- unsigned debug_id:28; ++ unsigned debug_id:29; + + struct binder_transaction *transaction; + diff --git a/queue-4.14/series b/queue-4.14/series index 232a659d57c..375a102ef9c 100644 --- a/queue-4.14/series +++ b/queue-4.14/series @@ -143,3 +143,4 @@ lib-test_kmod.c-fix-rmmod-double-free.patch mm-use-swp_offset-as-key-in-shmem_replace_page.patch drivers-hv-vmbus-check-the-creation_status-in-vmbus_establish_gpadl.patch misc-mic-scif-fix-copy-paste-error-in-scif_create_remote_lookup.patch +binder-fix-race-that-allows-malicious-free-of-live-buffer.patch -- 2.47.3