From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8A09BC352B9 for ; Sat, 10 Oct 2020 23:09:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5B52520760 for ; Sat, 10 Oct 2020 23:09:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1602371349; bh=dyGEdyrR3w2vQm2MV9dBBgesUJKrPJrgJAkMuyI+7Mg=; h=Subject:To:From:Date:List-ID:From; b=gepDnxwB0aH9pqLcZk81+netPRPEvBQAOVxbgiZdzOToTNFvy6GmKOPoWXP23GaE5 Z1KqfYDdyaSEfOaBKxm2KtntpJZG1J8pqTrqajW2biqR63ZHydKmpySc89AW4+UU/P SxDSZ3E+tOgJ3eGeaNrAodzxxtIB/Hn4+0sN60Fc= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390321AbgJJW4z (ORCPT ); Sat, 10 Oct 2020 18:56:55 -0400 Received: from mail.kernel.org ([198.145.29.99]:56976 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731085AbgJJTw1 (ORCPT ); Sat, 10 Oct 2020 15:52:27 -0400 Received: from localhost (83-86-74-64.cable.dynamic.v4.ziggo.nl [83.86.74.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 9129A21D7D; Sat, 10 Oct 2020 10:40:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1602326442; bh=dyGEdyrR3w2vQm2MV9dBBgesUJKrPJrgJAkMuyI+7Mg=; h=Subject:To:From:Date:From; b=jyaNcBdRQPag0GEPmR3P4/h+PMhbe/wvflE9HCrdZdaU3dxOB7qautnW77vwE61f+ IMkNigJ8WzVOs9KIaGBj7QutQ/uvfT3RON5vlT8VEfgz8FphNky5i5WoDA26cYoeQ1 2QYUE+z+eQDpDLnRWLwlJPHOQWF2d2N5dQ4zyqRE= Subject: patch "binder: fix UAF when releasing todo list" added to char-misc-testing To: tkjos@google.com, gregkh@linuxfoundation.org, stable@vger.kernel.org From: Date: Sat, 10 Oct 2020 12:41:26 +0200 Message-ID: <1602326486155117@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ANSI_X3.4-1968 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org This is a note to let you know that I've just added the patch titled binder: fix UAF when releasing todo list to my char-misc git tree which can be found at git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git in the char-misc-testing branch. The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.) The patch will be merged to the char-misc-next branch sometime soon, after it passes testing, and the merge window is open. If you have any questions about this process, please let me know. >From f3277cbfba763cd2826396521b9296de67cf1bbc Mon Sep 17 00:00:00 2001 From: Todd Kjos Date: Fri, 9 Oct 2020 16:24:55 -0700 Subject: binder: fix UAF when releasing todo list When releasing a thread todo list when tearing down a binder_proc, the following race was possible which could result in a use-after-free: 1. Thread 1: enter binder_release_work from binder_thread_release 2. Thread 2: binder_update_ref_for_handle() -> binder_dec_node_ilocked() 3. Thread 2: dec nodeA --> 0 (will free node) 4. Thread 1: ACQ inner_proc_lock 5. Thread 2: block on inner_proc_lock 6. Thread 1: dequeue work (BINDER_WORK_NODE, part of nodeA) 7. Thread 1: REL inner_proc_lock 8. Thread 2: ACQ inner_proc_lock 9. Thread 2: todo list cleanup, but work was already dequeued 10. Thread 2: free node 11. Thread 2: REL inner_proc_lock 12. Thread 1: deref w->type (UAF) The problem was that for a BINDER_WORK_NODE, the binder_work element must not be accessed after releasing the inner_proc_lock while processing the todo list elements since another thread might be handling a deref on the node containing the binder_work element leading to the node being freed. Signed-off-by: Todd Kjos Link: https://lore.kernel.org/r/20201009232455.4054810-1-tkjos@google.com Cc: # 4.14, 4.19, 5.4, 5.8 Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder.c | 35 ++++++++++------------------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 49c0700816a5..4b9476521da6 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -223,7 +223,7 @@ static struct binder_transaction_log_entry *binder_transaction_log_add( struct binder_work { struct list_head entry; - enum { + enum binder_work_type { BINDER_WORK_TRANSACTION = 1, BINDER_WORK_TRANSACTION_COMPLETE, BINDER_WORK_RETURN_ERROR, @@ -885,27 +885,6 @@ static struct binder_work *binder_dequeue_work_head_ilocked( return w; } -/** - * binder_dequeue_work_head() - Dequeues the item at head of list - * @proc: binder_proc associated with list - * @list: list to dequeue head - * - * Removes the head of the list if there are items on the list - * - * Return: pointer dequeued binder_work, NULL if list was empty - */ -static struct binder_work *binder_dequeue_work_head( - struct binder_proc *proc, - struct list_head *list) -{ - struct binder_work *w; - - binder_inner_proc_lock(proc); - w = binder_dequeue_work_head_ilocked(list); - binder_inner_proc_unlock(proc); - return w; -} - static void binder_defer_work(struct binder_proc *proc, enum binder_deferred_state defer); static void binder_free_thread(struct binder_thread *thread); @@ -4585,13 +4564,17 @@ static void binder_release_work(struct binder_proc *proc, struct list_head *list) { struct binder_work *w; + enum binder_work_type wtype; while (1) { - w = binder_dequeue_work_head(proc, list); + binder_inner_proc_lock(proc); + w = binder_dequeue_work_head_ilocked(list); + wtype = w ? w->type : 0; + binder_inner_proc_unlock(proc); if (!w) return; - switch (w->type) { + switch (wtype) { case BINDER_WORK_TRANSACTION: { struct binder_transaction *t; @@ -4625,9 +4608,11 @@ static void binder_release_work(struct binder_proc *proc, kfree(death); binder_stats_deleted(BINDER_STAT_DEATH); } break; + case BINDER_WORK_NODE: + break; default: pr_err("unexpected work type, %d, not freed\n", - w->type); + wtype); break; } } -- 2.28.0