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

[Baoquan He]

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.