linux-kernel - Re: [PATCH 08/10] x86, xen, mm: fix mapping_pagetable

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: <CAE9FiQXa9fqLOyqzap6o_0HA=oBRKqkcWm9bdpK9M6R5cQMU-A@mail.gmail.com>
Date:	Mon, 8 Oct 2012 23:33:54 -0700
From:	Yinghai Lu <yinghai@...nel.org>
To:	"H. Peter Anvin" <hpa@...or.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...e.hu>,
	Jacob Shin <jacob.shin@....com>, Tejun Heo <tj@...nel.org>,
	Stefano Stabellini <stefano.stabellini@...citrix.com>,
	linux-kernel@...r.kernel.org,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Jeremy Fitzhardinge <jeremy@...p.org>
Subject: Re: [PATCH 08/10] x86, xen, mm: fix mapping_pagetable_reserve logic

On Mon, Oct 8, 2012 at 11:12 PM, H. Peter Anvin <hpa@...or.com> wrote:
> On 10/09/2012 12:39 PM, Yinghai Lu wrote:
>>   */
>>  struct x86_init_mapping {
>> -     void (*pagetable_reserve)(u64 start, u64 end);
>> +     void (*make_range_readwrite)(u64 start, u64 end);
>>  };
>>
>
> Here you go from one misleading name to another.  Another classic case
> of "why hooks suck."

Run out of idea to have name for it.

>
> make_range_readwrite is particularly toxic, though, because it makes it
> sound like it something along the lines of set_memory_rw(), which it
> most distinctly is not.

it just change some page range from RO back to RW.

so how about update_range_ro_to_rw?

>
> Furthermore, the naming makes me really puzzled why it is there at all.
>  It makes me suspect, but because the patchset is so messy, it is hard
> for me to immediately prove, that you're still missing something important.
>
> However, for example:
>
>> unsigned long __initdata pgt_buf_start;
>>  unsigned long __meminitdata pgt_buf_end;
>>  unsigned long __meminitdata pgt_buf_top;
>> +unsigned long __initdata early_pgt_buf_start;
>> +unsigned long __meminitdata early_pgt_buf_end;
>> +unsigned long __meminitdata early_pgt_buf_top;
>>
>>  bool __init is_pfn_in_early_pgt_buf(unsigned long pfn)
>>  {
>> -     return pfn >= pgt_buf_start && pfn < pgt_buf_top;
>> +     return (pfn >= early_pgt_buf_start && pfn < early_pgt_buf_top) ||
>> +            (pfn >= pgt_buf_start && pfn < pgt_buf_top);
>>  }
>
> Magic variables augmented with more magic variables.  Why?  This also
> seems to assume that we still do all the kernel page tables in one
> chunk, which is exactly what we don't want to do.

for 64bit, page table will be three parts
1. initial page table from arch/x86/kernel/head_64.S
2. page table from BRK.
3. page near end of RAM.

verified from /sys/kernel/debug/kernel_page_tables

only range E820_RAM is mapped.

all initial page table for hole between [0, 1G) get cleared too.

Thanks

Yinghai
--
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/