Re: [PATCH] binder: fix null deref of proc->context

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 22, 2020 at 4:07 PM 'Todd Kjos' via kernel-team
<kernel-team@xxxxxxxxxxx> wrote:
>
> 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")
> Signed-off-by: Todd Kjos <tkjos@xxxxxxxxxx>

Reviewed-by: Joel Fernandes (Google) <joel@xxxxxxxxxxxxxxxxx>

Thanks,

 - Joel



> ---
>  drivers/android/binder.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index e47c8a4c83db..f50c5f182bb5 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -4686,8 +4686,15 @@ static struct binder_thread *binder_get_thread(struct binder_proc *proc)
>
>  static void binder_free_proc(struct binder_proc *proc)
>  {
> +       struct binder_device *device;
> +
>         BUG_ON(!list_empty(&proc->todo));
>         BUG_ON(!list_empty(&proc->delivered_death));
> +       device = container_of(proc->context, struct binder_device, context);
> +       if (refcount_dec_and_test(&device->ref)) {
> +               kfree(proc->context->name);
> +               kfree(device);
> +       }
>         binder_alloc_deferred_release(&proc->alloc);
>         put_task_struct(proc->tsk);
>         binder_stats_deleted(BINDER_STAT_PROC);
> @@ -5406,7 +5413,6 @@ static int binder_node_release(struct binder_node *node, int refs)
>  static void binder_deferred_release(struct binder_proc *proc)
>  {
>         struct binder_context *context = proc->context;
> -       struct binder_device *device;
>         struct rb_node *n;
>         int threads, nodes, incoming_refs, outgoing_refs, active_transactions;
>
> @@ -5423,12 +5429,6 @@ static void binder_deferred_release(struct binder_proc *proc)
>                 context->binder_context_mgr_node = NULL;
>         }
>         mutex_unlock(&context->context_mgr_node_lock);
> -       device = container_of(proc->context, struct binder_device, context);
> -       if (refcount_dec_and_test(&device->ref)) {
> -               kfree(context->name);
> -               kfree(device);
> -       }
> -       proc->context = NULL;
>         binder_inner_proc_lock(proc);
>         /*
>          * Make sure proc stays alive after we
> --
> 2.27.0.111.gc72c7da667-goog
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@xxxxxxxxxxx.
>
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux