Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

mremap: fix race between mremap() and page cleanning

Prior to 3.15, there was a race between zap_pte_range() and
page_mkclean() where writes to a page could be lost. Dave Hansen
discovered by inspection that there is a similar race between
move_ptes() and page_mkclean().

We've been able to reproduce the issue by enlarging the race window with
a msleep(), but have not been able to hit it without modifying the code.
So, we think it's a real issue, but is difficult or impossible to hit in
practice.

The zap_pte_range() issue is fixed by commit 1cf35d47712d("mm: split
'tlb_flush_mmu()' into tlb flushing and memory freeing parts"). And
this patch is to fix the race between page_mkclean() and mremap().

Here is one possible way to hit the race: suppose a process mmapped a
file with READ | WRITE and SHARED, it has two threads and they are bound
to 2 different CPUs, e.g. CPU1 and CPU2. mmap returned X, then thread
1 did a write to addr X so that CPU1 now has a writable TLB for addr X
on it. Thread 2 starts mremaping from addr X to Y while thread 1
cleaned the page and then did another write to the old addr X again.
The 2nd write from thread 1 could succeed but the value will get lost.

thread 1 thread 2
(bound to CPU1) (bound to CPU2)

1: write 1 to addr X to get a
writeable TLB on this CPU

2: mremap starts

3: move_ptes emptied PTE for addr X
and setup new PTE for addr Y and
then dropped PTL for X and Y

4: page laundering for N by doing
fadvise FADV_DONTNEED. When done,
pageframe N is deemed clean.

5: *write 2 to addr X

6: tlb flush for addr X

7: munmap (Y, pagesize) to make the
page unmapped

8: fadvise with FADV_DONTNEED again
to kick the page off the pagecache

9: pread the page from file to verify
the value. If 1 is there, it means
we have lost the written 2.

*the write may or may not cause segmentation fault, it depends on
if the TLB is still on the CPU.

Please note that this is only one specific way of how the race could
occur, it didn't mean that the race could only occur in exact the above
config, e.g. more than 2 threads could be involved and fadvise() could
be done in another thread, etc.

For anonymous pages, they could race between mremap() and page reclaim:
THP: a huge PMD is moved by mremap to a new huge PMD, then the new huge
PMD gets unmapped/splitted/pagedout before the flush tlb happened for
the old huge PMD in move_page_tables() and we could still write data to
it. The normal anonymous page has similar situation.

To fix this, check for any dirty PTE in move_ptes()/move_huge_pmd() and
if any, did the flush before dropping the PTL. If we did the flush for
every move_ptes()/move_huge_pmd() call then we do not need to do the
flush in move_pages_tables() for the whole range. But if we didn't, we
still need to do the whole range flush.

Alternatively, we can track which part of the range is flushed in
move_ptes()/move_huge_pmd() and which didn't to avoid flushing the whole
range in move_page_tables(). But that would require multiple tlb
flushes for the different sub-ranges and should be less efficient than
the single whole range flush.

KBuild test on my Sandybridge desktop doesn't show any noticeable change.
v4.9-rc4:
real 5m14.048s
user 32m19.800s
sys 4m50.320s

With this commit:
real 5m13.888s
user 32m19.330s
sys 4m51.200s

Reported-by: Dave Hansen <dave.hansen@intel.com>
Signed-off-by: Aaron Lu <aaron.lu@intel.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Aaron Lu and committed by
Linus Torvalds
5d190420 961b708e

+30 -11
+1 -1
include/linux/huge_mm.h
··· 22 22 unsigned char *vec); 23 23 extern bool move_huge_pmd(struct vm_area_struct *vma, unsigned long old_addr, 24 24 unsigned long new_addr, unsigned long old_end, 25 - pmd_t *old_pmd, pmd_t *new_pmd); 25 + pmd_t *old_pmd, pmd_t *new_pmd, bool *need_flush); 26 26 extern int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd, 27 27 unsigned long addr, pgprot_t newprot, 28 28 int prot_numa);
+8 -1
mm/huge_memory.c
··· 1426 1426 1427 1427 bool move_huge_pmd(struct vm_area_struct *vma, unsigned long old_addr, 1428 1428 unsigned long new_addr, unsigned long old_end, 1429 - pmd_t *old_pmd, pmd_t *new_pmd) 1429 + pmd_t *old_pmd, pmd_t *new_pmd, bool *need_flush) 1430 1430 { 1431 1431 spinlock_t *old_ptl, *new_ptl; 1432 1432 pmd_t pmd; 1433 1433 struct mm_struct *mm = vma->vm_mm; 1434 + bool force_flush = false; 1434 1435 1435 1436 if ((old_addr & ~HPAGE_PMD_MASK) || 1436 1437 (new_addr & ~HPAGE_PMD_MASK) || ··· 1456 1455 new_ptl = pmd_lockptr(mm, new_pmd); 1457 1456 if (new_ptl != old_ptl) 1458 1457 spin_lock_nested(new_ptl, SINGLE_DEPTH_NESTING); 1458 + if (pmd_present(*old_pmd) && pmd_dirty(*old_pmd)) 1459 + force_flush = true; 1459 1460 pmd = pmdp_huge_get_and_clear(mm, old_addr, old_pmd); 1460 1461 VM_BUG_ON(!pmd_none(*new_pmd)); 1461 1462 ··· 1470 1467 set_pmd_at(mm, new_addr, new_pmd, pmd_mksoft_dirty(pmd)); 1471 1468 if (new_ptl != old_ptl) 1472 1469 spin_unlock(new_ptl); 1470 + if (force_flush) 1471 + flush_tlb_range(vma, old_addr, old_addr + PMD_SIZE); 1472 + else 1473 + *need_flush = true; 1473 1474 spin_unlock(old_ptl); 1474 1475 return true; 1475 1476 }
+21 -9
mm/mremap.c
··· 104 104 static void move_ptes(struct vm_area_struct *vma, pmd_t *old_pmd, 105 105 unsigned long old_addr, unsigned long old_end, 106 106 struct vm_area_struct *new_vma, pmd_t *new_pmd, 107 - unsigned long new_addr, bool need_rmap_locks) 107 + unsigned long new_addr, bool need_rmap_locks, bool *need_flush) 108 108 { 109 109 struct mm_struct *mm = vma->vm_mm; 110 110 pte_t *old_pte, *new_pte, pte; 111 111 spinlock_t *old_ptl, *new_ptl; 112 + bool force_flush = false; 113 + unsigned long len = old_end - old_addr; 112 114 113 115 /* 114 116 * When need_rmap_locks is true, we take the i_mmap_rwsem and anon_vma ··· 148 146 new_pte++, new_addr += PAGE_SIZE) { 149 147 if (pte_none(*old_pte)) 150 148 continue; 149 + 150 + /* 151 + * We are remapping a dirty PTE, make sure to 152 + * flush TLB before we drop the PTL for the 153 + * old PTE or we may race with page_mkclean(). 154 + */ 155 + if (pte_present(*old_pte) && pte_dirty(*old_pte)) 156 + force_flush = true; 151 157 pte = ptep_get_and_clear(mm, old_addr, old_pte); 152 158 pte = move_pte(pte, new_vma->vm_page_prot, old_addr, new_addr); 153 159 pte = move_soft_dirty_pte(pte); ··· 166 156 if (new_ptl != old_ptl) 167 157 spin_unlock(new_ptl); 168 158 pte_unmap(new_pte - 1); 159 + if (force_flush) 160 + flush_tlb_range(vma, old_end - len, old_end); 161 + else 162 + *need_flush = true; 169 163 pte_unmap_unlock(old_pte - 1, old_ptl); 170 164 if (need_rmap_locks) 171 165 drop_rmap_locks(vma); ··· 215 201 if (need_rmap_locks) 216 202 take_rmap_locks(vma); 217 203 moved = move_huge_pmd(vma, old_addr, new_addr, 218 - old_end, old_pmd, new_pmd); 204 + old_end, old_pmd, new_pmd, 205 + &need_flush); 219 206 if (need_rmap_locks) 220 207 drop_rmap_locks(vma); 221 - if (moved) { 222 - need_flush = true; 208 + if (moved) 223 209 continue; 224 - } 225 210 } 226 211 split_huge_pmd(vma, old_pmd, old_addr); 227 212 if (pmd_trans_unstable(old_pmd)) ··· 233 220 extent = next - new_addr; 234 221 if (extent > LATENCY_LIMIT) 235 222 extent = LATENCY_LIMIT; 236 - move_ptes(vma, old_pmd, old_addr, old_addr + extent, 237 - new_vma, new_pmd, new_addr, need_rmap_locks); 238 - need_flush = true; 223 + move_ptes(vma, old_pmd, old_addr, old_addr + extent, new_vma, 224 + new_pmd, new_addr, need_rmap_locks, &need_flush); 239 225 } 240 - if (likely(need_flush)) 226 + if (need_flush) 241 227 flush_tlb_range(vma, old_end-len, old_addr); 242 228 243 229 mmu_notifier_invalidate_range_end(vma->vm_mm, mmun_start, mmun_end);