linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Hugh Dickins <hughd@google.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@suse.cz>,
	David Rientjes <rientjes@google.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCHv3 4/5] mm: make compound_head() robust
Date: Mon, 24 Aug 2015 13:17:56 +0300	[thread overview]
Message-ID: <20150824101755.GA2370@node.dhcp.inet.fi> (raw)
In-Reply-To: <1439976106-137226-5-git-send-email-kirill.shutemov@linux.intel.com>

On Wed, Aug 19, 2015 at 12:21:45PM +0300, Kirill A. Shutemov wrote:
> Hugh has pointed that compound_head() call can be unsafe in some
> context. There's one example:
> 
> 	CPU0					CPU1
> 
> isolate_migratepages_block()
>   page_count()
>     compound_head()
>       !!PageTail() == true
> 					put_page()
> 					  tail->first_page = NULL
>       head = tail->first_page
> 					alloc_pages(__GFP_COMP)
> 					   prep_compound_page()
> 					     tail->first_page = head
> 					     __SetPageTail(p);
>       !!PageTail() == true
>     <head == NULL dereferencing>
> 
> The race is pure theoretical. I don't it's possible to trigger it in
> practice. But who knows.
> 
> We can fix the race by changing how encode PageTail() and compound_head()
> within struct page to be able to update them in one shot.
> 
> The patch introduces page->compound_head into third double word block in
> front of compound_dtor and compound_order. That means it shares storage
> space with:
> 
>  - page->lru.next;
>  - page->next;
>  - page->rcu_head.next;
>  - page->pmd_huge_pte;
> 
> That's too long list to be absolutely sure, but looks like nobody uses
> bit 0 of the word. It can be used to encode PageTail(). And if the bit
> set, rest of the word is pointer to head page.
> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Acked-by: Michal Hocko <mhocko@suse.com>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>

If DEFERRED_STRUCT_PAGE_INIT=n, combining this patchset with my page-flags
patches causes oops in SetPageReserved() called from
reserve_bootmem_region().

It happens because we haven't yet initilized the word in struct page and
PageTail() inside SetPageReserved() can give false-positive, which leads
to bogus compound_head() result.

IIUC, we initialize the word only on first allocation of the page. It can
be too late: pfn scanner can see false-positive PageTail() from not yet
allocated pages too.

Here's fixlet for patch to address the issue.

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 347724850665..d0e3fca830f8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -892,6 +892,8 @@ static void init_reserved_page(unsigned long pfn)
 #else
 static inline void init_reserved_page(unsigned long pfn)
 {
+	/* Avoid false-positive PageTail() */
+	INIT_LIST_HEAD(&pfn_to_page(pfn)->lru);
 }
 #endif /* CONFIG_DEFERRED_STRUCT_PAGE_INIT */
 
-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2015-08-24 10:18 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-19  9:21 [PATCHv3 0/5] Fix compound_head() race Kirill A. Shutemov
2015-08-19  9:21 ` [PATCHv3 1/5] mm: drop page->slab_page Kirill A. Shutemov
2015-08-24 14:59   ` Vlastimil Babka
2015-08-24 15:02   ` Vlastimil Babka
2015-08-25 17:24     ` Kirill A. Shutemov
2015-08-19  9:21 ` [PATCHv3 2/5] zsmalloc: use page->private instead of page->first_page Kirill A. Shutemov
2015-08-24 15:04   ` Vlastimil Babka
2015-08-19  9:21 ` [PATCHv3 3/5] mm: pack compound_dtor and compound_order into one word in struct page Kirill A. Shutemov
2015-08-20 23:26   ` Andrew Morton
2015-08-21  7:13     ` Michal Hocko
2015-08-21 10:40       ` Kirill A. Shutemov
2015-08-21 10:51         ` Michal Hocko
2015-08-19  9:21 ` [PATCHv3 4/5] mm: make compound_head() robust Kirill A. Shutemov
2015-08-20 23:36   ` Andrew Morton
2015-08-21 12:10     ` Kirill A. Shutemov
2015-08-21 16:11       ` Christoph Lameter
2015-08-21 19:31         ` Kirill A. Shutemov
2015-08-21 19:34           ` Andrew Morton
2015-08-21 21:15             ` Christoph Lameter
2015-08-24 15:49             ` Vlastimil Babka
2015-08-25 11:44       ` Vlastimil Babka
2015-08-25 18:33         ` Kirill A. Shutemov
2015-08-25 20:11           ` Paul E. McKenney
2015-08-25 20:46             ` Vlastimil Babka
2015-08-25 21:19               ` Paul E. McKenney
2015-08-26 15:04                 ` Kirill A. Shutemov
2015-08-26 15:39                   ` Vlastimil Babka
2015-08-26 16:38                   ` Paul E. McKenney
2015-08-26 18:18                 ` Hugh Dickins
2015-08-26 21:29                   ` Paul E. McKenney
2015-08-26 22:28                     ` Hugh Dickins
2015-08-26 23:34                       ` Paul E. McKenney
2015-08-27 15:09                     ` Michal Hocko
2015-08-27 16:03                       ` Michal Hocko
2015-08-27 17:28                         ` Hugh Dickins
2015-08-27 18:06                           ` Michal Hocko
2015-08-27 16:36                       ` Paul E. McKenney
2015-08-27 18:14                         ` Michal Hocko
2015-08-27 19:01                           ` Paul E. McKenney
2015-08-23 23:59   ` Jesper Dangaard Brouer
2015-08-24  9:29     ` Kirill A. Shutemov
2015-08-24 10:17   ` Kirill A. Shutemov [this message]
2015-08-19  9:21 ` [PATCHv3 5/5] mm: use 'unsigned int' for page order Kirill A. Shutemov
2015-08-20  8:32   ` Michal Hocko
2015-08-20 12:31 ` [PATCHv3 0/5] Fix compound_head() race Kirill A. Shutemov
2015-08-20 23:38   ` Andrew Morton
2015-08-22 20:13     ` Hugh Dickins
2015-08-24  9:36       ` Kirill A. Shutemov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150824101755.GA2370@node.dhcp.inet.fi \
    --to=kirill@shutemov.name \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=dave.hansen@intel.com \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=rientjes@google.com \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).