linux-kernel - Re: [PATCH 4/4] anonvma: when setting up page->mapping, we need to pick the _oldest

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 13 Apr 2010 02:41:17 +0200
From:	Johannes Weiner <hannes@...xchg.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Borislav Petkov <bp@...en8.de>, Rik van Riel <riel@...hat.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Minchan Kim <minchan.kim@...il.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Lee Schermerhorn <Lee.Schermerhorn@...com>,
	Nick Piggin <npiggin@...e.de>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Hugh Dickins <hugh.dickins@...cali.co.uk>,
	sgunderson@...foot.com
Subject: Re: [PATCH 4/4] anonvma: when setting up page->mapping, we need to pick the _oldest_ anonvma

On Mon, Apr 12, 2010 at 01:23:50PM -0700, Linus Torvalds wrote:
> 
> From: Linus Torvalds <torvalds@...ux-foundation.org>
> Date: Mon, 12 Apr 2010 12:44:29 -0700
> Subject: [PATCH 4/4] anonvma: when setting up page->mapping, we need to pick the _oldest_ anonvma
> 
> Otherwise we might be mapping in a page in a new mapping, but that page
> (through the swapcache) would later be mapped into an old mapping too.
> The page->mapping must be the case that works for everybody, not just
> the mapping that happened to page it in first.
> 
> This can be improved in certain cases: if we know the page is private to
> just this particular mapping (for example, it's a new page, or it is the
> only swapcache entry), we could pick the top (most specific) anon_vma.
> 
> But that's a future optimization. Make it _work_ reliably first.
> 
> Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>

Acked-by: Johannes Weiner <hannes@...xchg.org>

Would you mind pasting that nice description of the error case from your
other email into that changelog?  I skimmed over the description but when
I read this patch several hours later, I had to go back to that previous
email to fully make sense of it.

> ---
>  mm/rmap.c |   15 +++++++++++++--
>  1 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/rmap.c b/mm/rmap.c
> index ee97d38..4bad326 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -734,9 +734,20 @@ void page_move_anon_rmap(struct page *page,
>  static void __page_set_anon_rmap(struct page *page,
>  	struct vm_area_struct *vma, unsigned long address)
>  {
> -	struct anon_vma *anon_vma = vma->anon_vma;
> +	struct anon_vma_chain *avc;
> +	struct anon_vma *anon_vma;
> +
> +	BUG_ON(!vma->anon_vma);
> +
> +	/*
> +	 * We must use the _oldest_ possible anon_vma for the page mapping!

I think the key here is not that it's the oldest (past) but also the one with
the longest extent (future), so that it's bound to stay until the last possible
mapping for this page vanishes.

Maybe it's just me, but I doubt the comment as it is would help me understand
that code if I didn't already.

> +	 *
> +	 * So take the last AVC chain entry in the vma, which is the deepest
> +	 * ancestor, and use the anon_vma from that.
> +	 */
> +	avc = list_entry(vma->anon_vma_chain.prev, struct anon_vma_chain, same_vma);
> +	anon_vma = avc->anon_vma;
>  
> -	BUG_ON(!anon_vma);
>  	anon_vma = (void *) anon_vma + PAGE_MAPPING_ANON;
>  	page->mapping = (struct address_space *) anon_vma;
>  	page->index = linear_page_index(vma, address);

Hannes
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/