mm: Fix use-after-free for MMU_GATHER_NO_GATHER
Subsystems (3)
PATCH 1/1 mm: Fix use-after-free for MMU_GATHER_NO_GATHER
The recent change to batch-zap anonymous ptes did not take into account that for platforms where MMU_GATHER_NO_GATHER is enabled (e.g. s390), __tlb_remove_page() drops a reference to the page. This means that the folio reference count can drop to zero while still in use (i.e. before folio_remove_rmap_range() is called). This does not happen on other platforms because the actual page freeing is deferred.
Solve this by appropriately getting/putting the folio to guarrantee it does not get freed early.
Given the new need to get/put the folio in the batch path, let's stick to the non-batched path if the folio is not large. In this case batching is not helpful since the batch size is 1.
---
Hi Andrew,
This fixes patch 3 in the series at [1], which is currently in mm-unstable. I'm not sure whether you want to take the fix or whether I should re-post the entire series?
Thanks,
Ryan
mm/memory.c | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-)
mm/memory.c +27 -15
@@ -1456,6 +1456,9 @@ static unsigned long try_zap_anon_pte_range(struct mmu_gather *tlb,bool full;int i; +/* __tlb_remove_page drops a ref; prevent it going to 0 while in use. */ +folio_get (folio); +for (i =0 ; i < nr_pages;) { ptent =ptep_get_and_clear_full (mm, addr, pte, tlb->fullmm);tlb_remove_tlb_entry (tlb, pte, addr);
@@ -1476,6 +1479,8 @@ static unsigned long try_zap_anon_pte_range(struct mmu_gather *tlb,folio_remove_rmap_range (folio, page - i, i, vma); +folio_put (folio); +return i; }
@@ -1526,26 +1531,33 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,*/ if (page &&PageAnon (page)) {struct folio *folio =page_folio (page); -int nr_pages_req, nr_pages; - nr_pages_req =folio_nr_pages_cont_mapped ( - folio, page, pte, addr, end); +if (folio_test_large (folio)) { +int nr_pages_req, nr_pages; +int counter =mm_counter (page); - nr_pages =try_zap_anon_pte_range (tlb, vma, - folio, page, pte, addr, - nr_pages_req, details); + nr_pages_req =folio_nr_pages_cont_mapped ( + folio, page, pte, addr, + end); - rss[mm_counter (page)] -= nr_pages; - nr_pages--; - pte += nr_pages; - addr += nr_pages <<PAGE_SHIFT ; +/* folio may be freed on return. */ + nr_pages =try_zap_anon_pte_range ( + tlb, vma, folio, page, + pte, addr, nr_pages_req, + details); -if (unlikely (nr_pages < nr_pages_req)) { - force_flush =1 ; - addr +=PAGE_SIZE ; -break ; + rss[counter] -= nr_pages; + nr_pages--; + pte += nr_pages; + addr += nr_pages <<PAGE_SHIFT ; + +if (unlikely (nr_pages < nr_pages_req)) { + force_flush =1 ; + addr +=PAGE_SIZE ; +break ; + } +continue ; } -continue ; } ptent = ptep_get_and_clear_full(mm, addr, pte,
--
2.25.1
Quoted 18 lines · Ryan Roberts
The recent change to batch-zap anonymous ptes did not take into account that for platforms where
MMU_GATHER_NO_GATHERis enabled (e.g. s390),__tlb_remove_page()drops a reference to the page. This means that the folio reference count can drop to zero while still in use (i.e. beforefolio_remove_rmap_range()is called). This does not happen on other platforms because the actual page freeing is deferred.Solve this by appropriately getting/putting the folio to guarrantee it does not get freed early.
Given the new need to get/put the folio in the batch path, let's stick to the non-batched path if the folio is not large. In this case batching is not helpful since the batch size is 1.
Signed-off-by Ryan Roberts <[email protected]>Fixes 904d9713b3b0 ("mm: batch-zap large anonymous folio PTE mappings")Reported-by Nathan Chancellor <[email protected]>
Quoted 8 lines
---
Hi Andrew,
This fixes patch 3 in the series at [1], which is currently in mm-unstable. I'm not sure whether you want to take the fix or whether I should re-post the entire series?
Please repost the complete thing, you're touching some sensible places that really need decent review.
Signature · 3 lines
Cheers, David / dhildenb
Sven Schnelle
reviewerRyan,
Quoted 26 lines · David Hildenbrand
Quoted 17 lines · Ryan Roberts
The recent change to batch-zap anonymous ptes did not take into account that for platforms where
MMU_GATHER_NO_GATHERis enabled (e.g. s390),__tlb_remove_page()drops a reference to the page. This means that the folio reference count can drop to zero while still in use (i.e. beforefolio_remove_rmap_range()is called). This does not happen on other platforms because the actual page freeing is deferred.
Solve this by appropriately getting/putting the folio to guarrantee it
does not get freed early.
Given the new need to get/put the folio in the batch path, let's stick
to the non-batched path if the folio is not large. In this case batching is not helpful since the batch size is 1.Signed-off-by Ryan Roberts <[email protected]>Fixes 904d9713b3b0 ("mm: batch-zap large anonymous folio PTE mappings")Reported-by Nathan Chancellor <[email protected]>Quoted 7 lines
---
Hi Andrew,
This fixes patch 3 in the series at [1], which is currently in mm-unstable. I'm
not sure whether you want to take the fix or whether I should re-post the entire series?
Quoted 2 lines
Please repost the complete thing, you're touching some sensible places that really need decent review.
Please also add:
Alexander Gordeev <[email protected]>
Gerald Schaefer <[email protected]>
when reposting. Thanks!