Commit 3ea27719 authored Aug 02, 2017 by Mel Gorman Committed by Linus Torvalds Aug 02, 2017

mm, mprotect: flush TLB if potentially racing with a parallel reclaim leaving stale TLB entries

Nadav Amit identified a theoritical race between page reclaim and
mprotect due to TLB flushes being batched outside of the PTL being held.

He described the race as follows:

        CPU0                            CPU1
        ----                            ----
                                        user accesses memory using RW PTE
                                        [PTE now cached in TLB]
        try_to_unmap_one()
        ==> ptep_get_and_clear()
        ==> set_tlb_ubc_flush_pending()
                                        mprotect(addr, PROT_READ)
                                        ==> change_pte_range()
                                        ==> [ PTE non-present - no flush ]

                                        user writes using cached RW PTE
        ...

        try_to_unmap_flush()

The same type of race exists for reads when protecting for PROT_NONE and
also exists for operations that can leave an old TLB entry behind such
as munmap, mremap and madvise.

For some operations like mprotect, it's not necessarily a data integrity
issue but it is a correctness issue as there is a window where an
mprotect that limits access still allows access.  For munmap, it's
potentially a data integrity issue although the race is massive as an
munmap, mmap and return to userspace must all complete between the
window when reclaim drops the PTL and flushes the TLB.  However, it's
theoritically possible so handle this issue by flushing the mm if
reclaim is potentially currently batching TLB flushes.

Other instances where a flush is required for a present pte should be ok
as either the page lock is held preventing parallel reclaim or a page
reference count is elevated preventing a parallel free leading to
corruption.  In the case of page_mkclean there isn't an obvious path
that userspace could take advantage of without using the operations that
are guarded by this patch.  Other users such as gup as a race with
reclaim looks just at PTEs.  huge page variants should be ok as they
don't race with reclaim.  mincore only looks at PTEs.  userfault also
should be ok as if a parallel reclaim takes place, it will either fault
the page back in or read some of the data before the flush occurs
triggering a fault.

Note that a variant of this patch was acked by Andy Lutomirski but this
was for the x86 parts on top of his PCID work which didn't make the 4.13
merge window as expected.  His ack is dropped from this version and
there will be a follow-on patch on top of PCID that will include his
ack.

[akpm@linux-foundation.org: tweak comments]
[akpm@linux-foundation.org: fix spello]
Link: http://lkml.kernel.org/r/20170717155523.emckq2esjro6hf3z@suse.de


Reported-by: Nadav Amit <nadav.amit@gmail.com>
Signed-off-by: Mel Gorman <mgorman@suse.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: <stable@vger.kernel.org>	[v4.4+]
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

parent 27e37d84

include/linux/mm_types.h

+4 −0

Original line number	Diff line number	Diff line
		@@ -494,6 +494,10 @@ struct mm_struct {
		* PROT_NONE or PROT_NUMA mapped page.
		*/
		bool tlb_flush_pending;
		#endif
		#ifdef CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
		/* See flush_tlb_batched_pending() */
		bool tlb_flush_batched;
		#endif
		struct uprobes_state uprobes_state;
		#ifdef CONFIG_HUGETLB_PAGE

mm/internal.h

+4 −1

Original line number	Diff line number	Diff line
		@@ -498,6 +498,7 @@ extern struct workqueue_struct *mm_percpu_wq;
		#ifdef CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
		void try_to_unmap_flush(void);
		void try_to_unmap_flush_dirty(void);
		void flush_tlb_batched_pending(struct mm_struct *mm);
		#else
		static inline void try_to_unmap_flush(void)
		{
		@@ -505,7 +506,9 @@ static inline void try_to_unmap_flush(void)
		static inline void try_to_unmap_flush_dirty(void)
		{
		}

		static inline void flush_tlb_batched_pending(struct mm_struct *mm)
		{
		}
		#endif /* CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH */

		extern const struct trace_print_flags pageflag_names[];

mm/madvise.c

+1 −0

Original line number	Diff line number	Diff line
		@@ -320,6 +320,7 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,

		tlb_remove_check_page_size_change(tlb, PAGE_SIZE);
		orig_pte = pte = pte_offset_map_lock(mm, pmd, addr, &ptl);
		flush_tlb_batched_pending(mm);
		arch_enter_lazy_mmu_mode();
		for (; addr != end; pte++, addr += PAGE_SIZE) {
		ptent = *pte;

mm/memory.c

+1 −0

Original line number	Diff line number	Diff line
		@@ -1197,6 +1197,7 @@ again:
		init_rss_vec(rss);
		start_pte = pte_offset_map_lock(mm, pmd, addr, &ptl);
		pte = start_pte;
		flush_tlb_batched_pending(mm);
		arch_enter_lazy_mmu_mode();
		do {
		pte_t ptent = *pte;

mm/mprotect.c

+1 −0

Original line number	Diff line number	Diff line
		@@ -64,6 +64,7 @@ static unsigned long change_pte_range(struct vm_area_struct vma, pmd_t pmd,
		atomic_read(&vma->vm_mm->mm_users) == 1)
		target_node = numa_node_id();

		flush_tlb_batched_pending(vma->vm_mm);
		arch_enter_lazy_mmu_mode();
		do {
		oldpte = *pte;

Admin message