| Message ID | 20190820160237.GB4983@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | userfaultfd_release: always remove uffd flags and clear vm_userfaultfd_ctx | expand |
On Tue, Aug 20, 2019 at 06:02:38PM +0200, Oleg Nesterov wrote: > userfaultfd_release() should clear vm_flags/vm_userfaultfd_ctx even > if mm->core_state != NULL. > > Otherwise a page fault can see userfaultfd_missing() == T and use an > already freed userfaultfd_ctx. > > Reported-by: Kefeng Wang <wangkefeng.wang@huawei.com> > Fixes: 04f5866e41fb ("coredump: fix race condition between mmget_not_zero()/get_task_mm() and core dumping") > Cc: stable@vger.kernel.org > Signed-off-by: Oleg Nesterov <oleg@redhat.com> > --- > fs/userfaultfd.c | 25 +++++++++++++------------ > 1 file changed, 13 insertions(+), 12 deletions(-) Reviewed-by: Andrea Arcangeli <aarcange@redhat.com> Thanks, Andrea
On 2019/8/21 0:02, Oleg Nesterov wrote: > userfaultfd_release() should clear vm_flags/vm_userfaultfd_ctx even > if mm->core_state != NULL. > > Otherwise a page fault can see userfaultfd_missing() == T and use an > already freed userfaultfd_ctx. > > Reported-by: Kefeng Wang <wangkefeng.wang@huawei.com> > Fixes: 04f5866e41fb ("coredump: fix race condition between mmget_not_zero()/get_task_mm() and core dumping") > Cc: stable@vger.kernel.org > Signed-off-by: Oleg Nesterov <oleg@redhat.com> > --- > fs/userfaultfd.c | 25 +++++++++++++------------ > 1 file changed, 13 insertions(+), 12 deletions(-) Tested on lts4.4 and 5.3-rc4, Thanks.
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index ccbdbd6..fe6d804 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -880,6 +880,7 @@ static int userfaultfd_release(struct inode *inode, struct file *file) /* len == 0 means wake all */ struct userfaultfd_wake_range range = { .len = 0, }; unsigned long new_flags; + bool still_valid; WRITE_ONCE(ctx->released, true); @@ -895,8 +896,7 @@ static int userfaultfd_release(struct inode *inode, struct file *file) * taking the mmap_sem for writing. */ down_write(&mm->mmap_sem); - if (!mmget_still_valid(mm)) - goto skip_mm; + still_valid = mmget_still_valid(mm); prev = NULL; for (vma = mm->mmap; vma; vma = vma->vm_next) { cond_resched(); @@ -907,19 +907,20 @@ static int userfaultfd_release(struct inode *inode, struct file *file) continue; } new_flags = vma->vm_flags & ~(VM_UFFD_MISSING | VM_UFFD_WP); - prev = vma_merge(mm, prev, vma->vm_start, vma->vm_end, - new_flags, vma->anon_vma, - vma->vm_file, vma->vm_pgoff, - vma_policy(vma), - NULL_VM_UFFD_CTX); - if (prev) - vma = prev; - else - prev = vma; + if (still_valid) { + prev = vma_merge(mm, prev, vma->vm_start, vma->vm_end, + new_flags, vma->anon_vma, + vma->vm_file, vma->vm_pgoff, + vma_policy(vma), + NULL_VM_UFFD_CTX); + if (prev) + vma = prev; + else + prev = vma; + } vma->vm_flags = new_flags; vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX; } -skip_mm: up_write(&mm->mmap_sem); mmput(mm); wakeup:
userfaultfd_release() should clear vm_flags/vm_userfaultfd_ctx even if mm->core_state != NULL. Otherwise a page fault can see userfaultfd_missing() == T and use an already freed userfaultfd_ctx. Reported-by: Kefeng Wang <wangkefeng.wang@huawei.com> Fixes: 04f5866e41fb ("coredump: fix race condition between mmget_not_zero()/get_task_mm() and core dumping") Cc: stable@vger.kernel.org Signed-off-by: Oleg Nesterov <oleg@redhat.com> --- fs/userfaultfd.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)