mirror of
https://github.com/torvalds/linux.git
synced 2026-11-06 21:58:01 +01:00
mm: fix deadlock of mremap() against UFFDIO_MOVE
With per-VMA locks, UFFDIO_MOVE can run concurrently with mremap()
on different VMAs in the same mm. Those VMAs can share page tables, so
both operations can acquire the same pair of page-table locks.
UFFDIO_MOVE orders the locks by address, while mremap() takes the source
lock first. These conflicting orders can deadlock; mremap() holding
mmap_lock for writing no longer helps.
Move double_pt_lock() and double_pt_unlock() to mm/internal.h and use
them for all page-table moves in mremap(). Preserve the source entry
checks after acquiring both locks. Remove the comments claiming that
exclusive mmap_lock makes lock ordering irrelevant.
In move_ptes(), make sure to just recheck source and destination PMD
after taking the locks and drop the overly-long comment.
Fix generated by an LLM and sent by Chris Mason in private.
[ david: use new helpers consistently; simplify description and comments;
move_ptes() fixup ]
Fixes: 867a43a34f ("userfaultfd: use per-vma locks in userfaultfd operations")
Cc: stable@vger.kernel.org
Signed-off-by: Chris Mason <mason@kernel.org>
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Acked-by: Muchun Song <muchun.song@linux.dev>
Tested-by: Muhammad Usama Anjum <usama.anjum@arm.com>
Link: https://patch.msgid.link/20261031-fixes_core-v3-38-d6437171ecc3@kernel.org
Reviewed-by: Lance Yang <lance.yang@linux.dev>
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
This commit is contained in:
parent
9f02ca8c15
commit
d03df42dda
|
|
@ -153,8 +153,6 @@ extern int mrwprotect_range(struct userfaultfd_ctx *ctx, unsigned long start,
|
|||
unsigned long len, bool enable_rwp);
|
||||
|
||||
/* move_pages */
|
||||
void double_pt_lock(spinlock_t *ptl1, spinlock_t *ptl2);
|
||||
void double_pt_unlock(spinlock_t *ptl1, spinlock_t *ptl2);
|
||||
int move_pages_huge_pmd(struct mm_struct *mm, pmd_t *dst_pmd, pmd_t *src_pmd, pmd_t dst_pmdval,
|
||||
struct vm_area_struct *dst_vma,
|
||||
struct vm_area_struct *src_vma,
|
||||
|
|
|
|||
|
|
@ -2646,15 +2646,11 @@ bool move_huge_pmd(struct vm_area_struct *vma, unsigned long old_addr,
|
|||
return false;
|
||||
}
|
||||
|
||||
/*
|
||||
* We don't have to worry about the ordering of src and dst
|
||||
* ptlocks because exclusive mmap_lock prevents deadlock.
|
||||
*/
|
||||
old_ptl = __pmd_trans_huge_lock(old_pmd, vma);
|
||||
if (old_ptl) {
|
||||
new_ptl = pmd_lockptr(mm, new_pmd);
|
||||
if (new_ptl != old_ptl)
|
||||
spin_lock_nested(new_ptl, SINGLE_DEPTH_NESTING);
|
||||
old_ptl = pmd_lockptr(mm, old_pmd);
|
||||
new_ptl = pmd_lockptr(mm, new_pmd);
|
||||
|
||||
double_pt_lock(old_ptl, new_ptl);
|
||||
if (likely(pmd_is_huge(*old_pmd))) {
|
||||
pmd = pmdp_huge_get_and_clear(mm, old_addr, old_pmd);
|
||||
if (pmd_present(pmd))
|
||||
force_flush = true;
|
||||
|
|
@ -2683,11 +2679,10 @@ bool move_huge_pmd(struct vm_area_struct *vma, unsigned long old_addr,
|
|||
set_pmd_at(mm, new_addr, new_pmd, pmd);
|
||||
if (force_flush)
|
||||
flush_pmd_tlb_range(vma, old_addr, old_addr + PMD_SIZE);
|
||||
if (new_ptl != old_ptl)
|
||||
spin_unlock(new_ptl);
|
||||
spin_unlock(old_ptl);
|
||||
double_pt_unlock(old_ptl, new_ptl);
|
||||
return true;
|
||||
}
|
||||
double_pt_unlock(old_ptl, new_ptl);
|
||||
return false;
|
||||
}
|
||||
|
||||
|
|
|
|||
14
mm/hugetlb.c
14
mm/hugetlb.c
|
|
@ -5153,15 +5153,9 @@ static void move_huge_pte(struct vm_area_struct *vma, unsigned long old_addr,
|
|||
spinlock_t *src_ptl, *dst_ptl;
|
||||
pte_t pte;
|
||||
|
||||
dst_ptl = huge_pte_lock(h, mm, dst_pte);
|
||||
dst_ptl = huge_pte_lockptr(h, mm, dst_pte);
|
||||
src_ptl = huge_pte_lockptr(h, mm, src_pte);
|
||||
|
||||
/*
|
||||
* We don't have to worry about the ordering of src and dst ptlocks
|
||||
* because exclusive mmap_lock (or the i_mmap_lock) prevents deadlock.
|
||||
*/
|
||||
if (src_ptl != dst_ptl)
|
||||
spin_lock_nested(src_ptl, SINGLE_DEPTH_NESTING);
|
||||
double_pt_lock(dst_ptl, src_ptl);
|
||||
|
||||
pte = huge_ptep_get_and_clear(mm, old_addr, src_pte, sz);
|
||||
|
||||
|
|
@ -5190,9 +5184,7 @@ static void move_huge_pte(struct vm_area_struct *vma, unsigned long old_addr,
|
|||
set_huge_pte_at(mm, new_addr, dst_pte, pte, sz);
|
||||
}
|
||||
|
||||
if (src_ptl != dst_ptl)
|
||||
spin_unlock(src_ptl);
|
||||
spin_unlock(dst_ptl);
|
||||
double_pt_unlock(dst_ptl, src_ptl);
|
||||
}
|
||||
|
||||
int move_hugetlb_page_tables(struct vm_area_struct *vma,
|
||||
|
|
|
|||
|
|
@ -624,6 +624,34 @@ static inline void force_page_cache_readahead(struct address_space *mapping,
|
|||
force_page_cache_ra(&ractl, nr_to_read);
|
||||
}
|
||||
|
||||
/*
|
||||
* Take two page table locks at the same level in address order such that
|
||||
* we cannot deadlock.
|
||||
*/
|
||||
static inline void double_pt_lock(spinlock_t *ptl1, spinlock_t *ptl2)
|
||||
__acquires(ptl1)
|
||||
__acquires(ptl2)
|
||||
{
|
||||
if (ptl1 > ptl2)
|
||||
swap(ptl1, ptl2);
|
||||
spin_lock(ptl1);
|
||||
if (ptl1 != ptl2)
|
||||
spin_lock_nested(ptl2, SINGLE_DEPTH_NESTING);
|
||||
else
|
||||
__acquire(ptl2);
|
||||
}
|
||||
|
||||
static inline void double_pt_unlock(spinlock_t *ptl1, spinlock_t *ptl2)
|
||||
__releases(ptl1)
|
||||
__releases(ptl2)
|
||||
{
|
||||
spin_unlock(ptl1);
|
||||
if (ptl1 != ptl2)
|
||||
spin_unlock(ptl2);
|
||||
else
|
||||
__release(ptl2);
|
||||
}
|
||||
|
||||
unsigned find_lock_entries(struct address_space *mapping, pgoff_t *start,
|
||||
pgoff_t end, struct folio_batch *fbatch, pgoff_t *indices);
|
||||
unsigned find_get_entries(struct address_space *mapping, pgoff_t *start,
|
||||
|
|
|
|||
87
mm/mremap.c
87
mm/mremap.c
|
|
@ -202,7 +202,7 @@ static int move_ptes(struct pagetable_move_control *pmc,
|
|||
struct mm_struct *mm = vma->vm_mm;
|
||||
pte_t *old_ptep, *new_ptep;
|
||||
pte_t old_pte, pte;
|
||||
pmd_t dummy_pmdval;
|
||||
pmd_t old_pmdval, new_pmdval;
|
||||
spinlock_t *old_ptl, *new_ptl;
|
||||
bool force_flush = false;
|
||||
unsigned long old_addr = pmc->old_addr;
|
||||
|
|
@ -234,31 +234,30 @@ static int move_ptes(struct pagetable_move_control *pmc,
|
|||
if (pmc->need_rmap_locks)
|
||||
take_rmap_locks(vma);
|
||||
|
||||
/*
|
||||
* We don't have to worry about the ordering of src and dst
|
||||
* pte locks because exclusive mmap_lock prevents deadlock.
|
||||
*/
|
||||
old_ptep = pte_offset_map_lock(mm, old_pmd, old_addr, &old_ptl);
|
||||
old_ptep = pte_offset_map_rw_nolock(mm, old_pmd, old_addr, &old_pmdval,
|
||||
&old_ptl);
|
||||
if (!old_ptep) {
|
||||
err = -EAGAIN;
|
||||
goto out;
|
||||
}
|
||||
/*
|
||||
* Now new_pte is none, so collapse_scan_file() path can not find
|
||||
* this by traversing file->f_mapping, so there is no concurrency with
|
||||
* retract_page_tables(). In addition, we already hold the exclusive
|
||||
* mmap_lock, so this new_pte page is stable, so there is no need to get
|
||||
* pmdval and do pmd_same() check.
|
||||
*/
|
||||
new_ptep = pte_offset_map_rw_nolock(mm, new_pmd, new_addr, &dummy_pmdval,
|
||||
&new_ptl);
|
||||
new_ptep = pte_offset_map_rw_nolock(mm, new_pmd, new_addr, &new_pmdval,
|
||||
&new_ptl);
|
||||
if (!new_ptep) {
|
||||
pte_unmap_unlock(old_ptep, old_ptl);
|
||||
pte_unmap(old_ptep);
|
||||
err = -EAGAIN;
|
||||
goto out;
|
||||
}
|
||||
if (new_ptl != old_ptl)
|
||||
spin_lock_nested(new_ptl, SINGLE_DEPTH_NESTING);
|
||||
|
||||
double_pt_lock(old_ptl, new_ptl);
|
||||
if (unlikely(!pmd_same(old_pmdval, pmdp_get_lockless(old_pmd)) ||
|
||||
!pmd_same(new_pmdval, pmdp_get_lockless(new_pmd)))) {
|
||||
double_pt_unlock(old_ptl, new_ptl);
|
||||
pte_unmap(new_ptep);
|
||||
pte_unmap(old_ptep);
|
||||
err = -EAGAIN;
|
||||
goto out;
|
||||
}
|
||||
|
||||
flush_tlb_batched_pending(vma->vm_mm);
|
||||
lazy_mmu_mode_enable();
|
||||
|
||||
|
|
@ -317,10 +316,9 @@ static int move_ptes(struct pagetable_move_control *pmc,
|
|||
lazy_mmu_mode_disable();
|
||||
if (force_flush)
|
||||
flush_tlb_range(vma, old_end - len, old_end);
|
||||
if (new_ptl != old_ptl)
|
||||
spin_unlock(new_ptl);
|
||||
double_pt_unlock(old_ptl, new_ptl);
|
||||
pte_unmap(new_ptep - 1);
|
||||
pte_unmap_unlock(old_ptep - 1, old_ptl);
|
||||
pte_unmap(old_ptep - 1);
|
||||
out:
|
||||
if (pmc->need_rmap_locks)
|
||||
drop_rmap_locks(vma);
|
||||
|
|
@ -395,14 +393,9 @@ static bool move_normal_pmd(struct pagetable_move_control *pmc,
|
|||
if (WARN_ON_ONCE(!pmd_none(*new_pmd)))
|
||||
return false;
|
||||
|
||||
/*
|
||||
* We don't have to worry about the ordering of src and dst
|
||||
* ptlocks because exclusive mmap_lock prevents deadlock.
|
||||
*/
|
||||
old_ptl = pmd_lock(mm, old_pmd);
|
||||
old_ptl = pmd_lockptr(mm, old_pmd);
|
||||
new_ptl = pmd_lockptr(mm, new_pmd);
|
||||
if (new_ptl != old_ptl)
|
||||
spin_lock_nested(new_ptl, SINGLE_DEPTH_NESTING);
|
||||
double_pt_lock(old_ptl, new_ptl);
|
||||
|
||||
pmd = *old_pmd;
|
||||
|
||||
|
|
@ -418,9 +411,7 @@ static bool move_normal_pmd(struct pagetable_move_control *pmc,
|
|||
pmd_populate(mm, new_pmd, pmd_pgtable(pmd));
|
||||
flush_tlb_range(vma, pmc->old_addr, pmc->old_addr + PMD_SIZE);
|
||||
out_unlock:
|
||||
if (new_ptl != old_ptl)
|
||||
spin_unlock(new_ptl);
|
||||
spin_unlock(old_ptl);
|
||||
double_pt_unlock(old_ptl, new_ptl);
|
||||
|
||||
return res;
|
||||
}
|
||||
|
|
@ -452,14 +443,10 @@ static bool move_normal_pud(struct pagetable_move_control *pmc,
|
|||
if (WARN_ON_ONCE(!pud_none(*new_pud)))
|
||||
return false;
|
||||
|
||||
/*
|
||||
* We don't have to worry about the ordering of src and dst
|
||||
* ptlocks because exclusive mmap_lock prevents deadlock.
|
||||
*/
|
||||
old_ptl = pud_lock(mm, old_pud);
|
||||
old_ptl = pud_lockptr(mm, old_pud);
|
||||
new_ptl = pud_lockptr(mm, new_pud);
|
||||
if (new_ptl != old_ptl)
|
||||
spin_lock_nested(new_ptl, SINGLE_DEPTH_NESTING);
|
||||
|
||||
double_pt_lock(old_ptl, new_ptl);
|
||||
|
||||
pud = *old_pud;
|
||||
if (unlikely(pud_none(pud)))
|
||||
|
|
@ -473,9 +460,7 @@ static bool move_normal_pud(struct pagetable_move_control *pmc,
|
|||
pud_populate(mm, new_pud, pud_pgtable(pud));
|
||||
flush_tlb_range(vma, pmc->old_addr, pmc->old_addr + PUD_SIZE);
|
||||
out_unlock:
|
||||
if (new_ptl != old_ptl)
|
||||
spin_unlock(new_ptl);
|
||||
spin_unlock(old_ptl);
|
||||
double_pt_unlock(old_ptl, new_ptl);
|
||||
|
||||
return true;
|
||||
}
|
||||
|
|
@ -503,16 +488,14 @@ static bool move_huge_pud(struct pagetable_move_control *pmc,
|
|||
if (WARN_ON_ONCE(!pud_none(*new_pud)))
|
||||
return false;
|
||||
|
||||
/*
|
||||
* We don't have to worry about the ordering of src and dst
|
||||
* ptlocks because exclusive mmap_lock prevents deadlock.
|
||||
*/
|
||||
old_ptl = __pud_trans_huge_lock(old_pud, vma);
|
||||
if (unlikely(!old_ptl))
|
||||
return false;
|
||||
old_ptl = pud_lockptr(mm, old_pud);
|
||||
new_ptl = pud_lockptr(mm, new_pud);
|
||||
if (new_ptl != old_ptl)
|
||||
spin_lock_nested(new_ptl, SINGLE_DEPTH_NESTING);
|
||||
|
||||
double_pt_lock(old_ptl, new_ptl);
|
||||
if (unlikely(!pud_trans_huge(*old_pud))) {
|
||||
double_pt_unlock(old_ptl, new_ptl);
|
||||
return false;
|
||||
}
|
||||
|
||||
/* Clear the pud */
|
||||
pud = pudp_huge_get_and_clear(mm, pmc->old_addr, old_pud);
|
||||
|
|
@ -523,9 +506,7 @@ static bool move_huge_pud(struct pagetable_move_control *pmc,
|
|||
/* mark soft_ditry when we add pud level soft dirty support */
|
||||
set_pud_at(mm, pmc->new_addr, new_pud, pud);
|
||||
flush_pud_tlb_range(vma, pmc->old_addr, pmc->old_addr + HPAGE_PUD_SIZE);
|
||||
if (new_ptl != old_ptl)
|
||||
spin_unlock(new_ptl);
|
||||
spin_unlock(old_ptl);
|
||||
double_pt_unlock(old_ptl, new_ptl);
|
||||
|
||||
return true;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1206,33 +1206,6 @@ int mrwprotect_range(struct userfaultfd_ctx *ctx, unsigned long start,
|
|||
return 0;
|
||||
}
|
||||
|
||||
void double_pt_lock(spinlock_t *ptl1,
|
||||
spinlock_t *ptl2)
|
||||
__acquires(ptl1)
|
||||
__acquires(ptl2)
|
||||
{
|
||||
if (ptl1 > ptl2)
|
||||
swap(ptl1, ptl2);
|
||||
/* lock in virtual address order to avoid lock inversion */
|
||||
spin_lock(ptl1);
|
||||
if (ptl1 != ptl2)
|
||||
spin_lock_nested(ptl2, SINGLE_DEPTH_NESTING);
|
||||
else
|
||||
__acquire(ptl2);
|
||||
}
|
||||
|
||||
void double_pt_unlock(spinlock_t *ptl1,
|
||||
spinlock_t *ptl2)
|
||||
__releases(ptl1)
|
||||
__releases(ptl2)
|
||||
{
|
||||
spin_unlock(ptl1);
|
||||
if (ptl1 != ptl2)
|
||||
spin_unlock(ptl2);
|
||||
else
|
||||
__release(ptl2);
|
||||
}
|
||||
|
||||
static inline bool is_pte_pages_stable(pte_t *dst_pte, pte_t *src_pte,
|
||||
pte_t orig_dst_pte, pte_t orig_src_pte,
|
||||
pmd_t *dst_pmd, pmd_t dst_pmdval)
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user