userfaultfd: shmem: UFFDIO_COPY: set the page dirty if VM_WRITE is not set
authorAndrea Arcangeli <[email protected]>
Fri, 30 Nov 2018 22:09:43 +0000 (14:09 -0800)
committerLinus Torvalds <[email protected]>
Fri, 30 Nov 2018 22:56:14 +0000 (14:56 -0800)
Set the page dirty if VM_WRITE is not set because in such case the pte
won't be marked dirty and the page would be reclaimed without writepage
(i.e.  swapout in the shmem case).

This was found by source review.  Most apps (certainly including QEMU)
only use UFFDIO_COPY on PROT_READ|PROT_WRITE mappings or the app can't
modify the memory in the first place.  This is for correctness and it
could help the non cooperative use case to avoid unexpected data loss.

Link: http://lkml.kernel.org/r/[email protected]
Reviewed-by: Hugh Dickins <[email protected]>
Cc: [email protected]
Fixes: 4c27fe4c4c84 ("userfaultfd: shmem: add shmem_mcopy_atomic_pte for userfaultfd support")
Reported-by: Hugh Dickins <[email protected]>
Signed-off-by: Andrea Arcangeli <[email protected]>
Cc: "Dr. David Alan Gilbert" <[email protected]>
Cc: Jann Horn <[email protected]>
Cc: Mike Kravetz <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Peter Xu <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
mm/shmem.c

index 99d5867daadb1fb8341f585d91377c0619cf49aa..16a3d7044c52b17014e5d4acf2986c763b7ea91c 100644 (file)
@@ -2274,6 +2274,16 @@ static int shmem_mfill_atomic_pte(struct mm_struct *dst_mm,
        _dst_pte = mk_pte(page, dst_vma->vm_page_prot);
        if (dst_vma->vm_flags & VM_WRITE)
                _dst_pte = pte_mkwrite(pte_mkdirty(_dst_pte));
+       else {
+               /*
+                * We don't set the pte dirty if the vma has no
+                * VM_WRITE permission, so mark the page dirty or it
+                * could be freed from under us. We could do it
+                * unconditionally before unlock_page(), but doing it
+                * only if VM_WRITE is not set is faster.
+                */
+               set_page_dirty(page);
+       }
 
        dst_pte = pte_offset_map_lock(dst_mm, dst_pmd, dst_addr, &ptl);
 
@@ -2307,6 +2317,7 @@ out:
        return ret;
 out_release_uncharge_unlock:
        pte_unmap_unlock(dst_pte, ptl);
+       ClearPageDirty(page);
        delete_from_page_cache(page);
 out_release_uncharge:
        mem_cgroup_cancel_charge(page, memcg, false);