linux-kernel - Re: [PATCH 1/2] x86/mm/KASLR: Fix the wrong calculation of memory region initial size

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]

Date:   Mon, 8 Apr 2019 15:54:14 +0800
From:   Baoquan He <bhe@...hat.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org, tglx@...utronix.de,
        mingo@...nel.org, hpa@...or.com, kirill@...temov.name,
        keescook@...omium.org, yamada.masahiro@...ionext.com,
        dave.hansen@...ux.intel.com, luto@...nel.org, peterz@...radead.org,
        thgarnie@...gle.com, mike.travis@....com, frank.ramsay@....com
Subject: Re: [PATCH 1/2] x86/mm/KASLR: Fix the wrong calculation of memory
 region initial size

On 04/06/19 at 06:43am, Borislav Petkov wrote:
> On Sat, Apr 06, 2019 at 09:51:19AM +0800, Baoquan He wrote:
> > It's KASLR happened in kernel_randomize_memory() of arch/x86/mm/kaslr.c .
> 
> What is "KASLR happened in"? This doesn't make any sense. When you look
> at that function, there's a comment above it:
> 
> /* Initialize base and padding for each memory region randomized with KASLR */
> 
> Do you mean, that, per chance?
> 
> > In fact, I don't know how to call it. Previously, I wrote it as mm
> > KASLR, to distinguish from KASLR during kernel decompression. Ingo
> > blamed the name,
> 
> Of course he did, because it didn't make any sense to him either.
> 
> > so I changed it to memory region KASLR. Seems Thomas
> > Garnier called it KASLR for kernel memory regions in his original KASLR
> > adding patch. May I call it 'KASLR for kernel memory regions', or 'KASLR
> > for memory regions'?
> 
> So you're fixing kaslr_regions[0].size_tb. It's base gets initialized to
> page_offset_base.
> 
> Now, if you look at
> 
> Documentation/x86/x86_64/mm.txt
> 
> it says there:
> 
>  ffff888000000000 | -119.5  TB | ffffc87fffffffff |   64 TB | direct mapping of all physical memory (page_offset_base)
> 
> so that is the direct mapping memory region of all physical memory.
> 
> Now, you're apparently fixing its size.
> 
> Am I making sense and are you catching my drift?

Yes, makes sense. I will make it more specific, and use
kernel_randomize_memory() instead to indicate the place where code issue
is found out. Thanks.

> 
> You need to explain what you change in your commit messages in
> *understandable* english so that reviewer/committer or even a person
> who's not deeply involved in KASLR inner workings, can at least get an
> idea about what the commit message is talking about.
> 
> If you come up with strange constructs like "memory region KASLR" or
> "KASLR happened in" or "mm KASLR" which only make sense in your head,
> you're not doing anyone any favour.
> 
> Commit messages need to be very understandable when someone is looking
> at them months or even years from now. And you need to restrain yourself
> when you write them. You will appreciate that the first time you have to
> do git archeology, dig out an ancient commit and wonder why we did it
> this way.
> 
> Because we didn't document as good in previous years and our commits
> from the past suck big time.
> 
> Thanks!
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> Good mailing practices for 400: avoid top-posting and trim the reply.