| From tkjos@android.com Thu Jun 6 09:18:12 2019 |
| From: Todd Kjos <tkjos@android.com> |
| Date: Wed, 5 Jun 2019 09:38:25 -0700 |
| Subject: binder: fix race between munmap() and direct reclaim |
| To: tkjos@google.com, gregkh@linuxfoundation.org, stable@vger.kernel.org |
| Cc: ben.hutchings@codethink.co.uk, Alexander.Levin@microsoft.com, Joel Fernandes <joel@joelfernandes.org> |
| Message-ID: <20190605163825.178537-2-tkjos@google.com> |
| |
| From: Todd Kjos <tkjos@android.com> |
| |
| 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. |
| |
| 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.19 |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/android/binder_alloc.c | 18 ++++++++---------- |
| 1 file changed, 8 insertions(+), 10 deletions(-) |
| |
| --- a/drivers/android/binder_alloc.c |
| +++ b/drivers/android/binder_alloc.c |
| @@ -958,14 +958,13 @@ enum lru_status binder_alloc_free_page(s |
| |
| index = page - alloc->pages; |
| page_addr = (uintptr_t)alloc->buffer + index * PAGE_SIZE; |
| + |
| + mm = alloc->vma_vm_mm; |
| + if (!mmget_not_zero(mm)) |
| + goto err_mmget; |
| + if (!down_write_trylock(&mm->mmap_sem)) |
| + goto err_down_write_mmap_sem_failed; |
| vma = binder_alloc_get_vma(alloc); |
| - if (vma) { |
| - if (!mmget_not_zero(alloc->vma_vm_mm)) |
| - 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); |
| @@ -978,10 +977,9 @@ enum lru_status binder_alloc_free_page(s |
| PAGE_SIZE); |
| |
| 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); |
| |