hugetlbfs: remove call to huge_pte_alloc without i_mmap_rwsem

Commit c0d0381ade79 ("hugetlbfs: use i_mmap_rwsem for more pmd sharing synchronization") requires callers of huge_pte_alloc to hold i_mmap_rwsem in at least read mode. This is because the explicit locking in huge_pmd_share (called by huge_pte_alloc) was removed. When restructuring the code, the call to huge_pte_alloc in the else block at the beginning of hugetlb_fault was missed. Unfortunately, that else clause is exercised when there is no page table entry. This will likely lead to a call to huge_pmd_share. If huge_pmd_share thinks pmd sharing is possible, it will traverse the mapping tree (i_mmap) without holding i_mmap_rwsem. If someone else is modifying the tree, bad things such as addressing exceptions or worse could happen. Simply remove the else clause. It should have been removed previously. The code following the else will call huge_pte_alloc with the appropriate locking. To prevent this type of issue in the future, add routines to assert that i_mmap_rwsem is held, and call these routines in huge pmd sharing routines.

Mike Kravetz committed 6 years ago

Commit 34ae2041 parent 15568294 files · +23 −12Browse files

Unified · Split

4 files

include/linux/fs.h +10 −0
include/linux/hugetlb.h +5 −3
mm/hugetlb.c +7 −8
mm/rmap.c +1 −1

include/linux/fs.h+10−0

@@ -518,6 +518,16 @@ static inline void i_mmap_unlock_read(struct address_space *mapping) · 517 unmodified lines
518	518	up_read(&mapping->i_mmap_rwsem);
519	519	}
520	520
	521	+static inline void i_mmap_assert_locked(struct address_space *mapping)
	522	+{
	523	+ lockdep_assert_held(&mapping->i_mmap_rwsem);
	524	+}
	525	+
	526	+static inline void i_mmap_assert_write_locked(struct address_space *mapping)
	527	+{
	528	+ lockdep_assert_held_write(&mapping->i_mmap_rwsem);
	529	+}
	530	+
521	531	/*
522	532	* Might pages of this file be mapped into userspace?
523	533	*/

include/linux/hugetlb.h+5−3

@@ -164,7 +164,8 @@ pte_t huge_pte_alloc(struct mm_struct mm, · 163 unmodified lines
164	164	unsigned long addr, unsigned long sz);
165	165	pte_t huge_pte_offset(struct mm_struct mm,
166	166	unsigned long addr, unsigned long sz);
167		−int huge_pmd_unshare(struct mm_struct mm, unsigned long addr, pte_t *ptep);
	167	+int huge_pmd_unshare(struct mm_struct mm, struct vm_area_struct vma,
	168	+ unsigned long addr, pte_t ptep);
168	169	void adjust_range_if_pmd_sharing_possible(struct vm_area_struct *vma,
169	170	unsigned long start, unsigned long end);
170	171	struct page follow_huge_addr(struct mm_struct mm, unsigned long address,
@@ -203,8 +204,9 @@ static inline struct address_space *hugetlb_page_mapping_lock_write( · 32 unmodified lines
203	204	return NULL;
204	205	}
205	206
206		−static inline int huge_pmd_unshare(struct mm_struct mm, unsigned long addr,
207		− pte_t *ptep)
	207	+static inline int huge_pmd_unshare(struct mm_struct *mm,
	208	+ struct vm_area_struct *vma,
	209	+ unsigned long addr, pte_t ptep)
208	210	{
209	211	return 0;
210	212	}

mm/hugetlb.c+7−8

@@ -3967,7 +3967,7 @@ void __unmap_hugepage_range(struct mmu_gather tlb, struct vm_area_struct vma, · 3966 unmodified lines
3967	3967	continue;
3968	3968
3969	3969	ptl = huge_pte_lock(h, mm, ptep);
3970		− if (huge_pmd_unshare(mm, &address, ptep)) {
	3970	+ if (huge_pmd_unshare(mm, vma, &address, ptep)) {
3971	3971	spin_unlock(ptl);
3972	3972	/*
3973	3973	* We just unmapped a page of PMDs by clearing a PUD.
@@ -4554,10 +4554,6 @@ vm_fault_t hugetlb_fault(struct mm_struct mm, struct vm_area_struct vma, · 580 unmodified lines
4554	4554	} else if (unlikely(is_hugetlb_entry_hwpoisoned(entry)))
4555	4555	return VM_FAULT_HWPOISON_LARGE \|
4556	4556	VM_FAULT_SET_HINDEX(hstate_index(h));
4557		− } else {
4558		− ptep = huge_pte_alloc(mm, haddr, huge_page_size(h));
4559		− if (!ptep)
4560		− return VM_FAULT_OOM;
4561	4557	}
4562	4558
4563	4559	/*
@@ -5034,7 +5030,7 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma, · 470 unmodified lines
5034	5030	if (!ptep)
5035	5031	continue;
5036	5032	ptl = huge_pte_lock(h, mm, ptep);
5037		− if (huge_pmd_unshare(mm, &address, ptep)) {
	5033	+ if (huge_pmd_unshare(mm, vma, &address, ptep)) {
5038	5034	pages++;
5039	5035	spin_unlock(ptl);
5040	5036	shared_pmd = true;
@@ -5415,12 +5411,14 @@ out: · 374 unmodified lines
5415	5411	* returns: 1 successfully unmapped a shared pte page
5416	5412	* 0 the underlying pte page is not shared, or it is the last user
5417	5413	*/
5418		−int huge_pmd_unshare(struct mm_struct mm, unsigned long addr, pte_t *ptep)
	5414	+int huge_pmd_unshare(struct mm_struct mm, struct vm_area_struct vma,
	5415	+ unsigned long addr, pte_t ptep)
5419	5416	{
5420	5417	pgd_t pgd = pgd_offset(mm, addr);
5421	5418	p4d_t p4d = p4d_offset(pgd, addr);
5422	5419	pud_t pud = pud_offset(p4d, addr);
5423	5420
	5421	+ i_mmap_assert_write_locked(vma->vm_file->f_mapping);
5424	5422	BUG_ON(page_count(virt_to_page(ptep)) == 0);
5425	5423	if (page_count(virt_to_page(ptep)) == 1)
5426	5424	return 0;
@@ -5438,7 +5436,8 @@ pte_t huge_pmd_share(struct mm_struct mm, unsigned long addr, pud_t *pud) · 11 unmodified lines
5438	5436	return NULL;
5439	5437	}
5440	5438
5441		−int huge_pmd_unshare(struct mm_struct mm, unsigned long addr, pte_t *ptep)
	5439	+int huge_pmd_unshare(struct mm_struct mm, struct vm_area_struct vma,
	5440	+ unsigned long addr, pte_t ptep)
5442	5441	{
5443	5442	return 0;
5444	5443	}

mm/rmap.c+1−1

@@ -1469,7 +1469,7 @@ static bool try_to_unmap_one(struct page page, struct vm_area_struct vma, · 1468 unmodified lines
1469	1469	* do this outside rmap routines.
1470	1470	*/
1471	1471	VM_BUG_ON(!(flags & TTU_RMAP_LOCKED));
1472		− if (huge_pmd_unshare(mm, &address, pvmw.pte)) {
	1472	+ if (huge_pmd_unshare(mm, vma, &address, pvmw.pte)) {
1473	1473	/*
1474	1474	* huge_pmd_unshare unmapped an entire PMD
1475	1475	* page. There is no way of knowing exactly