Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932443AbbEEBtr (ORCPT ); Mon, 4 May 2015 21:49:47 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:51830 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932350AbbEEBsh (ORCPT ); Mon, 4 May 2015 21:48:37 -0400 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 From: Ben Hutchings To: linux-kernel@vger.kernel.org, stable@vger.kernel.org CC: akpm@linux-foundation.org, "Lee Schermerhorn" , "David Rientjes" , "Mel Gorman" , "Rik van Riel" , "James Hogan" , "Naoya Horiguchi" , "Andrea Arcangeli" , "Steve Capper" , "Nishanth Aravamudan" , "Hugh Dickins" , "Johannes Weiner" , "Luiz Capitulino" , "Michal Hocko" , "Linus Torvalds" Date: Tue, 05 May 2015 02:16:39 +0100 Message-ID: X-Mailer: LinuxStableQueue (scripts by bwh) Subject: [PATCH 3.2 034/221] mm/hugetlb: fix getting refcount 0 page in hugetlb_fault() In-Reply-To: X-SA-Exim-Connect-IP: 192.168.4.249 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5186 Lines: 152 3.2.69-rc1 review patch. If anyone has any objections, please let me know. ------------------ From: Naoya Horiguchi 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: 66aebce747ea ("hugetlb: fix race condition in hugetlb_fault()") Signed-off-by: Naoya Horiguchi Cc: Hugh Dickins Cc: James Hogan Cc: David Rientjes Cc: Mel Gorman Cc: Johannes Weiner Cc: Michal Hocko Cc: Rik van Riel Cc: Andrea Arcangeli Cc: Luiz Capitulino Cc: Nishanth Aravamudan Cc: Lee Schermerhorn Cc: Steve Capper Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds [bwh: Backported to 3.2: - Adjust context - Error label is named 'out_page_table_lock' not 'out_ptl'] Signed-off-by: Ben Hutchings --- mm/hugetlb.c | 52 ++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 16 deletions(-) --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -2798,6 +2798,7 @@ int hugetlb_fault(struct mm_struct *mm, struct page *pagecache_page = NULL; static DEFINE_MUTEX(hugetlb_instantiation_mutex); struct hstate *h = hstate_vma(vma); + int need_wait_lock = 0; ptep = huge_pte_offset(mm, address); if (ptep) { @@ -2829,6 +2830,16 @@ int hugetlb_fault(struct mm_struct *mm, 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 * allocations necessary to record that reservation occur outside the @@ -2847,29 +2858,30 @@ int hugetlb_fault(struct mm_struct *mm, vma, address); } + spin_lock(&mm->page_table_lock); + /* Check for a racing update before calling hugetlb_cow */ + if (unlikely(!pte_same(entry, huge_ptep_get(ptep)))) + goto out_page_table_lock; + /* * 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); - - spin_lock(&mm->page_table_lock); - /* Check for a racing update before calling hugetlb_cow */ - if (unlikely(!pte_same(entry, huge_ptep_get(ptep)))) - goto out_page_table_lock; + if (!trylock_page(page)) { + need_wait_lock = 1; + goto out_page_table_lock; + } + get_page(page); if (flags & FAULT_FLAG_WRITE) { if (!pte_write(entry)) { ret = hugetlb_cow(mm, vma, address, ptep, entry, pagecache_page); - goto out_page_table_lock; + goto out_put_page; } entry = pte_mkdirty(entry); } @@ -2877,7 +2889,10 @@ int hugetlb_fault(struct mm_struct *mm, 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_page_table_lock: spin_unlock(&mm->page_table_lock); @@ -2885,13 +2900,18 @@ out_page_table_lock: unlock_page(pagecache_page); put_page(pagecache_page); } - if (page != pagecache_page) - unlock_page(page); - put_page(page); - out_mutex: mutex_unlock(&hugetlb_instantiation_mutex); + /* + * 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; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/