Skip to content

Commit 930ca9e

Browse files
Ryan RobertsSasha Levin
authored andcommitted
mm: close theoretical race where stale TLB entries could linger
commit 383c461 upstream. Commit 3ea2771 ("mm, mprotect: flush TLB if potentially racing with a parallel reclaim leaving stale TLB entries") described a theoretical race as such: """ Nadav Amit identified a theoretical 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. """ The solution was to introduce flush_tlb_batched_pending() and call it under the PTL from mprotect/madvise/munmap/mremap to complete any pending tlb flushes. However, while madvise_free_pte_range() and madvise_cold_or_pageout_pte_range() were both retro-fitted to call flush_tlb_batched_pending() immediately after initially acquiring the PTL, they both temporarily release the PTL to split a large folio if they stumble upon one. In this case, where re-acquiring the PTL flush_tlb_batched_pending() must be called again, but it previously was not. Let's fix that. There are 2 Fixes: tags here: the first is the commit that fixed madvise_free_pte_range(). The second is the commit that added madvise_cold_or_pageout_pte_range(), which looks like it copy/pasted the faulty pattern from madvise_free_pte_range(). This is a theoretical bug discovered during code review. Link: https://lkml.kernel.org/r/[email protected] Fixes: 3ea2771 ("mm, mprotect: flush TLB if potentially racing with a parallel reclaim leaving stale TLB entries") Fixes: 9c276cc ("mm: introduce MADV_COLD") Signed-off-by: Ryan Roberts <[email protected]> Reviewed-by: Jann Horn <[email protected]> Acked-by: David Hildenbrand <[email protected]> Cc: Liam Howlett <[email protected]> Cc: Lorenzo Stoakes <[email protected]> Cc: Mel Gorman <mgorman <[email protected]> Cc: Vlastimil Babka <[email protected]> Cc: <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 903f0a8 commit 930ca9e

File tree

1 file changed

+2
-0
lines changed

1 file changed

+2
-0
lines changed

mm/madvise.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
495495
pte_offset_map_lock(mm, pmd, addr, &ptl);
496496
if (!start_pte)
497497
break;
498+
flush_tlb_batched_pending(mm);
498499
arch_enter_lazy_mmu_mode();
499500
if (!err)
500501
nr = 0;
@@ -728,6 +729,7 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
728729
start_pte = pte;
729730
if (!start_pte)
730731
break;
732+
flush_tlb_batched_pending(mm);
731733
arch_enter_lazy_mmu_mode();
732734
if (!err)
733735
nr = 0;

0 commit comments

Comments
 (0)