| From 5fdb55c1ac9585eb23bb2541d5819224429e103d Mon Sep 17 00:00:00 2001 |
| From: Todd Kjos <tkjos@google.com> |
| Date: Mon, 30 Aug 2021 12:51:46 -0700 |
| Subject: binder: make sure fd closes complete |
| |
| From: Todd Kjos <tkjos@google.com> |
| |
| commit 5fdb55c1ac9585eb23bb2541d5819224429e103d upstream. |
| |
| During BC_FREE_BUFFER processing, the BINDER_TYPE_FDA object |
| cleanup may close 1 or more fds. The close operations are |
| completed using the task work mechanism -- which means the thread |
| needs to return to userspace or the file object may never be |
| dereferenced -- which can lead to hung processes. |
| |
| Force the binder thread back to userspace if an fd is closed during |
| BC_FREE_BUFFER handling. |
| |
| Fixes: 80cd795630d6 ("binder: fix use-after-free due to ksys_close() during fdget()") |
| Cc: stable <stable@vger.kernel.org> |
| Reviewed-by: Martijn Coenen <maco@android.com> |
| Acked-by: Christian Brauner <christian.brauner@ubuntu.com> |
| Signed-off-by: Todd Kjos <tkjos@google.com> |
| Link: https://lore.kernel.org/r/20210830195146.587206-1-tkjos@google.com |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/android/binder.c | 23 +++++++++++++++++------ |
| 1 file changed, 17 insertions(+), 6 deletions(-) |
| |
| --- a/drivers/android/binder.c |
| +++ b/drivers/android/binder.c |
| @@ -2239,6 +2239,7 @@ 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, |
| bool is_failure) |
| @@ -2398,8 +2399,16 @@ static void binder_transaction_buffer_re |
| &proc->alloc, &fd, buffer, |
| offset, sizeof(fd)); |
| WARN_ON(err); |
| - if (!err) |
| + if (!err) { |
| binder_deferred_fd_close(fd); |
| + /* |
| + * Need to make sure the thread goes |
| + * back to userspace to complete the |
| + * deferred close |
| + */ |
| + if (thread) |
| + thread->looper_need_return = true; |
| + } |
| } |
| } break; |
| default: |
| @@ -3469,7 +3478,7 @@ err_bad_parent: |
| err_copy_data_failed: |
| binder_free_txn_fixups(t); |
| trace_binder_transaction_failed_buffer_release(t->buffer); |
| - binder_transaction_buffer_release(target_proc, t->buffer, |
| + binder_transaction_buffer_release(target_proc, NULL, t->buffer, |
| buffer_offset, true); |
| if (target_node) |
| binder_dec_node_tmpref(target_node); |
| @@ -3546,7 +3555,9 @@ err_invalid_target_handle: |
| * Cleanup buffer and free it. |
| */ |
| static void |
| -binder_free_buf(struct binder_proc *proc, struct binder_buffer *buffer) |
| +binder_free_buf(struct binder_proc *proc, |
| + struct binder_thread *thread, |
| + struct binder_buffer *buffer) |
| { |
| binder_inner_proc_lock(proc); |
| if (buffer->transaction) { |
| @@ -3574,7 +3585,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, buffer, 0, false); |
| + binder_transaction_buffer_release(proc, thread, buffer, 0, false); |
| binder_alloc_free_buf(&proc->alloc, buffer); |
| } |
| |
| @@ -3775,7 +3786,7 @@ static int binder_thread_write(struct bi |
| proc->pid, thread->pid, (u64)data_ptr, |
| buffer->debug_id, |
| buffer->transaction ? "active" : "finished"); |
| - binder_free_buf(proc, buffer); |
| + binder_free_buf(proc, thread, buffer); |
| break; |
| } |
| |
| @@ -4463,7 +4474,7 @@ retry: |
| buffer->transaction = NULL; |
| binder_cleanup_transaction(t, "fd fixups failed", |
| BR_FAILED_REPLY); |
| - binder_free_buf(proc, buffer); |
| + binder_free_buf(proc, thread, buffer); |
| binder_debug(BINDER_DEBUG_FAILED_TRANSACTION, |
| "%d:%d %stransaction %d fd fixups failed %d/%d, line %d\n", |
| proc->pid, thread->pid, |