Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 674eefde authored by Naoya Horiguchi's avatar Naoya Horiguchi Committed by Sasha Levin
Browse files

mm/hugetlb: fix getting refcount 0 page in hugetlb_fault() 



commit 0f792cf949a0be506c2aa8bfac0605746b146dda upstream.

When running the test which causes the race as shown in the previous patch,
we can hit the BUG "get_page() on refcount 0 page" in hugetlb_fault().

This race happens when pte turns into migration entry just after the first
check of is_hugetlb_entry_migration() in hugetlb_fault() passed with false.
To fix this, we need to check pte_present() again after huge_ptep_get().

This patch also reorders taking ptl and doing pte_page(), because
pte_page() should be done in ptl.  Due to this reordering, we need use
trylock_page() in page != pagecache_page case to respect locking order.

Fixes: 66aebce7 ("hugetlb: fix race condition in hugetlb_fault()")
Signed-off-by: default avatarNaoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: James Hogan <james.hogan@imgtec.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Mel Gorman <mel@csn.ul.ie>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Rik van Riel <riel@redhat.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Luiz Capitulino <lcapitulino@redhat.com>
Cc: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
Cc: Lee Schermerhorn <lee.schermerhorn@hp.com>
Cc: Steve Capper <steve.capper@linaro.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: default avatarSasha Levin <sasha.levin@oracle.com>
parent c3b2b0df

mm/hugetlb.c

+36 −16
Original line number Diff line number Diff line
@@ -3128,6 +3128,7 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, 
	struct page *pagecache_page = NULL;

	struct hstate *h = hstate_vma(vma);

	struct address_space *mapping;

	int need_wait_lock = 0;



	address &= huge_page_mask(h);
@@ -3165,6 +3166,16 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, 


	ret = 0;



	/*

	 * entry could be a migration/hwpoison entry at this point, so this

	 * check prevents the kernel from going below assuming that we have

	 * a active hugepage in pagecache. This goto expects the 2nd page fault,

	 * and is_hugetlb_entry_(migration|hwpoisoned) check will properly

	 * handle it.

	 */

	if (!pte_present(entry))

		goto out_mutex;



	/*

	 * If we are going to COW the mapping later, we examine the pending

	 * reservations for this page now. This will ensure that any
@@ -3184,30 +3195,31 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, 
								vma, address);

	}



	ptl = huge_pte_lock(h, mm, ptep);



	/* Check for a racing update before calling hugetlb_cow */

	if (unlikely(!pte_same(entry, huge_ptep_get(ptep))))

		goto out_ptl;



	/*

	 * hugetlb_cow() requires page locks of pte_page(entry) and

	 * pagecache_page, so here we need take the former one

	 * when page != pagecache_page or !pagecache_page.

	 * Note that locking order is always pagecache_page -> page,

	 * so no worry about deadlock.

	 */

	page = pte_page(entry);

	get_page(page);

	if (page != pagecache_page)

		lock_page(page);



	ptl = huge_pte_lockptr(h, mm, ptep);

	spin_lock(ptl);

	/* Check for a racing update before calling hugetlb_cow */

	if (unlikely(!pte_same(entry, huge_ptep_get(ptep))))

		if (!trylock_page(page)) {

			need_wait_lock = 1;

			goto out_ptl;

		}



	get_page(page);



	if (flags & FAULT_FLAG_WRITE) {

		if (!huge_pte_write(entry)) {

			ret = hugetlb_cow(mm, vma, address, ptep, entry,

					pagecache_page, ptl);

			goto out_ptl;

			goto out_put_page;

		}

		entry = huge_pte_mkdirty(entry);

	}
@@ -3215,7 +3227,10 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, 
	if (huge_ptep_set_access_flags(vma, address, ptep, entry,

						flags & FAULT_FLAG_WRITE))

		update_mmu_cache(vma, address, ptep);



out_put_page:

	if (page != pagecache_page)

		unlock_page(page);

	put_page(page);

out_ptl:

	spin_unlock(ptl);
@@ -3223,12 +3238,17 @@ out_ptl: 
		unlock_page(pagecache_page);

		put_page(pagecache_page);

	}

	if (page != pagecache_page)

		unlock_page(page);

	put_page(page);



out_mutex:

	mutex_unlock(&htlb_fault_mutex_table[hash]);

	/*

	 * Generally it's safe to hold refcount during waiting page lock. But

	 * here we just wait to defer the next page fault to avoid busy loop and

	 * the page is not used after unlocked before returning from the current

	 * page fault. So we are safe from accessing freed page, even if we wait

	 * here without taking refcount.

	 */

	if (need_wait_lock)

		wait_on_page_locked(page);

	return ret;

}