[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220802081320.kbrnqnzlectmcb3y@wittgenstein>
Date: Tue, 2 Aug 2022 10:13:20 +0200
From: Christian Brauner <brauner@...nel.org>
To: Carlos Llamas <cmllamas@...gle.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Arve Hjønnevåg <arve@...roid.com>,
Todd Kjos <tkjos@...roid.com>,
Martijn Coenen <maco@...roid.com>,
Joel Fernandes <joel@...lfernandes.org>,
Suren Baghdasaryan <surenb@...gle.com>,
kernel-team@...roid.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] binder: fix UAF of ref->proc caused by race condition
On Mon, Aug 01, 2022 at 06:25:11PM +0000, Carlos Llamas wrote:
> 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
>
> Signed-off-by: Carlos Llamas <cmllamas@...gle.com>
> ---
Seems sane,
Acked-by: Christian Brauner (Microsoft) <brauner@...nel.org>
Powered by blists - more mailing lists