linux-kernel - Re: [PATCH] memory-hotplug: Fix bad area access on dissolve_free_huge

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 Sep 2016 10:32:17 -0700
From:   Dave Hansen <dave.hansen@...ux.intel.com>
To:     Rui Teng <rui.teng@...ux.vnet.ibm.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
        Michal Hocko <mhocko@...e.com>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        "Aneesh Kumar K . V" <aneesh.kumar@...ux.vnet.ibm.com>,
        Paul Gortmaker <paul.gortmaker@...driver.com>,
        Santhosh G <santhog4@...ibm.com>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Subject: Re: [PATCH] memory-hotplug: Fix bad area access on
 dissolve_free_huge_pages()

On 09/13/2016 01:39 AM, Rui Teng wrote:
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 87e11d8..64b5f81 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1442,7 +1442,7 @@ static int free_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed,
>  static void dissolve_free_huge_page(struct page *page)
>  {
>  	spin_lock(&hugetlb_lock);
> -	if (PageHuge(page) && !page_count(page)) {
> +	if (PageHuge(page) && !page_count(page) && PageHead(page)) {
>  		struct hstate *h = page_hstate(page);
>  		int nid = page_to_nid(page);
>  		list_del(&page->lru);

This is goofy.  What is calling dissolve_free_huge_page() on a tail page?

Hmm:

>         for (pfn = start_pfn; pfn < end_pfn; pfn += 1 << minimum_order)
>                 dissolve_free_huge_page(pfn_to_page(pfn));

So, skip through the area being offlined at the smallest huge page size,
and try to dissolve a huge page in each place one might appear.  But,
after we dissolve a 16GB huge page, we continue looking through the
remaining 15.98GB tail area for huge pages in the area we just
dissolved.  The tail pages are still PageHuge() (how??), and we call
page_hstate() on the tail page whose head was just dissolved.

Note, even with the fix, this taking a (global) spinlock 1023 more times
that it doesn't have to.

This seems inefficient, and fails to fully explain what is going on, and
how tail pages still _look_ like PageHuge(), which seems pretty wrong.

I guess the patch _works_.  But, sheesh, it leaves a lot of room for
improvement.