Re: [PATCH 0/3] x86/boot/KASLR: enhance randomness of kernel load addr when using GiB hugepage

[Pingfan Liu]

On Mon, Sep 10, 2018 at 7:33 AM Baoquan He <bhe@xxxxxxxxxx> wrote:
>
> Hi Pingfan,
>
> On 09/06/18 at 10:36am, Pingfan Liu wrote:
> > commit 747ff6265db4 ("x86/boot/KASLR: Skip specified number of 1GB huge
> > pages when doing physical randomization (KASLR)") and commit
> > 9b912485e0e7 ("x86/boot/KASLR: Add two new functions for 1GB huge pages
> > handling") prevent the physical load addr of kernel from spoiling a good
> > candidate of GiB page. But the algorithm deterministicly chooses the most
> > front GiB page for hugetlb, and sacrifices the randomness, which
> > is the heart of the KASLR. This patch tries to enlarge the randomness in
> > the cases where hugepages=X < the num Y of good candidate of GiB
> > page.
>
> Better tell how you improve in cover letter or patch log.
>
Yes, good advice.

> > To comparison, taking a typical KVM guest for example, the head 3GiB mem
> > can not be used as GiB hugetlb. Denoting the total mem size as Z(GiB), when
> > Z=5, then Y=2, assuming X=1, the randomness range before this patch is
> > 4GiB, after this patch is 5GiB, and get a 25% improvement of randomness.
> > If Z=6, then Y=3, assuming X=2, the improvement equals: 50%( 6/(6-2) - 1);
> > assuming X=1, the improvement will be: 20% (6/(6-1) - 1)
>
> Hmm, what if hugepages=1, or 2, even 100, but system owns 1PB memory?
>
Not agree, since the kvm guest is much popular, but most of them have
no much memory.

> Secondly, even in the case that hugepages=1, system memory is 5G, if no
> 'movable_node' specified, the last 1G can't be chosen for hugepage.
> Because memblock will allocate memory top to down. This is not
> improving, but make the code not work.
>
Yes, you are right. It is too hard to consider the runtime layout at
this very early stage.

> Lastly, getting better randomness is the heart of KASLR, while
> process_mem_region() is the heart of kernel KASLR code. Unless the
> current code blocks serious fix, we'd better not touch it. Keeping
> it can make the current code logic simple and more readable. It's

Yes.

Thanks,
Pingfan
> like a proved running well machine, we just dig out the unwanted
> middle, feed the good head and tail regions into it, then it gives
> back good slot for kernel position one by one.
>
> To sum up, I personally think this patchset is not a good idea.
>
> Thanks
> Baoquan