summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTodd Kjos <tkjos@android.com>2019-06-05 09:37:46 -0700
committerTodd Kjos <tkjos@google.com>2019-06-11 10:19:30 -0700
commit7846725776483c5e916c5f2954fe2a96c2fcbff7 (patch)
tree4332d21761e75b72ad0e3471e5d4f105e1b054f8
parentbd858d730975b89bbe203dd1f0fafa7c0b2b4b07 (diff)
BACKPORT: binder: fix race between munmap() and direct reclaim
commit 5cec2d2e5839f9c0fec319c523a911e0a7fd299f upstream. An munmap() on a binder device causes binder_vma_close() to be called which clears the alloc->vma pointer. If direct reclaim causes binder_alloc_free_page() to be called, there is a race where alloc->vma is read into a local vma pointer and then used later after the mm->mmap_sem is acquired. This can result in calling zap_page_range() with an invalid vma which manifests as a use-after-free in zap_page_range(). The fix is to check alloc->vma after acquiring the mmap_sem (which we were acquiring anyway) and skip zap_page_range() if it has changed to NULL. Bug: 120025196 Change-Id: I2f3284d294326ec7736303374769640a1e028783 Cc: Ben Hutchings <ben.hutchings@codethink.co.uk> Signed-off-by: Todd Kjos <tkjos@google.com> Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org> Cc: stable <stable@vger.kernel.org> # 4.14 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--drivers/android/binder_alloc.c20
1 files changed, 9 insertions, 11 deletions
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index baecb4bf9d9b..c8614f3c70c4 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -924,15 +924,14 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
index = page - alloc->pages;
page_addr = (uintptr_t)alloc->buffer + index * PAGE_SIZE;
+
+ mm = alloc->vma_vm_mm;
+ /* Same as mmget_not_zero() in later kernel versions */
+ if (!atomic_inc_not_zero(&alloc->vma_vm_mm->mm_users))
+ goto err_mmget;
+ if (!down_write_trylock(&mm->mmap_sem))
+ goto err_down_write_mmap_sem_failed;
vma = alloc->vma;
- if (vma) {
- /* Same as mmget_not_zero() in later kernel versions */
- if (!atomic_inc_not_zero(&alloc->vma_vm_mm->mm_users))
- goto err_mmget;
- mm = alloc->vma_vm_mm;
- if (!down_write_trylock(&mm->mmap_sem))
- goto err_down_write_mmap_sem_failed;
- }
list_lru_isolate(lru, item);
spin_unlock(lock);
@@ -946,10 +945,9 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
PAGE_SIZE, NULL);
trace_binder_unmap_user_end(alloc, index);
-
- up_write(&mm->mmap_sem);
- mmput(mm);
}
+ up_write(&mm->mmap_sem);
+ mmput(mm);
trace_binder_unmap_kernel_start(alloc, index);