binder: ensure binder_node has reference when starting transaction
When setting up a transaction, we look up a binder_ref based on the given handle and use the associated node as the target_node for the transaction. However, since we call binder_put_ref on the reference after discovering the node, there is a risk that the ref will be deleted -- and if that is the last ref on the underlying node, the node could also be deleted. When we call binder_inc_node(), it may enqueue work for the proc. There is a timing window where this could result in a corrupt list entry on the proc->todo list when the node is kfree'd. Bug: 35933974 Change-Id: Ie351c9e080a33b0cf0354f357876877f9d59784d Test: tested manually Signed-off-by: Todd Kjos <tkjos@google.com>
This commit is contained in:
committed by
Thierry Strudel
parent
8b8d920edd
commit
f120798fab
@@ -1762,6 +1762,7 @@ static void binder_transaction(struct binder_proc *proc,
|
||||
struct binder_proc *target_proc;
|
||||
struct binder_thread *target_thread = NULL;
|
||||
struct binder_node *target_node = NULL;
|
||||
struct binder_ref *target_ref = NULL;
|
||||
struct binder_worklist *target_list;
|
||||
wait_queue_head_t *target_wait;
|
||||
struct binder_transaction *in_reply_to = NULL;
|
||||
@@ -1828,17 +1829,15 @@ static void binder_transaction(struct binder_proc *proc,
|
||||
target_proc = target_thread->proc;
|
||||
} else {
|
||||
if (tr->target.handle) {
|
||||
struct binder_ref *ref;
|
||||
|
||||
ref = binder_get_ref(proc, tr->target.handle, true);
|
||||
if (ref == NULL) {
|
||||
target_ref = binder_get_ref(proc, tr->target.handle,
|
||||
true);
|
||||
if (target_ref == NULL) {
|
||||
binder_user_error("%d:%d got transaction to invalid handle\n",
|
||||
proc->pid, thread->pid);
|
||||
return_error = BR_FAILED_REPLY;
|
||||
goto err_invalid_target_handle;
|
||||
}
|
||||
target_node = ref->node;
|
||||
binder_put_ref(ref);
|
||||
target_node = target_ref->node;
|
||||
} else {
|
||||
target_node = context->binder_context_mgr_node;
|
||||
if (target_node == NULL) {
|
||||
@@ -1954,8 +1953,12 @@ static void binder_transaction(struct binder_proc *proc,
|
||||
t->buffer->transaction = t;
|
||||
t->buffer->target_node = target_node;
|
||||
trace_binder_transaction_alloc_buf(t->buffer);
|
||||
if (target_node)
|
||||
if (target_node) {
|
||||
binder_inc_node(target_node, 1, 0, NULL);
|
||||
if (target_ref)
|
||||
binder_put_ref(target_ref);
|
||||
target_ref = NULL;
|
||||
}
|
||||
|
||||
off_start = (binder_size_t *)(t->buffer->data +
|
||||
ALIGN(tr->data_size, sizeof(void *)));
|
||||
@@ -2196,6 +2199,9 @@ err_empty_call_stack:
|
||||
err_dead_binder:
|
||||
err_invalid_target_handle:
|
||||
err_no_context_mgr_node:
|
||||
if (target_ref)
|
||||
binder_put_ref(target_ref);
|
||||
|
||||
binder_debug(BINDER_DEBUG_FAILED_TRANSACTION,
|
||||
"%d:%d transaction failed %d, size %lld-%lld\n",
|
||||
proc->pid, thread->pid, return_error,
|
||||
|
||||
Reference in New Issue
Block a user