linux-kernel - Re: [rfc patch 3/3] mm: munlock COW pages on truncation unmap

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]

Message-Id: <20091006100724.5F97.A69D9226@jp.fujitsu.com>
Date:	Tue,  6 Oct 2009 10:11:13 +0900 (JST)
From:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
To:	Johannes Weiner <hannes@...xchg.org>
Cc:	kosaki.motohiro@...fujitsu.com, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org,
	Hugh Dickins <hugh.dickins@...cali.co.uk>,
	Mel Gorman <mel@....ul.ie>,
	Lee Schermerhorn <Lee.Schermerhorn@...com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [rfc patch 3/3] mm: munlock COW pages on truncation unmap

> ---
> From: Johannes Weiner <hannes@...xchg.org>
> Subject: mm: order evictable rescue in LRU putback
> 
> Isolators putting a page back to the LRU do not hold the page lock,
> and if the page is mlocked, another thread might munlock it
> concurrently.
> 
> Expecting this, the putback code re-checks the evictability of a page
> when it just moved it to the unevictable list in order to correct its
> decision.
> 
> The problem, however, is that ordering is not garuanteed between
> setting PG_lru when moving the page to the list and checking
> PG_mlocked afterwards:
> 
> 	#0 putback			#1 munlock
> 
> 	spin_lock()
> 					if (TestClearPageMlocked())
> 					  if (PageLRU())
> 					    move to evictable list
> 	SetPageLRU()
> 	spin_unlock()
> 	if (!PageMlocked())
> 	  move to evictable list
> 
> The PageMlocked() reading may get reordered before SetPageLRU() in #0,
> resulting in #0 not moving the still mlocked page, and in #1 failing
> to isolate and move the page as well.  The evictable page is now
> stranded on the unevictable list.
> 
> TestClearPageMlocked() in #1 already provides full memory barrier
> semantics.
> 
> This patch adds an explicit full barrier to force ordering between
> SetPageLRU() and PageMlocked() in #0 so that either one of the
> competitors rescues the page.
> 
> Signed-off-by: Johannes Weiner <hannes@...xchg.org>
> Cc: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
> Cc: Hugh Dickins <hugh.dickins@...cali.co.uk>
> Cc: Mel Gorman <mel@....ul.ie>
> Cc: Lee Schermerhorn <Lee.Schermerhorn@...com>
> Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> ---
>  mm/vmscan.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -544,6 +544,16 @@ redo:
>  		 */
>  		lru = LRU_UNEVICTABLE;
>  		add_page_to_unevictable_list(page);
> +		/*
> +		 * When racing with an mlock clearing (page is
> +		 * unlocked), make sure that if the other thread does
> +		 * not observe our setting of PG_lru and fails
> +		 * isolation, we see PG_mlocked cleared below and move
> +		 * the page back to the evictable list.
> +		 *
> +		 * The other side is TestClearPageMlocked().
> +		 */
> +		smp_mb();
>  	}

IA64 is most relax cpu reorder architecture. I'm usually test on it
and my test found no problem.
Then, I don't think this issue occur in the real world. but I think
this patch is right.

Hannes, you are great.

	Reviewed-by: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>





--
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/