linux-kernel - Re: [PATCH 03/19] x86, mm: Don't clear page table if range is ram

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: <CAE9FiQXc7SPmTyX_jkJteO+3UU5+ifDviXNK+DD_1ryH0EsQOQ@mail.gmail.com>
Date:	Mon, 22 Oct 2012 09:56:01 -0700
From:	Yinghai Lu <yinghai@...nel.org>
To:	Konrad Rzeszutek Wilk <konrad@...nel.org>
Cc:	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...e.hu>,
	"H. Peter Anvin" <hpa@...or.com>, Jacob Shin <jacob.shin@....com>,
	Tejun Heo <tj@...nel.org>,
	Stefano Stabellini <stefano.stabellini@...citrix.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 03/19] x86, mm: Don't clear page table if range is ram

On Mon, Oct 22, 2012 at 7:28 AM, Konrad Rzeszutek Wilk
<konrad@...nel.org> wrote:
> On Thu, Oct 18, 2012 at 01:50:14PM -0700, Yinghai Lu wrote:
>> After we add code use buffer in BRK to pre-map page table,
>                    ^- to
>
> So .. which patch is that? Can you include the title of the
> patch here?
>
>> it should be safe to remove early_memmap for page table accessing.
>> Instead we get panic with that.
>>
>> It turns out we clear the initial page table wrongly for next range that is
>               ^- that
>
>> separated by holes.
>> And it only happens when we are trying to map range one by one range separately.
>                                                      ^-s
>
>>
>> We need to check if the range is ram before clearing page table.
>
> Ok, so that sounds like a bug-fix... but
>>
>> Signed-off-by: Yinghai Lu <yinghai@...nel.org>
>> ---
>>  arch/x86/mm/init_64.c |   37 ++++++++++++++++---------------------
>>  1 files changed, 16 insertions(+), 21 deletions(-)
>>
>> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
>> index f40f383..61b3c44 100644
>> --- a/arch/x86/mm/init_64.c
>> +++ b/arch/x86/mm/init_64.c
>> @@ -363,20 +363,19 @@ static unsigned long __meminit
>>  phys_pte_init(pte_t *pte_page, unsigned long addr, unsigned long end,
>>             pgprot_t prot)
>>  {
>> -     unsigned pages = 0;
>> +     unsigned long pages = 0, next;
>>       unsigned long last_map_addr = end;
>>       int i;
>>
>>       pte_t *pte = pte_page + pte_index(addr);
>>
>> -     for(i = pte_index(addr); i < PTRS_PER_PTE; i++, addr += PAGE_SIZE, pte++) {
>> -
>> +     for (i = pte_index(addr); i < PTRS_PER_PTE; i++, addr = next, pte++) {
>> +             next = (addr & PAGE_MASK) + PAGE_SIZE;
>>               if (addr >= end) {
>> -                     if (!after_bootmem) {
>> -                             for(; i < PTRS_PER_PTE; i++, pte++)
>> -                                     set_pte(pte, __pte(0));
>> -                     }
>> -                     break;
>> +                     if (!after_bootmem &&
>> +                         !e820_any_mapped(addr & PAGE_MASK, next, 0))
>> +                             set_pte(pte, __pte(0));
>> +                     continue;
>
> .. Interestingly, you also removed the extra loop. How come? Why not
> retain the little loop? (which could call e820_any_mapped?) Is that
> an improvement and cleanup? If so, I would think you should at least
> explain in the git commit:

Merge that loop to top loop, and we need to use "next" from the top loop.

>
> "And while we are at it, also axe the extra loop and instead depend on
> the top loop which we can safely piggyback on."


update commit change log to:

---
After we add code use buffer in BRK to pre-map buf for page table in
following patch:
        x86, mm: setup page table in top-down
it should be safe to remove early_memmap for page table accessing.
Instead we get panic with that.

It turns out that we clear the initial page table wrongly for next range
that is separated by holes.
And it only happens when we are trying to map ram range one by one.

We need to check if the range is ram before clearing page table.

We change the loop structure to remove the extra little loop and use
one loop only, and in that loop will caculate next at first, and check if
[addr,next) is covered by E820_RAM.
---
--
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/