Commit 5f2f6369 authored by Todd Kjos's avatar Todd Kjos Committed by Greg Kroah-Hartman

binder: protect against stale pointers in print_binder_transaction

When printing transactions there were several race conditions
that could cause a stale pointer to be deferenced. Fixed by
reading the pointer once and using it if valid (which is
safe). The transaction buffer also needed protection via proc
lock, so it is only printed if we are holding the correct lock.
Signed-off-by: default avatarTodd Kjos <tkjos@google.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 2c1838dc
...@@ -4702,35 +4702,52 @@ binder_defer_work(struct binder_proc *proc, enum binder_deferred_state defer) ...@@ -4702,35 +4702,52 @@ binder_defer_work(struct binder_proc *proc, enum binder_deferred_state defer)
mutex_unlock(&binder_deferred_lock); mutex_unlock(&binder_deferred_lock);
} }
static void print_binder_transaction(struct seq_file *m, const char *prefix, static void print_binder_transaction_ilocked(struct seq_file *m,
struct binder_transaction *t) struct binder_proc *proc,
const char *prefix,
struct binder_transaction *t)
{ {
struct binder_proc *to_proc;
struct binder_buffer *buffer = t->buffer;
WARN_ON(!spin_is_locked(&proc->inner_lock));
spin_lock(&t->lock); spin_lock(&t->lock);
to_proc = t->to_proc;
seq_printf(m, seq_printf(m,
"%s %d: %p from %d:%d to %d:%d code %x flags %x pri %ld r%d", "%s %d: %p from %d:%d to %d:%d code %x flags %x pri %ld r%d",
prefix, t->debug_id, t, prefix, t->debug_id, t,
t->from ? t->from->proc->pid : 0, t->from ? t->from->proc->pid : 0,
t->from ? t->from->pid : 0, t->from ? t->from->pid : 0,
t->to_proc ? t->to_proc->pid : 0, to_proc ? to_proc->pid : 0,
t->to_thread ? t->to_thread->pid : 0, t->to_thread ? t->to_thread->pid : 0,
t->code, t->flags, t->priority, t->need_reply); t->code, t->flags, t->priority, t->need_reply);
spin_unlock(&t->lock); spin_unlock(&t->lock);
if (t->buffer == NULL) { if (proc != to_proc) {
/*
* Can only safely deref buffer if we are holding the
* correct proc inner lock for this node
*/
seq_puts(m, "\n");
return;
}
if (buffer == NULL) {
seq_puts(m, " buffer free\n"); seq_puts(m, " buffer free\n");
return; return;
} }
if (t->buffer->target_node) if (buffer->target_node)
seq_printf(m, " node %d", seq_printf(m, " node %d", buffer->target_node->debug_id);
t->buffer->target_node->debug_id);
seq_printf(m, " size %zd:%zd data %p\n", seq_printf(m, " size %zd:%zd data %p\n",
t->buffer->data_size, t->buffer->offsets_size, buffer->data_size, buffer->offsets_size,
t->buffer->data); buffer->data);
} }
static void print_binder_work_ilocked(struct seq_file *m, const char *prefix, static void print_binder_work_ilocked(struct seq_file *m,
const char *transaction_prefix, struct binder_proc *proc,
struct binder_work *w) const char *prefix,
const char *transaction_prefix,
struct binder_work *w)
{ {
struct binder_node *node; struct binder_node *node;
struct binder_transaction *t; struct binder_transaction *t;
...@@ -4738,7 +4755,8 @@ static void print_binder_work_ilocked(struct seq_file *m, const char *prefix, ...@@ -4738,7 +4755,8 @@ static void print_binder_work_ilocked(struct seq_file *m, const char *prefix,
switch (w->type) { switch (w->type) {
case BINDER_WORK_TRANSACTION: case BINDER_WORK_TRANSACTION:
t = container_of(w, struct binder_transaction, work); t = container_of(w, struct binder_transaction, work);
print_binder_transaction(m, transaction_prefix, t); print_binder_transaction_ilocked(
m, proc, transaction_prefix, t);
break; break;
case BINDER_WORK_RETURN_ERROR: { case BINDER_WORK_RETURN_ERROR: {
struct binder_error *e = container_of( struct binder_error *e = container_of(
...@@ -4789,20 +4807,21 @@ static void print_binder_thread_ilocked(struct seq_file *m, ...@@ -4789,20 +4807,21 @@ static void print_binder_thread_ilocked(struct seq_file *m,
t = thread->transaction_stack; t = thread->transaction_stack;
while (t) { while (t) {
if (t->from == thread) { if (t->from == thread) {
print_binder_transaction(m, print_binder_transaction_ilocked(m, thread->proc,
" outgoing transaction", t); " outgoing transaction", t);
t = t->from_parent; t = t->from_parent;
} else if (t->to_thread == thread) { } else if (t->to_thread == thread) {
print_binder_transaction(m, print_binder_transaction_ilocked(m, thread->proc,
" incoming transaction", t); " incoming transaction", t);
t = t->to_parent; t = t->to_parent;
} else { } else {
print_binder_transaction(m, " bad transaction", t); print_binder_transaction_ilocked(m, thread->proc,
" bad transaction", t);
t = NULL; t = NULL;
} }
} }
list_for_each_entry(w, &thread->todo, entry) { list_for_each_entry(w, &thread->todo, entry) {
print_binder_work_ilocked(m, " ", print_binder_work_ilocked(m, thread->proc, " ",
" pending transaction", w); " pending transaction", w);
} }
if (!print_always && m->count == header_pos) if (!print_always && m->count == header_pos)
...@@ -4837,7 +4856,7 @@ static void print_binder_node_nilocked(struct seq_file *m, ...@@ -4837,7 +4856,7 @@ static void print_binder_node_nilocked(struct seq_file *m,
seq_puts(m, "\n"); seq_puts(m, "\n");
if (node->proc) { if (node->proc) {
list_for_each_entry(w, &node->async_todo, entry) list_for_each_entry(w, &node->async_todo, entry)
print_binder_work_ilocked(m, " ", print_binder_work_ilocked(m, node->proc, " ",
" pending async transaction", w); " pending async transaction", w);
} }
} }
...@@ -4909,7 +4928,8 @@ static void print_binder_proc(struct seq_file *m, ...@@ -4909,7 +4928,8 @@ static void print_binder_proc(struct seq_file *m,
binder_alloc_print_allocated(m, &proc->alloc); binder_alloc_print_allocated(m, &proc->alloc);
binder_inner_proc_lock(proc); binder_inner_proc_lock(proc);
list_for_each_entry(w, &proc->todo, entry) list_for_each_entry(w, &proc->todo, entry)
print_binder_work_ilocked(m, " ", " pending transaction", w); print_binder_work_ilocked(m, proc, " ",
" pending transaction", w);
list_for_each_entry(w, &proc->delivered_death, entry) { list_for_each_entry(w, &proc->delivered_death, entry) {
seq_puts(m, " has delivered dead binder\n"); seq_puts(m, " has delivered dead binder\n");
break; break;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment