diff mbox series

[5/5] userfaultfd: shmem: UFFDIO_COPY: set the page dirty if VM_WRITE is not set

Message ID 20181126173452.26955-6-aarcange@redhat.com (mailing list archive)
State New, archived
Headers show
Series userfaultfd shmem updates | expand

Commit Message

Andrea Arcangeli Nov. 26, 2018, 5:34 p.m. UTC 
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.

Reviewed-by: Hugh Dickins <hughd@google.com>
Cc: stable@vger.kernel.org
Fixes: 4c27fe4c4c84 ("userfaultfd: shmem: add shmem_mcopy_atomic_pte for userfaultfd support")
Reported-by: Hugh Dickins <hughd@google.com>
Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
---
 mm/shmem.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Sasha Levin Nov. 27, 2018, 6:57 a.m. UTC | #1 
Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: 4c27fe4c4c84 userfaultfd: shmem: add shmem_mcopy_atomic_pte for userfaultfd support.

The bot has tested the following trees: v4.19.4, v4.14.83, 

v4.19.4: Build OK!
v4.14.83: Failed to apply! Possible dependencies:
    2a70f6a76bb8 ("memcg, thp: do not invoke oom killer on thp charges")
    2cf855837b89 ("memcontrol: schedule throttling if we are congested")
    fa27dfd489b9 ("userfaultfd: shmem: add i_size checks")


How should we proceed with this patch?

--
Thanks,
Sasha
diff mbox series

Patch

diff --git a/mm/shmem.c b/mm/shmem.c
index c3ece7a51949..82a381d463bc 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2272,6 +2272,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);
 
@@ -2305,6 +2315,7 @@  static int shmem_mfill_atomic_pte(struct mm_struct *dst_mm,
 	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);