linux-kernel - Re: [PATCH RESEND -mm 2/2] mm/mempolicy.c: add comment in queue_pages

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: <5329C4CC.2000200@oracle.com>
Date:	Wed, 19 Mar 2014 12:24:44 -0400
From:	Sasha Levin <sasha.levin@...cle.com>
To:	Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
	Andrew Morton <akpm@...ux-foundation.org>
CC:	Rik van Riel <riel@...hat.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND -mm 2/2] mm/mempolicy.c: add comment in queue_pages_hugetlb()

On 03/18/2014 10:29 PM, Naoya Horiguchi wrote:
> We have a race where we try to migrate an invalid page, resulting in
> hitting VM_BUG_ON_PAGE in isolate_huge_page().
> queue_pages_hugetlb() is OK to fail, so let's check !PageHeadHuge to keep
> invalid hugepage from queuing.
>
> Reported-by: Sasha Levin <sasha.levin@...cle.com>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@...jp.nec.com>
> ---
>   mm/mempolicy.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git v3.14-rc7-mmotm-2014-03-18-16-37.orig/mm/mempolicy.c v3.14-rc7-mmotm-2014-03-18-16-37/mm/mempolicy.c
> index 9d2ef4111a4c..ae6e2d9dc855 100644
> --- v3.14-rc7-mmotm-2014-03-18-16-37.orig/mm/mempolicy.c
> +++ v3.14-rc7-mmotm-2014-03-18-16-37/mm/mempolicy.c
> @@ -530,6 +530,17 @@ static int queue_pages_hugetlb(pte_t *pte, unsigned long addr,
>   	if (!pte_present(entry))
>   		return 0;
>   	page = pte_page(entry);
> +
> +	/*
> +	 * Trinity found that page could be a non-hugepage. This is an
> +	 * unexpected behavior, but it's not clear how this problem happens.
> +	 * So let's simply skip such corner case. Page migration can often
> +	 * fail for various reasons, so it's ok to just skip the address
> +	 * unsuitable to hugepage migration.
> +	 */
> +	if (!PageHeadHuge(page))
> +		return 0;
> +
>   	nid = page_to_nid(page);
>   	if (node_isset(nid, *qp->nmask) == !!(flags & MPOL_MF_INVERT))
>   		return 0;
>

I have to say that I really dislike this method of solving the issue.

I think it's something fine to do for testing, but this will just hide this issue
and will let it sneak upstream. I'm really not sure if the trace I've reported is
the only codepath that would trigger it, so if we let it sneak upstream we're risking
of someone hitting it some other way.


Thanks,
Sasha
--
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/