Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

binder: fix null deref of proc->context

The binder driver makes the assumption proc->context pointer is invariant after
initialization (as documented in the kerneldoc header for struct proc).
However, in commit f0fe2c0f050d ("binder: prevent UAF for binderfs devices II")
proc->context is set to NULL during binder_deferred_release().

Another proc was in the middle of setting up a transaction to the dying
process and crashed on a NULL pointer deref on "context" which is a local
set to &proc->context:

new_ref->data.desc = (node == context->binder_context_mgr_node) ? 0 : 1;

Here's the stack:

[ 5237.855435] Call trace:
[ 5237.855441] binder_get_ref_for_node_olocked+0x100/0x2ec
[ 5237.855446] binder_inc_ref_for_node+0x140/0x280
[ 5237.855451] binder_translate_binder+0x1d0/0x388
[ 5237.855456] binder_transaction+0x2228/0x3730
[ 5237.855461] binder_thread_write+0x640/0x25bc
[ 5237.855466] binder_ioctl_write_read+0xb0/0x464
[ 5237.855471] binder_ioctl+0x30c/0x96c
[ 5237.855477] do_vfs_ioctl+0x3e0/0x700
[ 5237.855482] __arm64_sys_ioctl+0x78/0xa4
[ 5237.855488] el0_svc_common+0xb4/0x194
[ 5237.855493] el0_svc_handler+0x74/0x98
[ 5237.855497] el0_svc+0x8/0xc

The fix is to move the kfree of the binder_device to binder_free_proc()
so the binder_device is freed when we know there are no references
remaining on the binder_proc.

Fixes: f0fe2c0f050d ("binder: prevent UAF for binderfs devices II")
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Todd Kjos <tkjos@google.com>
Cc: stable <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20200622200715.114382-1-tkjos@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Todd Kjos and committed by
Greg Kroah-Hartman
d35d3660 48778464

+7 -7
+7 -7
drivers/android/binder.c
··· 4686 4686 4687 4687 static void binder_free_proc(struct binder_proc *proc) 4688 4688 { 4689 + struct binder_device *device; 4690 + 4689 4691 BUG_ON(!list_empty(&proc->todo)); 4690 4692 BUG_ON(!list_empty(&proc->delivered_death)); 4693 + device = container_of(proc->context, struct binder_device, context); 4694 + if (refcount_dec_and_test(&device->ref)) { 4695 + kfree(proc->context->name); 4696 + kfree(device); 4697 + } 4691 4698 binder_alloc_deferred_release(&proc->alloc); 4692 4699 put_task_struct(proc->tsk); 4693 4700 binder_stats_deleted(BINDER_STAT_PROC); ··· 5413 5406 static void binder_deferred_release(struct binder_proc *proc) 5414 5407 { 5415 5408 struct binder_context *context = proc->context; 5416 - struct binder_device *device; 5417 5409 struct rb_node *n; 5418 5410 int threads, nodes, incoming_refs, outgoing_refs, active_transactions; 5419 5411 ··· 5429 5423 context->binder_context_mgr_node = NULL; 5430 5424 } 5431 5425 mutex_unlock(&context->context_mgr_node_lock); 5432 - device = container_of(proc->context, struct binder_device, context); 5433 - if (refcount_dec_and_test(&device->ref)) { 5434 - kfree(context->name); 5435 - kfree(device); 5436 - } 5437 - proc->context = NULL; 5438 5426 binder_inner_proc_lock(proc); 5439 5427 /* 5440 5428 * Make sure proc stays alive after we