binder: make sure fd closes complete

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: 80cd795630 ("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>
This commit is contained in:
Todd Kjos 2021-08-30 12:51:46 -07:00 committed by Greg Kroah-Hartman
parent b564171ade
commit 5fdb55c1ac

View File

@ -1852,6 +1852,7 @@ static void binder_deferred_fd_close(int fd)
} }
static void binder_transaction_buffer_release(struct binder_proc *proc, static void binder_transaction_buffer_release(struct binder_proc *proc,
struct binder_thread *thread,
struct binder_buffer *buffer, struct binder_buffer *buffer,
binder_size_t failed_at, binder_size_t failed_at,
bool is_failure) bool is_failure)
@ -2011,8 +2012,16 @@ static void binder_transaction_buffer_release(struct binder_proc *proc,
&proc->alloc, &fd, buffer, &proc->alloc, &fd, buffer,
offset, sizeof(fd)); offset, sizeof(fd));
WARN_ON(err); WARN_ON(err);
if (!err) if (!err) {
binder_deferred_fd_close(fd); 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; } break;
default: default:
@ -3104,7 +3113,7 @@ err_bad_parent:
err_copy_data_failed: err_copy_data_failed:
binder_free_txn_fixups(t); binder_free_txn_fixups(t);
trace_binder_transaction_failed_buffer_release(t->buffer); 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); buffer_offset, true);
if (target_node) if (target_node)
binder_dec_node_tmpref(target_node); binder_dec_node_tmpref(target_node);
@ -3183,7 +3192,9 @@ err_invalid_target_handle:
* Cleanup buffer and free it. * Cleanup buffer and free it.
*/ */
static void 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); binder_inner_proc_lock(proc);
if (buffer->transaction) { if (buffer->transaction) {
@ -3211,7 +3222,7 @@ binder_free_buf(struct binder_proc *proc, struct binder_buffer *buffer)
binder_node_inner_unlock(buf_node); binder_node_inner_unlock(buf_node);
} }
trace_binder_transaction_buffer_release(buffer); 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); binder_alloc_free_buf(&proc->alloc, buffer);
} }
@ -3413,7 +3424,7 @@ static int binder_thread_write(struct binder_proc *proc,
proc->pid, thread->pid, (u64)data_ptr, proc->pid, thread->pid, (u64)data_ptr,
buffer->debug_id, buffer->debug_id,
buffer->transaction ? "active" : "finished"); buffer->transaction ? "active" : "finished");
binder_free_buf(proc, buffer); binder_free_buf(proc, thread, buffer);
break; break;
} }
@ -4106,7 +4117,7 @@ retry:
buffer->transaction = NULL; buffer->transaction = NULL;
binder_cleanup_transaction(t, "fd fixups failed", binder_cleanup_transaction(t, "fd fixups failed",
BR_FAILED_REPLY); BR_FAILED_REPLY);
binder_free_buf(proc, buffer); binder_free_buf(proc, thread, buffer);
binder_debug(BINDER_DEBUG_FAILED_TRANSACTION, binder_debug(BINDER_DEBUG_FAILED_TRANSACTION,
"%d:%d %stransaction %d fd fixups failed %d/%d, line %d\n", "%d:%d %stransaction %d fd fixups failed %d/%d, line %d\n",
proc->pid, thread->pid, proc->pid, thread->pid,