linux-kernel - Re: [PATCH 1/2] mm: migrate: Close race between migration completion and mprotect

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: <alpine.LSU.2.11.1410021211500.7465@eggly.anvils>
Date:	Thu, 2 Oct 2014 12:20:05 -0700 (PDT)
From:	Hugh Dickins <hughd@...gle.com>
To:	Mel Gorman <mgorman@...e.de>
cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Dave Jones <davej@...hat.com>, Hugh Dickins <hughd@...gle.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	Rik van Riel <riel@...hat.com>,
	Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Aneesh Kumar <aneesh.kumar@...ux.vnet.ibm.com>,
	Michel Lespinasse <walken@...gle.com>,
	Kirill A Shutemov <kirill.shutemov@...ux.intel.com>,
	Sasha Levin <sasha.levin@...cle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] mm: migrate: Close race between migration completion
 and mprotect

On Thu, 2 Oct 2014, Mel Gorman wrote:

> A migration entry is marked as write if pte_write was true at the time the
> entry was created. The VMA protections are not double checked when migration
> entries are being removed as mprotect marks write-migration-entries as
> read. It means that potentially we take a spurious fault to mark PTEs write
> again but it's straight-forward. However, there is a race between write
> migrations being marked read and migrations finishing. This potentially
> allows a PTE to be write that should have been read. Close this race by
> double checking the VMA permissions using maybe_mkwrite when migration
> completes.
> 
> [torvalds@...ux-foundation.org: use maybe_mkwrite]
> Cc: stable@...r.kernel.org
> Signed-off-by: Mel Gorman <mgorman@...e.de>
> Acked-by: Rik van Riel <riel@...hat.com>

Sort-of-Acked-by: Hugh Dickins <hughd@...gle.com>

Safe patch, but I stand by the conclusion we both arrived at when
looking at this case a few weeks ago: it eliminates the surprise
we got in seeing a PROTNONE pte with RW set, but does not appear
to fix any actual bug.  As mprotect proceeds, it's inevitable that
some of the ptes will have more or less permissions than mprotect
is imposing, and it doesn't matter, because mprotect has mmap_sem
exclusively, so faults on the area can only complete either before
or after the whole mprotect operation.

> ---
>  mm/migrate.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index f78ec9b..0c07339 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -146,8 +146,11 @@ static int remove_migration_pte(struct page *new, struct vm_area_struct *vma,
>  	pte = pte_mkold(mk_pte(new, vma->vm_page_prot));
>  	if (pte_swp_soft_dirty(*ptep))
>  		pte = pte_mksoft_dirty(pte);
> +
> +	/* Recheck VMA as permissions can change since migration started  */
>  	if (is_write_migration_entry(entry))
> -		pte = pte_mkwrite(pte);
> +		pte = maybe_mkwrite(pte, vma);
> +
>  #ifdef CONFIG_HUGETLB_PAGE
>  	if (PageHuge(new)) {
>  		pte = pte_mkhuge(pte);
> -- 
> 1.8.4.5
--
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/