linux-kernel - [PATCH 5.15 25/45] binder: remove inaccurate mmap_assert

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 21 Sep 2022 17:46:15 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     linux-kernel@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org, Liam Howlett <liam.howlett@...cle.com>,
        Suren Baghdasaryan <surenb@...gle.com>,
        Carlos Llamas <cmllamas@...gle.com>
Subject: [PATCH 5.15 25/45] binder: remove inaccurate mmap_assert_locked()

From: Carlos Llamas <cmllamas@...gle.com>

Acquiring the mmap_lock during exit_mmap() was only added recently in
v5.17 by commit 64591e8605d6 ("mm: protect free_pgtables with mmap_lock
write lock in exit_mmap"). Soon after, asserts for holding this lock
were added to the binder_alloc_set_vma() callback by the following two
fix commits in mainline: commit b0cab80ecd54 ("android: binder: fix
lockdep check on clearing vma") and commit a43cfc87caaf ("android:
binder: stop saving a pointer to the VMA").

These two fix commits were picked for stable trees including v5.15 were
unfortunately the mmap_lock is not held during exit_mmap() yet and this
unmet dependency leads to the following BUG report:

  ------------[ cut here ]------------
  kernel BUG at include/linux/mmap_lock.h:156!
  Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
  Modules linked in:
  CPU: 3 PID: 437 Comm: binder Not tainted 5.15.68 #5
  Hardware name: linux,dummy-virt (DT)
  pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : binder_alloc_vma_close+0x6c/0x70
  lr : binder_alloc_vma_close+0x6c/0x70
  sp : ffff800008687a70
  x29: ffff800008687a70 x28: ffff02a7ccf89d00 x27: ffff02a7c92f99e8
  x26: 000000000000012a x25: ffff02a7c6284740 x24: ffff02a7ccf8a360
  x23: ffff02a7c92f9980 x22: 1ffff000010d0f6c x21: ffff02a7c92f99e8
  x20: ffff02a7c92f9980 x19: ffff02a7d16b79a8 x18: 0000ffffe1702d20
  x17: 3334373239343932 x16: 34206e6163735f74 x15: 78656e5f616d756e
  x14: 0a30303030303030 x13: 7366666f5f6e6163 x12: ffff60550564a12b
  x11: 1fffe0550564a12a x10: ffff60550564a12a x9 : dfff800000000000
  x8 : ffff02a82b250957 x7 : 0000000000000001 x6 : ffff60550564a12a
  x5 : ffff02a82b250950 x4 : dfff800000000000 x3 : 0000000000000000
  x2 : 0000000000000000 x1 : ffff02a7ccf89d00 x0 : 0000000000000374
  Call trace:
   binder_alloc_vma_close+0x6c/0x70
   binder_vma_close+0x38/0xf4
   remove_vma+0x4c/0x94
   exit_mmap+0x14c/0x2bc
   __mmput+0x70/0x19c
   mmput+0x68/0x80
   do_exit+0x484/0xeb0
   do_group_exit+0x5c/0x100
   [...]

This patch removes the inaccurate assert specifically from v5.15 since
it's the only release with such issue. Note the mmap_lock is technically
not needed here as the mm->mm_users has dropped to zero at this point.
More context: https://lore.kernel.org/all/YxpQaio7xm3z9TUw@google.com/.

Fixes: b0cab80ecd54 ("android: binder: fix lockdep check on clearing vma")
Fixes: a43cfc87caaf ("android: binder: stop saving a pointer to the VMA")
Cc: Liam Howlett <liam.howlett@...cle.com>
Cc: Suren Baghdasaryan <surenb@...gle.com>
Cc: <stable@...r.kernel.org> # v5.15
Signed-off-by: Carlos Llamas <cmllamas@...gle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
---
 drivers/android/binder_alloc.c |    7 -------
 1 file changed, 7 deletions(-)

--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -315,16 +315,9 @@ static inline void binder_alloc_set_vma(
 {
 	unsigned long vm_start = 0;
 
-	/*
-	 * Allow clearing the vma with holding just the read lock to allow
-	 * munmapping downgrade of the write lock before freeing and closing the
-	 * file using binder_alloc_vma_close().
-	 */
 	if (vma) {
 		vm_start = vma->vm_start;
 		mmap_assert_write_locked(alloc->vma_vm_mm);
-	} else {
-		mmap_assert_locked(alloc->vma_vm_mm);
 	}
 
 	alloc->vma_addr = vm_start;