| From 32e9f56a96d8d0f23cb2aeb2a3cd18d40393e787 Mon Sep 17 00:00:00 2001 |
| From: Todd Kjos <tkjos@google.com> |
| Date: Fri, 15 Oct 2021 16:38:11 -0700 |
| Subject: binder: don't detect sender/target during buffer cleanup |
| |
| From: Todd Kjos <tkjos@google.com> |
| |
| commit 32e9f56a96d8d0f23cb2aeb2a3cd18d40393e787 upstream. |
| |
| When freeing txn buffers, binder_transaction_buffer_release() |
| attempts to detect whether the current context is the target by |
| comparing current->group_leader to proc->tsk. This is an unreliable |
| test. Instead explicitly pass an 'is_failure' boolean. |
| |
| Detecting the sender was being used as a way to tell if the |
| transaction failed to be sent. When cleaning up after |
| failing to send a transaction, there is no need to close |
| the fds associated with a BINDER_TYPE_FDA object. Now |
| 'is_failure' can be used to accurately detect this case. |
| |
| Fixes: 44d8047f1d87 ("binder: use standard functions to allocate fds") |
| Cc: stable <stable@vger.kernel.org> |
| Acked-by: Christian Brauner <christian.brauner@ubuntu.com> |
| Signed-off-by: Todd Kjos <tkjos@google.com> |
| Link: https://lore.kernel.org/r/20211015233811.3532235-1-tkjos@google.com |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/android/binder.c | 14 +++++++------- |
| 1 file changed, 7 insertions(+), 7 deletions(-) |
| |
| --- a/drivers/android/binder.c |
| +++ b/drivers/android/binder.c |
| @@ -2257,7 +2257,7 @@ static void binder_transaction_buffer_re |
| binder_dec_node(buffer->target_node, 1, 0); |
| |
| off_start_offset = ALIGN(buffer->data_size, sizeof(void *)); |
| - off_end_offset = is_failure ? failed_at : |
| + 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)) { |
| @@ -2343,9 +2343,8 @@ static void binder_transaction_buffer_re |
| binder_size_t fd_buf_size; |
| binder_size_t num_valid; |
| |
| - if (proc->tsk != current->group_leader) { |
| + if (is_failure) { |
| /* |
| - * Nothing to do if running in sender context |
| * The fd fixups have not been applied so no |
| * fds need to be closed. |
| */ |
| @@ -3548,6 +3547,7 @@ err_invalid_target_handle: |
| * binder_free_buf() - free the specified buffer |
| * @proc: binder proc that owns buffer |
| * @buffer: buffer to be freed |
| + * @is_failure: failed to send transaction |
| * |
| * If buffer for an async transaction, enqueue the next async |
| * transaction from the node. |
| @@ -3557,7 +3557,7 @@ err_invalid_target_handle: |
| static void |
| binder_free_buf(struct binder_proc *proc, |
| struct binder_thread *thread, |
| - struct binder_buffer *buffer) |
| + struct binder_buffer *buffer, bool is_failure) |
| { |
| binder_inner_proc_lock(proc); |
| if (buffer->transaction) { |
| @@ -3585,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, thread, buffer, 0, false); |
| + binder_transaction_buffer_release(proc, thread, buffer, 0, is_failure); |
| binder_alloc_free_buf(&proc->alloc, buffer); |
| } |
| |
| @@ -3786,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, thread, buffer); |
| + binder_free_buf(proc, thread, buffer, false); |
| break; |
| } |
| |
| @@ -4474,7 +4474,7 @@ retry: |
| buffer->transaction = NULL; |
| binder_cleanup_transaction(t, "fd fixups failed", |
| BR_FAILED_REPLY); |
| - binder_free_buf(proc, thread, buffer); |
| + binder_free_buf(proc, thread, buffer, true); |
| binder_debug(BINDER_DEBUG_FAILED_TRANSACTION, |
| "%d:%d %stransaction %d fd fixups failed %d/%d, line %d\n", |
| proc->pid, thread->pid, |