diff options
author | Todd Kjos <tkjos@android.com> | 2019-06-05 09:37:46 -0700 |
---|---|---|
committer | Todd Kjos <tkjos@google.com> | 2019-06-11 10:19:30 -0700 |
commit | 7846725776483c5e916c5f2954fe2a96c2fcbff7 (patch) | |
tree | 4332d21761e75b72ad0e3471e5d4f105e1b054f8 | |
parent | bd858d730975b89bbe203dd1f0fafa7c0b2b4b07 (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.c | 20 |
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); |