Commit 78ecca66 authored by Naoya Horiguchi's avatar Naoya Horiguchi Committed by Luis Henriques

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

commit 0f792cf9 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 avatarLuis Henriques <luis.henriques@canonical.com>
parent 7b0f6722
...@@ -3142,6 +3142,7 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, ...@@ -3142,6 +3142,7 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
struct page *pagecache_page = NULL; struct page *pagecache_page = NULL;
struct hstate *h = hstate_vma(vma); struct hstate *h = hstate_vma(vma);
struct address_space *mapping; struct address_space *mapping;
int need_wait_lock = 0;
address &= huge_page_mask(h); address &= huge_page_mask(h);
...@@ -3179,6 +3180,16 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, ...@@ -3179,6 +3180,16 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
ret = 0; 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 * If we are going to COW the mapping later, we examine the pending
* reservations for this page now. This will ensure that any * reservations for this page now. This will ensure that any
...@@ -3198,30 +3209,31 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, ...@@ -3198,30 +3209,31 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
vma, address); 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 * hugetlb_cow() requires page locks of pte_page(entry) and
* pagecache_page, so here we need take the former one * pagecache_page, so here we need take the former one
* when page != pagecache_page or !pagecache_page. * 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); page = pte_page(entry);
get_page(page);
if (page != pagecache_page) if (page != pagecache_page)
lock_page(page); if (!trylock_page(page)) {
need_wait_lock = 1;
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))))
goto out_ptl; goto out_ptl;
}
get_page(page);
if (flags & FAULT_FLAG_WRITE) { if (flags & FAULT_FLAG_WRITE) {
if (!huge_pte_write(entry)) { if (!huge_pte_write(entry)) {
ret = hugetlb_cow(mm, vma, address, ptep, entry, ret = hugetlb_cow(mm, vma, address, ptep, entry,
pagecache_page, ptl); pagecache_page, ptl);
goto out_ptl; goto out_put_page;
} }
entry = huge_pte_mkdirty(entry); entry = huge_pte_mkdirty(entry);
} }
...@@ -3229,7 +3241,10 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, ...@@ -3229,7 +3241,10 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
if (huge_ptep_set_access_flags(vma, address, ptep, entry, if (huge_ptep_set_access_flags(vma, address, ptep, entry,
flags & FAULT_FLAG_WRITE)) flags & FAULT_FLAG_WRITE))
update_mmu_cache(vma, address, ptep); update_mmu_cache(vma, address, ptep);
out_put_page:
if (page != pagecache_page)
unlock_page(page);
put_page(page);
out_ptl: out_ptl:
spin_unlock(ptl); spin_unlock(ptl);
...@@ -3237,12 +3252,17 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, ...@@ -3237,12 +3252,17 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
unlock_page(pagecache_page); unlock_page(pagecache_page);
put_page(pagecache_page); put_page(pagecache_page);
} }
if (page != pagecache_page)
unlock_page(page);
put_page(page);
out_mutex: out_mutex:
mutex_unlock(&htlb_fault_mutex_table[hash]); 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; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment