From db220a05533f1b066b427e50c25d09da8ab0db79 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 5 Sep 2022 18:32:14 +0200 Subject: [PATCH] 4.19-stable patches added patches: binder-fix-uaf-of-ref-proc-caused-by-race-condition.patch --- ...of-ref-proc-caused-by-race-condition.patch | 75 +++++++++++++++++++ queue-4.19/series | 1 + 2 files changed, 76 insertions(+) create mode 100644 queue-4.19/binder-fix-uaf-of-ref-proc-caused-by-race-condition.patch diff --git a/queue-4.19/binder-fix-uaf-of-ref-proc-caused-by-race-condition.patch b/queue-4.19/binder-fix-uaf-of-ref-proc-caused-by-race-condition.patch new file mode 100644 index 00000000000..8e47f35d661 --- /dev/null +++ b/queue-4.19/binder-fix-uaf-of-ref-proc-caused-by-race-condition.patch @@ -0,0 +1,75 @@ +From a0e44c64b6061dda7e00b7c458e4523e2331b739 Mon Sep 17 00:00:00 2001 +From: Carlos Llamas +Date: Mon, 1 Aug 2022 18:25:11 +0000 +Subject: binder: fix UAF of ref->proc caused by race condition + +From: Carlos Llamas + +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) +Signed-off-by: Carlos Llamas +Cc: stable # 4.14+ +Link: https://lore.kernel.org/r/20220801182511.3371447-1-cmllamas@google.com +Signed-off-by: Greg Kroah-Hartman +Signed-off-by: Greg Kroah-Hartman +--- + drivers/android/binder.c | 12 ++++++++++++ + 1 file changed, 12 insertions(+) + +--- a/drivers/android/binder.c ++++ b/drivers/android/binder.c +@@ -1809,6 +1809,18 @@ static int binder_inc_ref_for_node(struc + } + 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) + /* diff --git a/queue-4.19/series b/queue-4.19/series index 377b798af4e..26add73e153 100644 --- a/queue-4.19/series +++ b/queue-4.19/series @@ -14,3 +14,4 @@ serial-fsl_lpuart-rs485-rts-polariy-is-inverse.patch staging-rtl8712-fix-use-after-free-bugs.patch vt-clear-selection-before-changing-the-font.patch usb-serial-ftdi_sio-add-omron-cs1w-cif31-device-id.patch +binder-fix-uaf-of-ref-proc-caused-by-race-condition.patch -- 2.47.2