linux-kernel - Re: [PATCH v2 2/2] x86/KASLR/64: Determine kernel text mapping size at runtime

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: <20161210134156.GF1034@x1>
Date:   Sat, 10 Dec 2016 21:41:56 +0800
From:   Baoquan He <bhe@...hat.com>
To:     Borislav Petkov <bp@...e.de>
Cc:     linux-kernel@...r.kernel.org, tglx@...utronix.de, hpa@...or.com,
        mingo@...hat.com, x86@...nel.org, keescook@...omium.org,
        yinghai@...nel.org, thgarnie@...gle.com, kuleshovmail@...il.com,
        luto@...nel.org, mcgrof@...nel.org, anderson@...hat.com,
        dyoung@...hat.com, xlpang@...hat.com
Subject: Re: [PATCH v2 2/2] x86/KASLR/64: Determine kernel text mapping size
 at runtime

On 12/10/16 at 01:33pm, Borislav Petkov wrote:
> On Sat, Dec 10, 2016 at 08:27:57PM +0800, Baoquan He wrote:
> > Whether CONFIG_RANDOMIZE_BASE is yes or not, with 'nokaslr' specified,
> > Kernel text mapping size should be 512M, just the same as no kaslr code
> > compiled in.
> 
> "should be" still doesn't really explain what the problem is. What's
> wrong with it remaining 1G?
> 
> IOW, something like "The problem is X and the issues it causes are Y.
> That's why we need to do Z."
> 
> Now please replace X,Y and Z with the real content.

I made this patchset because of two things:

1) Fedora 25 defaults to enable CONFIG_RANDOMIZE_BASE. And this worries
maintainers of several Fedora component. People ever asked me how to
judge whether it's a kaslr kernel. I told them I usually read elf header
of kcore - "readelf -l /proc/kcore" to check it. If the 'VirtAddr' of
segments like kernel text, modules, direct mapping is changed, it should
be kaslr kernel. Then they said why I have specified 'nokaslr', the
virtual address of modules is not '0xffffffffa0000000', but
'0xffffffffc0000000'. OK, I realized this is not right, it need be
fixed.

2) The second thing I remember Dave said he judged the kaslr kernel by
the value of KERNEL_IMAGE_SIZE. Then I decide this is a wrong behaviour
and I should change it. But in v1 post, Dave denied this. Checking Crash
code, what he has done is like below:

if ((kt->flags2 & KASLR) && (THIS_KERNEL_VERSION >= LINUX(4,7,0)))                                                                               
        machdep->machspec->modules_vaddr = __START_KERNEL_map + 
                (machdep->machspec->kernel_image_size ?
                machdep->machspec->kernel_image_size : GIGABYTES(1));

You can see that if a kaslr kernel, Dave will get modules_vaddr by
KERNEL_IAMGE_SIZE or 1G directly if that value is not exported. However
KERNEL_IMAGE_SIZE is always 1G as long as CONFIG_RANDOMIZE_BASE is set
to 'y', whether kaslr is enabled or not. As you said, in this case,
remaining 1G doesn't impact things.

So in v2 I didn't mention problem about Crash. But case 1) need be
cared, whether kaslr code is compiled or not, it should not confuse
people, should not make difference between kaslr code not compiled in
and kaslr code compiled in with 'nokaslr' specified. Now the thing is
I am wondering if confusing people is a problem. Except of this I
haven't get report that it impacts things and caused problem.

Usually in redhat we have a convention that when we fix a bug, we
should write patch log like this:
what: what problem you have met.
why:  why does it happen, what is the root cause you got from analysis.
how   How do you fix it in this patch.

I personally think the 'what' is 'Y' you mentioned, and 'why' is 'X'.
Whatever it is, it's good if people can easily understand what you say.
In this patch log, since the problem is so obvious, I mean the
confusing difference, when I descirbed the proble, the root cause has
been told too. 

So I would like to adjust the log as you suggested, does it please you?
____________________________________________________________________
X86 64 kernel takes KERNEL_IMAGE_SIZE as the kernel text mapping size,
and it's fixed as compiling time, changing from 512M to 1G as long as
CONFIG_RANDOMIZE_BASE is enabled, though people specify kernel option
"nokaslr" explicitly. This is really confusing people when check if it's
a kaslr kernel, E.g checking the outout of 'readelf -l /proc/kcore'.

This is obviously a wrong behaviour. CONFIG_RANDOMIZE_BASE should only
decide if the KASLR code need be compiled in. If user specify "nokaslr",
the kernel should behave as no KASLR code compiled in at all.

So in this patch, define a new MACRO KERNEL_MAPPING_SIZE to represent
the size of kernel text mapping area, and let KERNEL_IMAGE_SIZE limit
the size of kernel runtime space. And change to determine the size of
kernel text mapping area at runtime. Though KASLR code compiled in, if
"nokaslr" specified, still set kernel mapping size to be 512M.

Thanks
Baoquan