Skip to content

Commit

Permalink
android: binder: stop saving a pointer to the VMA
Browse files Browse the repository at this point in the history
[upstream commit a43cfc8]

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: da1b956 ("android: binder: fix the race mmap and alloc_new_buf_locked")
Reported-by: syzbot+58b51ac2b04e388ab7b0@syzkaller.appspotmail.com
Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
Cc: Minchan Kim <minchan@kernel.org>
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>
Cc: Todd Kjos <tkjos@android.com>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
  • Loading branch information
howlett authored and Dinh Nguyen committed Mar 28, 2023
1 parent 184ebb0 commit 779e6c1
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion drivers/android/binder_alloc.c
Expand Up @@ -317,9 +317,10 @@ static inline void binder_alloc_set_vma(struct binder_alloc *alloc,

if (vma) {
vm_start = vma->vm_start;
mmap_assert_write_locked(alloc->vma_vm_mm);
alloc->vma_vm_mm = vma->vm_mm;
}

mmap_assert_write_locked(alloc->vma_vm_mm);
alloc->vma_addr = vm_start;
}

Expand Down

0 comments on commit 779e6c1

Please sign in to comment.