linux-kernel - Re: [PATCH v2] x86/mm: fix cpu stuck issue in __change_page_attr_set

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] [day] [month] [year] [list]

Message-ID: <451f54b6-e5f5-cf30-f73f-c155c293cbe7@intel.com>
Date:   Thu, 5 Jul 2018 15:20:54 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     Bin Yang <bin.yang@...el.com>, tglx@...utronix.de,
        mingo@...hat.com, hpa@...or.com, x86@...nel.org,
        linux-kernel@...r.kernel.org, "Gross, Mark" <mark.gross@...el.com>
Subject: Re: [PATCH v2] x86/mm: fix cpu stuck issue in
 __change_page_attr_set_clr

On 07/04/2018 10:47 PM, Bin Yang wrote:
> When changing a 4K page attr inside 1G/2M large page range,
> __change_page_attr() will call try_preserve_large_page() to decide
> to split the big page or not. And try_preserve_large_page() will
> call static_protections() to check all 4K pages inside the large page
> range one by one. The check loop is very inefficient. In worst case,
> static_protections() will be called for 1G/4K (262144) times.

I wrote this before I read the entire thread and Thomas's analysis, but
I think we came to the same conclusion.  Here goes anyway:

Let me make sure I got this right.

1. free_init_pages() frees a single 4k page
2. set_memory_rw() tries to set that 4k range read-write
3. try_preserve_large_page()->_lookup_address_cpa() finds a 1GB
   page covering that 4k range.
4. We now go over the 1GB range (outside the 4k one we may or may not be
   changing) to see if it needs to be converted because of
   static_protections()

Correct?

#4 seems insane.  Why is it the new call's job to fix up protections
outside the 4k area which it was called to change?

There's an argument to be made that we need the static_protections()
loop for the area that try_to_preserve_large_page() is being called
over, but by no means do we need that for anything larger.

So, for this patch's purposes, if we do a change_page_attr(), walk down
to the PTE/PMD/PUD/... mapping that virtual address, and find that that
pte fulfills the 'prot' we are going for (adjusted for
static_protections() and for large page prot bits), we are done.
Totally done.  Done-aroo.