Skip to content

Commit

Permalink
Revert "android: binder: stop saving a pointer to the VMA"
Browse files Browse the repository at this point in the history
commit c0fd210 upstream.

This reverts commit a43cfc8.

This patch fixed an issue reported by syzkaller in [1]. However, this
turned out to be only a band-aid in binder. The root cause, as bisected
by syzkaller, was fixed by commit 5789151 ("mm/mmap: undo ->mmap()
when mas_preallocate() fails"). We no longer need the patch for binder.

Reverting such patch allows us to have a lockless access to alloc->vma
in specific cases where the mmap_lock is not required. This approach
avoids the contention that caused a performance regression.

[1] https://lore.kernel.org/all/[email protected]

[cmllamas: resolved conflicts with rework of alloc->mm and removal of
 binder_alloc_set_vma() also fixed comment section]

Fixes: a43cfc8 ("android: binder: stop saving a pointer to the VMA")
Cc: Liam Howlett <[email protected]>
Cc: Suren Baghdasaryan <[email protected]>
Cc: [email protected]
Signed-off-by: Carlos Llamas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
[cmllamas: fixed merge conflict in binder_alloc_set_vma()]
Signed-off-by: Carlos Llamas <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
  • Loading branch information
Carlos Llamas authored and gregkh committed Jun 5, 2023
1 parent 6802c70 commit dd7aff4
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 16 deletions.
27 changes: 13 additions & 14 deletions drivers/android/binder_alloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ static int binder_update_page_range(struct binder_alloc *alloc, int allocate,

if (mm) {
mmap_read_lock(mm);
vma = vma_lookup(mm, alloc->vma_addr);
vma = alloc->vma;
}

if (!vma && need_mm) {
Expand Down Expand Up @@ -313,24 +313,24 @@ static int binder_update_page_range(struct binder_alloc *alloc, int allocate,
static inline void binder_alloc_set_vma(struct binder_alloc *alloc,
struct vm_area_struct *vma)
{
unsigned long vm_start = 0;

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

alloc->vma_addr = vm_start;
/*
* If we see alloc->vma is not NULL, buffer data structures set up
* completely. Look at smp_rmb side binder_alloc_get_vma.
*/
smp_wmb();
alloc->vma = vma;
}

static inline struct vm_area_struct *binder_alloc_get_vma(
struct binder_alloc *alloc)
{
struct vm_area_struct *vma = NULL;

if (alloc->vma_addr)
vma = vma_lookup(alloc->vma_vm_mm, alloc->vma_addr);

if (alloc->vma) {
/* Look at description in binder_alloc_set_vma */
smp_rmb();
vma = alloc->vma;
}
return vma;
}

Expand Down Expand Up @@ -819,8 +819,7 @@ void binder_alloc_deferred_release(struct binder_alloc *alloc)

buffers = 0;
mutex_lock(&alloc->mutex);
BUG_ON(alloc->vma_addr &&
vma_lookup(alloc->vma_vm_mm, alloc->vma_addr));
BUG_ON(alloc->vma);

while ((n = rb_first(&alloc->allocated_buffers))) {
buffer = rb_entry(n, struct binder_buffer, rb_node);
Expand Down
2 changes: 1 addition & 1 deletion drivers/android/binder_alloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ struct binder_lru_page {
*/
struct binder_alloc {
struct mutex mutex;
unsigned long vma_addr;
struct vm_area_struct *vma;
struct mm_struct *vma_vm_mm;
void __user *buffer;
struct list_head buffers;
Expand Down
2 changes: 1 addition & 1 deletion drivers/android/binder_alloc_selftest.c
Original file line number Diff line number Diff line change
Expand Up @@ -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_addr)
if (!binder_selftest_run || !alloc->vma)
goto done;
pr_info("STARTED\n");
binder_selftest_alloc_offset(alloc, end_offset, 0);
Expand Down

0 comments on commit dd7aff4

Please sign in to comment.