lkml.org 
[lkml]   [2022]   [Sep]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.19 080/155] binder: fix UAF of ref->proc caused by race condition
    Date
    From: Carlos Llamas <cmllamas@google.com>

    commit a0e44c64b6061dda7e00b7c458e4523e2331b739 upstream.

    A transaction of type BINDER_TYPE_WEAK_HANDLE can fail to increment the
    reference for a node. In this case, the target proc normally releases
    the failed reference upon close as expected. However, if the target is
    dying in parallel the call will race with binder_deferred_release(), so
    the target could have released all of its references by now leaving the
    cleanup of the new failed reference unhandled.

    The transaction then ends and the target proc gets released making the
    ref->proc now a dangling pointer. Later on, ref->node is closed and we
    attempt to take spin_lock(&ref->proc->inner_lock), which leads to the
    use-after-free bug reported below. Let's fix this by cleaning up the
    failed reference on the spot instead of relying on the target to do so.

    ==================================================================
    BUG: KASAN: use-after-free in _raw_spin_lock+0xa8/0x150
    Write of size 4 at addr ffff5ca207094238 by task kworker/1:0/590

    CPU: 1 PID: 590 Comm: kworker/1:0 Not tainted 5.19.0-rc8 #10
    Hardware name: linux,dummy-virt (DT)
    Workqueue: events binder_deferred_func
    Call trace:
    dump_backtrace.part.0+0x1d0/0x1e0
    show_stack+0x18/0x70
    dump_stack_lvl+0x68/0x84
    print_report+0x2e4/0x61c
    kasan_report+0xa4/0x110
    kasan_check_range+0xfc/0x1a4
    __kasan_check_write+0x3c/0x50
    _raw_spin_lock+0xa8/0x150
    binder_deferred_func+0x5e0/0x9b0
    process_one_work+0x38c/0x5f0
    worker_thread+0x9c/0x694
    kthread+0x188/0x190
    ret_from_fork+0x10/0x20

    Acked-by: Christian Brauner (Microsoft) <brauner@kernel.org>
    Signed-off-by: Carlos Llamas <cmllamas@google.com>
    Cc: stable <stable@kernel.org> # 4.14+
    Link: https://lore.kernel.org/r/20220801182511.3371447-1-cmllamas@google.com
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    drivers/android/binder.c | 12 ++++++++++++
    1 file changed, 12 insertions(+)

    diff --git a/drivers/android/binder.c b/drivers/android/binder.c
    index c964d7c8c384..6428f6be69e3 100644
    --- a/drivers/android/binder.c
    +++ b/drivers/android/binder.c
    @@ -1385,6 +1385,18 @@ static int binder_inc_ref_for_node(struct binder_proc *proc,
    }
    ret = binder_inc_ref_olocked(ref, strong, target_list);
    *rdata = ref->data;
    + if (ret && ref == new_ref) {
    + /*
    + * Cleanup the failed reference here as the target
    + * could now be dead and have already released its
    + * references by now. Calling on the new reference
    + * with strong=0 and a tmp_refs will not decrement
    + * the node. The new_ref gets kfree'd below.
    + */
    + binder_cleanup_ref_olocked(new_ref);
    + ref = NULL;
    + }
    +
    binder_proc_unlock(proc);
    if (new_ref && ref != new_ref)
    /*
    --
    2.37.3


    \
     
     \ /
      Last update: 2022-09-06 16:12    [W:4.068 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site