lkml.org 
[lkml]   [2016]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2016-12-10 14:42    [W:0.044 / U:1.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site