binder: make sure fd closes complete
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: 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:
parent
302e60e26a
commit
d5b0473707
@ -2236,6 +2236,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)
|
||||||
@ -2395,8 +2396,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:
|
||||||
@ -3465,7 +3474,7 @@ static void binder_transaction(struct binder_proc *proc,
|
|||||||
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);
|
||||||
@ -3542,7 +3551,9 @@ static void binder_transaction(struct binder_proc *proc,
|
|||||||
* 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) {
|
||||||
@ -3570,7 +3581,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);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -3771,7 +3782,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;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -4459,7 +4470,7 @@ static int binder_thread_read(struct binder_proc *proc,
|
|||||||
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,
|
||||||
|
Loading…
Reference in New Issue
Block a user