From nobody Fri Oct 24 20:32:36 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B49A0C00140 for ; Mon, 15 Aug 2022 22:28:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1348575AbiHOWZj (ORCPT ); Mon, 15 Aug 2022 18:25:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55652 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1350812AbiHOWSh (ORCPT ); Mon, 15 Aug 2022 18:18:37 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 267A241D0B; Mon, 15 Aug 2022 12:42:22 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id BAB88B80EA9; Mon, 15 Aug 2022 19:42:20 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CDFE7C433D6; Mon, 15 Aug 2022 19:42:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1660592539; bh=LI9xyrlxXoxri9GLybfnF+zXTFS4hqMK+A3lwjDVl7U=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=xoKaxBbYYi5TgjXdNAAnkh+58Vn4o9pe7q9y46M7S3KLL74dRE79nAzbUUR9n+4RW DY9q7GgHpgsvuQvA8JSWOmmn6Yvszx90miv4TqfDVKExEf6wrA+1EPiVcFOIh/bgFh lRLOFqUpZJtiYJXMja3WtQdhjx+0n3A5FNdh6534= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, syzbot+58b51ac2b04e388ab7b0@syzkaller.appspotmail.com, "Liam R. Howlett" , Minchan Kim , "Christian Brauner (Microsoft)" , Hridya Valsaraju , Joel Fernandes , Martijn Coenen , Suren Baghdasaryan , Todd Kjos , "Matthew Wilcox (Oracle)" , Andrew Morton , Sasha Levin Subject: [PATCH 5.18 0805/1095] android: binder: stop saving a pointer to the VMA Date: Mon, 15 Aug 2022 20:03:24 +0200 Message-Id: <20220815180502.571614542@linuxfoundation.org> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20220815180429.240518113@linuxfoundation.org> References: <20220815180429.240518113@linuxfoundation.org> User-Agent: quilt/0.67 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" From: Liam R. Howlett [ Upstream commit a43cfc87caaf46710c8027a8c23b8a55f1078f19 ] Do not record a pointer to a VMA outside of the mmap_lock for later use. This is unsafe and there are a number of failure paths *after* the recorded VMA pointer may be freed during setup. There is no callback to the driver to clear the saved pointer from generic mm code. Furthermore, the VMA pointer may become stale if any number of VMA operations end up freeing the VMA so saving it was fragile to being with. Instead, change the binder_alloc struct to record the start address of the VMA and use vma_lookup() to get the vma when needed. Add lockdep mmap_lock checks on updates to the vma pointer to ensure the lock is held and depend on that lock for synchronization of readers and writers - which was already the case anyways, so the smp_wmb()/smp_rmb() was not necessary. [akpm@linux-foundation.org: fix drivers/android/binder_alloc_selftest.c] Link: https://lkml.kernel.org/r/20220621140212.vpkio64idahetbyf@revolver Fixes: da1b9564e85b ("android: binder: fix the race mmap and alloc_new_buf_= locked") Reported-by: syzbot+58b51ac2b04e388ab7b0@syzkaller.appspotmail.com Signed-off-by: Liam R. Howlett Cc: Minchan Kim Cc: Christian Brauner (Microsoft) Cc: Greg Kroah-Hartman Cc: Hridya Valsaraju Cc: Joel Fernandes Cc: Martijn Coenen Cc: Suren Baghdasaryan Cc: Todd Kjos Cc: Matthew Wilcox (Oracle) Signed-off-by: Andrew Morton Signed-off-by: Sasha Levin --- drivers/android/binder_alloc.c | 30 ++++++++++++------------- drivers/android/binder_alloc.h | 2 +- drivers/android/binder_alloc_selftest.c | 2 +- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c index 2ac1008a5f39..a93a7d2a8caf 100644 --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -213,7 +213,7 @@ static int binder_update_page_range(struct binder_alloc= *alloc, int allocate, =20 if (mm) { mmap_read_lock(mm); - vma =3D alloc->vma; + vma =3D vma_lookup(mm, alloc->vma_addr); } =20 if (!vma && need_mm) { @@ -313,16 +313,15 @@ static int binder_update_page_range(struct binder_all= oc *alloc, int allocate, static inline void binder_alloc_set_vma(struct binder_alloc *alloc, struct vm_area_struct *vma) { - if (vma) + unsigned long vm_start =3D 0; + + if (vma) { + vm_start =3D vma->vm_start; alloc->vma_vm_mm =3D vma->vm_mm; - /* - * If we see alloc->vma is not NULL, buffer data structures set up - * completely. Look at smp_rmb side binder_alloc_get_vma. - * We also want to guarantee new alloc->vma_vm_mm is always visible - * if alloc->vma is set. - */ - smp_wmb(); - alloc->vma =3D vma; + } + + mmap_assert_write_locked(alloc->vma_vm_mm); + alloc->vma_addr =3D vm_start; } =20 static inline struct vm_area_struct *binder_alloc_get_vma( @@ -330,11 +329,9 @@ static inline struct vm_area_struct *binder_alloc_get_= vma( { struct vm_area_struct *vma =3D NULL; =20 - if (alloc->vma) { - /* Look at description in binder_alloc_set_vma */ - smp_rmb(); - vma =3D alloc->vma; - } + if (alloc->vma_addr) + vma =3D vma_lookup(alloc->vma_vm_mm, alloc->vma_addr); + return vma; } =20 @@ -817,7 +814,8 @@ void binder_alloc_deferred_release(struct binder_alloc = *alloc) =20 buffers =3D 0; mutex_lock(&alloc->mutex); - BUG_ON(alloc->vma); + BUG_ON(alloc->vma_addr && + vma_lookup(alloc->vma_vm_mm, alloc->vma_addr)); =20 while ((n =3D rb_first(&alloc->allocated_buffers))) { buffer =3D rb_entry(n, struct binder_buffer, rb_node); diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h index 7dea57a84c79..1e4fd37af5e0 100644 --- a/drivers/android/binder_alloc.h +++ b/drivers/android/binder_alloc.h @@ -100,7 +100,7 @@ struct binder_lru_page { */ struct binder_alloc { struct mutex mutex; - struct vm_area_struct *vma; + unsigned long vma_addr; struct mm_struct *vma_vm_mm; void __user *buffer; struct list_head buffers; diff --git a/drivers/android/binder_alloc_selftest.c b/drivers/android/bind= er_alloc_selftest.c index c2b323bc3b3a..43a881073a42 100644 --- a/drivers/android/binder_alloc_selftest.c +++ b/drivers/android/binder_alloc_selftest.c @@ -287,7 +287,7 @@ void binder_selftest_alloc(struct binder_alloc *alloc) if (!binder_selftest_run) return; mutex_lock(&binder_selftest_lock); - if (!binder_selftest_run || !alloc->vma) + if (!binder_selftest_run || !alloc->vma_addr) goto done; pr_info("STARTED\n"); binder_selftest_alloc_offset(alloc, end_offset, 0); --=20 2.35.1