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

[Hugh Dickins]

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@xxxxxxxxxxxxxxxxxxxx: use maybe_mkwrite]
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Mel Gorman <mgorman@xxxxxxx>
> Acked-by: Rik van Riel <riel@xxxxxxxxxx>

Sort-of-Acked-by: Hugh Dickins <hughd@xxxxxxxxxx>

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/