linux.git/drivers/android, branch v5.15.114 Clone of https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git binder: Gracefully handle BINDER_TYPE_FDA objects with num_fds=0 2023-02-25T11:06:45+00:00 Alessandro Astone ales.astone@gmail.com 2023-02-22T12:12:08+00:00 d518ca02542fda332b34c2a3db9164363ac3f58e commit ef38de9217a04c9077629a24652689d8fdb4c6c6 upstream. Some android userspace is sending BINDER_TYPE_FDA objects with num_fds=0. Like the previous patch, this is reproducible when playing a video. Before commit 09184ae9b575 BINDER_TYPE_FDA objects with num_fds=0 were 'correctly handled', as in no fixup was performed. After commit 09184ae9b575 we aggregate fixup and skip regions in binder_ptr_fixup structs and distinguish between the two by using the skip_size field: if it's 0, then it's a fixup, otherwise skip. When processing BINDER_TYPE_FDA objects with num_fds=0 we add a skip region of skip_size=0, and this causes issues because now binder_do_deferred_txn_copies will think this was a fixup region. To address that, return early from binder_translate_fd_array to avoid adding an empty skip region. Fixes: 09184ae9b575 ("binder: defer copies of pre-patched txn data") Acked-by: Todd Kjos <tkjos@google.com> Cc: stable <stable@kernel.org> Signed-off-by: Alessandro Astone <ales.astone@gmail.com> Link: https://lore.kernel.org/r/20220415120015.52684-1-ales.astone@gmail.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Carlos Llamas <cmllamas@google.com> Signed-off-by: Lee Jones <lee@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit ef38de9217a04c9077629a24652689d8fdb4c6c6 upstream.

Some android userspace is sending BINDER_TYPE_FDA objects with
num_fds=0. Like the previous patch, this is reproducible when
playing a video.

Before commit 09184ae9b575 BINDER_TYPE_FDA objects with num_fds=0
were 'correctly handled', as in no fixup was performed.

After commit 09184ae9b575 we aggregate fixup and skip regions in
binder_ptr_fixup structs and distinguish between the two by using
the skip_size field: if it's 0, then it's a fixup, otherwise skip.
When processing BINDER_TYPE_FDA objects with num_fds=0 we add a
skip region of skip_size=0, and this causes issues because now
binder_do_deferred_txn_copies will think this was a fixup region.

To address that, return early from binder_translate_fd_array to
avoid adding an empty skip region.

Fixes: 09184ae9b575 ("binder: defer copies of pre-patched txn data")
Acked-by: Todd Kjos <tkjos@google.com>
Cc: stable <stable@kernel.org>
Signed-off-by: Alessandro Astone <ales.astone@gmail.com>
Link: https://lore.kernel.org/r/20220415120015.52684-1-ales.astone@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Signed-off-by: Lee Jones <lee@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder: Address corner cases in deferred copy and fixup 2023-02-25T11:06:45+00:00 Alessandro Astone ales.astone@gmail.com 2023-02-22T12:12:07+00:00 367d0456c79264d8fe743a4ab2961c772db4d495 commit 2d1746e3fda0c3612143d7c06f8e1d1830c13e23 upstream. When handling BINDER_TYPE_FDA object we are pushing a parent fixup with a certain skip_size but no scatter-gather copy object, since the copy is handled standalone. If BINDER_TYPE_FDA is the last children the scatter-gather copy loop will never stop to skip it, thus we are left with an item in the parent fixup list. This will trigger the BUG_ON(). This is reproducible in android when playing a video. We receive a transaction that looks like this: obj[0] BINDER_TYPE_PTR, parent obj[1] BINDER_TYPE_PTR, child obj[2] BINDER_TYPE_PTR, child obj[3] BINDER_TYPE_FDA, child Fixes: 09184ae9b575 ("binder: defer copies of pre-patched txn data") Acked-by: Todd Kjos <tkjos@google.com> Cc: stable <stable@kernel.org> Signed-off-by: Alessandro Astone <ales.astone@gmail.com> Link: https://lore.kernel.org/r/20220415120015.52684-2-ales.astone@gmail.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Carlos Llamas <cmllamas@google.com> Signed-off-by: Lee Jones <lee@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit 2d1746e3fda0c3612143d7c06f8e1d1830c13e23 upstream.

When handling BINDER_TYPE_FDA object we are pushing a parent fixup
with a certain skip_size but no scatter-gather copy object, since
the copy is handled standalone.
If BINDER_TYPE_FDA is the last children the scatter-gather copy
loop will never stop to skip it, thus we are left with an item in
the parent fixup list. This will trigger the BUG_ON().

This is reproducible in android when playing a video.
We receive a transaction that looks like this:
    obj[0] BINDER_TYPE_PTR, parent
    obj[1] BINDER_TYPE_PTR, child
    obj[2] BINDER_TYPE_PTR, child
    obj[3] BINDER_TYPE_FDA, child

Fixes: 09184ae9b575 ("binder: defer copies of pre-patched txn data")
Acked-by: Todd Kjos <tkjos@google.com>
Cc: stable <stable@kernel.org>
Signed-off-by: Alessandro Astone <ales.astone@gmail.com>
Link: https://lore.kernel.org/r/20220415120015.52684-2-ales.astone@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Signed-off-by: Lee Jones <lee@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder: fix pointer cast warning 2023-02-25T11:06:45+00:00 Arnd Bergmann arnd@arndb.de 2023-02-22T12:12:06+00:00 b345b22002889b943c50db25cd7f37c93def722a commit 9a0a930fe2535a76ad70d3f43caeccf0d86a3009 upstream. binder_uintptr_t is not the same as uintptr_t, so converting it into a pointer requires a second cast: drivers/android/binder.c: In function 'binder_translate_fd_array': drivers/android/binder.c:2511:28: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] 2511 | sender_ufda_base = (void __user *)sender_uparent->buffer + fda->parent_offset; | ^ Fixes: 656e01f3ab54 ("binder: read pre-translated fds from sender buffer") Acked-by: Todd Kjos <tkjos@google.com> Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Arnd Bergmann <arnd@arndb.de> Link: https://lore.kernel.org/r/20211207122448.1185769-1-arnd@kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Carlos Llamas <cmllamas@google.com> Signed-off-by: Lee Jones <lee@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit 9a0a930fe2535a76ad70d3f43caeccf0d86a3009 upstream.

binder_uintptr_t is not the same as uintptr_t, so converting it into a
pointer requires a second cast:

drivers/android/binder.c: In function 'binder_translate_fd_array':
drivers/android/binder.c:2511:28: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
 2511 |         sender_ufda_base = (void __user *)sender_uparent->buffer + fda->parent_offset;
      |                            ^

Fixes: 656e01f3ab54 ("binder: read pre-translated fds from sender buffer")
Acked-by: Todd Kjos <tkjos@google.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Link: https://lore.kernel.org/r/20211207122448.1185769-1-arnd@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Signed-off-by: Lee Jones <lee@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder: defer copies of pre-patched txn data 2023-02-25T11:06:45+00:00 Todd Kjos tkjos@google.com 2023-02-22T12:12:05+00:00 c194fc351fecb419e7f3a33ed7e9b273b427d263 commit 09184ae9b5756cc469db6fd1d1cfdcffbf627c2d upstream. BINDER_TYPE_PTR objects point to memory areas in the source process to be copied into the target buffer as part of a transaction. This implements a scatter- gather model where non-contiguous memory in a source process is "gathered" into a contiguous region in the target buffer. The data can include pointers that must be fixed up to correctly point to the copied data. To avoid making source process pointers visible to the target process, this patch defers the copy until the fixups are known and then copies and fixeups are done together. There is a special case of BINDER_TYPE_FDA which applies the fixup later in the target process context. In this case the user data is skipped (so no untranslated fds become visible to the target). Reviewed-by: Martijn Coenen <maco@android.com> Signed-off-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20211130185152.437403-5-tkjos@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Lee Jones <lee@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit 09184ae9b5756cc469db6fd1d1cfdcffbf627c2d upstream.

BINDER_TYPE_PTR objects point to memory areas in the
source process to be copied into the target buffer
as part of a transaction. This implements a scatter-
gather model where non-contiguous memory in a source
process is "gathered" into a contiguous region in
the target buffer.

The data can include pointers that must be fixed up
to correctly point to the copied data. To avoid making
source process pointers visible to the target process,
this patch defers the copy until the fixups are known
and then copies and fixeups are done together.

There is a special case of BINDER_TYPE_FDA which applies
the fixup later in the target process context. In this
case the user data is skipped (so no untranslated fds
become visible to the target).

Reviewed-by: Martijn Coenen <maco@android.com>
Signed-off-by: Todd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20211130185152.437403-5-tkjos@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Lee Jones <lee@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder: read pre-translated fds from sender buffer 2023-02-25T11:06:44+00:00 Todd Kjos tkjos@google.com 2023-02-22T12:12:04+00:00 d107b4352284aff85e9dae0b13d4b05e17a1520c commit 656e01f3ab54afe71bed066996fc2640881e1220 upstream. This patch is to prepare for an up coming patch where we read pre-translated fds from the sender buffer and translate them before copying them to the target. It does not change run time. The patch adds two new parameters to binder_translate_fd_array() to hold the sender buffer and sender buffer parent. These parameters let us call copy_from_user() directly from the sender instead of using binder_alloc_copy_from_buffer() to copy from the target. Also the patch adds some new alignment checks. Previously the alignment checks would have been done in a different place, but this lets us print more useful error messages. Reviewed-by: Martijn Coenen <maco@android.com> Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20211130185152.437403-4-tkjos@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Carlos Llamas <cmllamas@google.com> Signed-off-by: Lee Jones <lee@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit 656e01f3ab54afe71bed066996fc2640881e1220 upstream.

This patch is to prepare for an up coming patch where we read
pre-translated fds from the sender buffer and translate them before
copying them to the target.  It does not change run time.

The patch adds two new parameters to binder_translate_fd_array() to
hold the sender buffer and sender buffer parent.  These parameters let
us call copy_from_user() directly from the sender instead of using
binder_alloc_copy_from_buffer() to copy from the target.  Also the patch
adds some new alignment checks.  Previously the alignment checks would
have been done in a different place, but this lets us print more
useful error messages.

Reviewed-by: Martijn Coenen <maco@android.com>
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Todd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20211130185152.437403-4-tkjos@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Signed-off-by: Lee Jones <lee@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder: validate alloc->mm in ->mmap() handler 2022-12-02T16:41:00+00:00 Carlos Llamas cmllamas@google.com 2022-11-04T23:12:35+00:00 d276fb4a7eb8bec3d596d412daf6d4b2419674d9 [ Upstream commit 3ce00bb7e91cf57d723905371507af57182c37ef ] Since commit 1da52815d5f1 ("binder: fix alloc->vma_vm_mm null-ptr dereference") binder caches a pointer to the current->mm during open(). This fixes a null-ptr dereference reported by syzkaller. Unfortunately, it also opens the door for a process to update its mm after the open(), (e.g. via execve) making the cached alloc->mm pointer invalid. Things get worse when the process continues to mmap() a vma. From this point forward, binder will attempt to find this vma using an obsolete alloc->mm reference. Such as in binder_update_page_range(), where the wrong vma is obtained via vma_lookup(), yet binder proceeds to happily insert new pages into it. To avoid this issue fail the ->mmap() callback if we detect a mismatch between the vma->vm_mm and the original alloc->mm pointer. This prevents alloc->vm_addr from getting set, so that any subsequent vma_lookup() calls fail as expected. Fixes: 1da52815d5f1 ("binder: fix alloc->vma_vm_mm null-ptr dereference") Reported-by: Jann Horn <jannh@google.com> Cc: <stable@vger.kernel.org> # 5.15+ Signed-off-by: Carlos Llamas <cmllamas@google.com> Acked-by: Todd Kjos <tkjos@google.com> Link: https://lore.kernel.org/r/20221104231235.348958-1-cmllamas@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Sasha Levin <sashal@kernel.org> 
[ Upstream commit 3ce00bb7e91cf57d723905371507af57182c37ef ]

Since commit 1da52815d5f1 ("binder: fix alloc->vma_vm_mm null-ptr
dereference") binder caches a pointer to the current->mm during open().
This fixes a null-ptr dereference reported by syzkaller. Unfortunately,
it also opens the door for a process to update its mm after the open(),
(e.g. via execve) making the cached alloc->mm pointer invalid.

Things get worse when the process continues to mmap() a vma. From this
point forward, binder will attempt to find this vma using an obsolete
alloc->mm reference. Such as in binder_update_page_range(), where the
wrong vma is obtained via vma_lookup(), yet binder proceeds to happily
insert new pages into it.

To avoid this issue fail the ->mmap() callback if we detect a mismatch
between the vma->vm_mm and the original alloc->mm pointer. This prevents
alloc->vm_addr from getting set, so that any subsequent vma_lookup()
calls fail as expected.

Fixes: 1da52815d5f1 ("binder: fix alloc->vma_vm_mm null-ptr dereference")
Reported-by: Jann Horn <jannh@google.com>
Cc: <stable@vger.kernel.org> # 5.15+
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Acked-by: Todd Kjos <tkjos@google.com>
Link: https://lore.kernel.org/r/20221104231235.348958-1-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
binder: remove inaccurate mmap_assert_locked() 2022-09-23T12:15:49+00:00 Carlos Llamas cmllamas@google.com 2022-09-20T05:24:43+00:00 899f4160b1404c6ee04f87a34546ab6a3767ac46 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@oracle.com> Cc: Suren Baghdasaryan <surenb@google.com> Cc: <stable@vger.kernel.org> # v5.15 Signed-off-by: Carlos Llamas <cmllamas@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
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@oracle.com>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: <stable@vger.kernel.org> # v5.15
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder: fix alloc->vma_vm_mm null-ptr dereference 2022-09-08T10:28:04+00:00 Carlos Llamas cmllamas@google.com 2022-08-29T20:12:48+00:00 81203ab7a6ef843a2b904a0a494f28c457d44d27 commit 1da52815d5f1b654c89044db0cdc6adce43da1f1 upstream. Syzbot reported a couple issues introduced by commit 44e602b4e52f ("binder_alloc: add missing mmap_lock calls when using the VMA"), in which we attempt to acquire the mmap_lock when alloc->vma_vm_mm has not been initialized yet. This can happen if a binder_proc receives a transaction without having previously called mmap() to setup the binder_proc->alloc space in [1]. Also, a similar issue occurs via binder_alloc_print_pages() when we try to dump the debugfs binder stats file in [2]. Sample of syzbot's crash report: ================================================================== KASAN: null-ptr-deref in range [0x0000000000000128-0x000000000000012f] CPU: 0 PID: 3755 Comm: syz-executor229 Not tainted 6.0.0-rc1-next-20220819-syzkaller #0 syz-executor229[3755] cmdline: ./syz-executor2294415195 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/22/2022 RIP: 0010:__lock_acquire+0xd83/0x56d0 kernel/locking/lockdep.c:4923 [...] Call Trace: <TASK> lock_acquire kernel/locking/lockdep.c:5666 [inline] lock_acquire+0x1ab/0x570 kernel/locking/lockdep.c:5631 down_read+0x98/0x450 kernel/locking/rwsem.c:1499 mmap_read_lock include/linux/mmap_lock.h:117 [inline] binder_alloc_new_buf_locked drivers/android/binder_alloc.c:405 [inline] binder_alloc_new_buf+0xa5/0x19e0 drivers/android/binder_alloc.c:593 binder_transaction+0x242e/0x9a80 drivers/android/binder.c:3199 binder_thread_write+0x664/0x3220 drivers/android/binder.c:3986 binder_ioctl_write_read drivers/android/binder.c:5036 [inline] binder_ioctl+0x3470/0x6d00 drivers/android/binder.c:5323 vfs_ioctl fs/ioctl.c:51 [inline] __do_sys_ioctl fs/ioctl.c:870 [inline] __se_sys_ioctl fs/ioctl.c:856 [inline] __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:856 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x63/0xcd [...] ================================================================== Fix these issues by setting up alloc->vma_vm_mm pointer during open() and caching directly from current->mm. This guarantees we have a valid reference to take the mmap_lock during scenarios described above. [1] https://syzkaller.appspot.com/bug?extid=f7dc54e5be28950ac459 [2] https://syzkaller.appspot.com/bug?extid=a75ebe0452711c9e56d9 Fixes: 44e602b4e52f ("binder_alloc: add missing mmap_lock calls when using the VMA") Cc: <stable@vger.kernel.org> # v5.15+ Cc: Liam R. Howlett <Liam.Howlett@oracle.com> Reported-by: syzbot+f7dc54e5be28950ac459@syzkaller.appspotmail.com Reported-by: syzbot+a75ebe0452711c9e56d9@syzkaller.appspotmail.com Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com> Acked-by: Todd Kjos <tkjos@google.com> Signed-off-by: Carlos Llamas <cmllamas@google.com> Link: https://lore.kernel.org/r/20220829201254.1814484-2-cmllamas@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit 1da52815d5f1b654c89044db0cdc6adce43da1f1 upstream.

Syzbot reported a couple issues introduced by commit 44e602b4e52f
("binder_alloc: add missing mmap_lock calls when using the VMA"), in
which we attempt to acquire the mmap_lock when alloc->vma_vm_mm has not
been initialized yet.

This can happen if a binder_proc receives a transaction without having
previously called mmap() to setup the binder_proc->alloc space in [1].
Also, a similar issue occurs via binder_alloc_print_pages() when we try
to dump the debugfs binder stats file in [2].

Sample of syzbot's crash report:
  ==================================================================
  KASAN: null-ptr-deref in range [0x0000000000000128-0x000000000000012f]
  CPU: 0 PID: 3755 Comm: syz-executor229 Not tainted 6.0.0-rc1-next-20220819-syzkaller #0
  syz-executor229[3755] cmdline: ./syz-executor2294415195
  Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/22/2022
  RIP: 0010:__lock_acquire+0xd83/0x56d0 kernel/locking/lockdep.c:4923
  [...]
  Call Trace:
   <TASK>
   lock_acquire kernel/locking/lockdep.c:5666 [inline]
   lock_acquire+0x1ab/0x570 kernel/locking/lockdep.c:5631
   down_read+0x98/0x450 kernel/locking/rwsem.c:1499
   mmap_read_lock include/linux/mmap_lock.h:117 [inline]
   binder_alloc_new_buf_locked drivers/android/binder_alloc.c:405 [inline]
   binder_alloc_new_buf+0xa5/0x19e0 drivers/android/binder_alloc.c:593
   binder_transaction+0x242e/0x9a80 drivers/android/binder.c:3199
   binder_thread_write+0x664/0x3220 drivers/android/binder.c:3986
   binder_ioctl_write_read drivers/android/binder.c:5036 [inline]
   binder_ioctl+0x3470/0x6d00 drivers/android/binder.c:5323
   vfs_ioctl fs/ioctl.c:51 [inline]
   __do_sys_ioctl fs/ioctl.c:870 [inline]
   __se_sys_ioctl fs/ioctl.c:856 [inline]
   __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:856
   do_syscall_x64 arch/x86/entry/common.c:50 [inline]
   do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
   entry_SYSCALL_64_after_hwframe+0x63/0xcd
   [...]
  ==================================================================

Fix these issues by setting up alloc->vma_vm_mm pointer during open()
and caching directly from current->mm. This guarantees we have a valid
reference to take the mmap_lock during scenarios described above.

[1] https://syzkaller.appspot.com/bug?extid=f7dc54e5be28950ac459
[2] https://syzkaller.appspot.com/bug?extid=a75ebe0452711c9e56d9

Fixes: 44e602b4e52f ("binder_alloc: add missing mmap_lock calls when using the VMA")
Cc: <stable@vger.kernel.org> # v5.15+
Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
Reported-by: syzbot+f7dc54e5be28950ac459@syzkaller.appspotmail.com
Reported-by: syzbot+a75ebe0452711c9e56d9@syzkaller.appspotmail.com
Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
Acked-by: Todd Kjos <tkjos@google.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20220829201254.1814484-2-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
binder: fix UAF of ref->proc caused by race condition 2022-09-08T10:28:04+00:00 Carlos Llamas cmllamas@google.com 2022-08-01T18:25:11+00:00 c2a4b5dc8fa71af73bab704d0cac42ac39767ed6 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) <brauner@kernel.org> Signed-off-by: Carlos Llamas <cmllamas@google.com> Cc: stable <stable@kernel.org> # 4.14+ Link: https://lore.kernel.org/r/20220801182511.3371447-1-cmllamas@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
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) <brauner@kernel.org>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Cc: stable <stable@kernel.org> # 4.14+
Link: https://lore.kernel.org/r/20220801182511.3371447-1-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
android: binder: fix lockdep check on clearing vma 2022-09-05T08:30:12+00:00 Liam Howlett liam.howlett@oracle.com 2022-06-27T15:18:59+00:00 591a98b823fb58ecfb80ced10fdc764882e753b1 commit b0cab80ecd54ae3b2356bb081af0bffd538c8265 upstream. When munmapping a vma, the mmap_lock can be degraded to a write before calling close() on the file handle. The binder close() function calls binder_alloc_set_vma() to clear the vma address, which now has a lock dep check for writing on the mmap_lock. Change the lockdep check to ensure the reading lock is held while clearing and keep the write check while writing. Link: https://lkml.kernel.org/r/20220627151857.2316964-1-Liam.Howlett@oracle.com Fixes: 472a68df605b ("android: binder: stop saving a pointer to the VMA") Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com> Reported-by: syzbot+da54fa8d793ca89c741f@syzkaller.appspotmail.com Acked-by: Todd Kjos <tkjos@google.com> Cc: "Arve Hjønnevåg" <arve@android.com> Cc: Christian Brauner (Microsoft) <brauner@kernel.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Hridya Valsaraju <hridya@google.com> Cc: Joel Fernandes <joel@joelfernandes.org> Cc: Martijn Coenen <maco@android.com> Cc: Suren Baghdasaryan <surenb@google.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
commit b0cab80ecd54ae3b2356bb081af0bffd538c8265 upstream.

When munmapping a vma, the mmap_lock can be degraded to a write before
calling close() on the file handle.  The binder close() function calls
binder_alloc_set_vma() to clear the vma address, which now has a lock dep
check for writing on the mmap_lock.  Change the lockdep check to ensure
the reading lock is held while clearing and keep the write check while
writing.

Link: https://lkml.kernel.org/r/20220627151857.2316964-1-Liam.Howlett@oracle.com
Fixes: 472a68df605b ("android: binder: stop saving a pointer to the VMA")
Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
Reported-by: syzbot+da54fa8d793ca89c741f@syzkaller.appspotmail.com
Acked-by: Todd Kjos <tkjos@google.com>
Cc: "Arve Hjønnevåg" <arve@android.com>
Cc: Christian Brauner (Microsoft) <brauner@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Hridya Valsaraju <hridya@google.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Martijn Coenen <maco@android.com>
Cc: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>