linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Baoquan He <bhe@redhat.com>
To: Borislav Petkov <bp@suse.de>
Cc: linux-kernel@vger.kernel.org, tglx@linutronix.de, hpa@zytor.com,
	mingo@redhat.com, x86@kernel.org, keescook@chromium.org,
	yinghai@kernel.org, thgarnie@google.com, kuleshovmail@gmail.com,
	luto@kernel.org, mcgrof@kernel.org, anderson@redhat.com,
	dyoung@redhat.com, xlpang@redhat.com
Subject: Re: [PATCH v2 2/2] x86/KASLR/64: Determine kernel text mapping size at runtime
Date: Sat, 10 Dec 2016 21:41:56 +0800	[thread overview]
Message-ID: <20161210134156.GF1034@x1> (raw)
In-Reply-To: <20161210123340.begeyyiegktangh2@pd.tnic>

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

  reply	other threads:[~2016-12-10 13:42 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-09 14:41 [PATCH v2 0/2] Determine kernel text mapping size at runtime for x86_64 Baoquan He
2016-12-09 14:41 ` [PATCH v2 1/2] x86/64: Make kernel text mapping always take one whole page table in early boot code Baoquan He
2016-12-09 14:41 ` [PATCH v2 2/2] x86/KASLR/64: Determine kernel text mapping size at runtime Baoquan He
2016-12-10 10:31   ` Borislav Petkov
2016-12-10 12:27     ` Baoquan He
2016-12-10 12:33       ` Borislav Petkov
2016-12-10 13:41         ` Baoquan He [this message]
2016-12-10 16:28           ` Borislav Petkov
2016-12-11 10:58             ` Baoquan He
2016-12-11 12:06               ` Borislav Petkov
2016-12-12  2:32                 ` Baoquan He

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161210134156.GF1034@x1 \
    --to=bhe@redhat.com \
    --cc=anderson@redhat.com \
    --cc=bp@suse.de \
    --cc=dyoung@redhat.com \
    --cc=hpa@zytor.com \
    --cc=keescook@chromium.org \
    --cc=kuleshovmail@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=thgarnie@google.com \
    --cc=x86@kernel.org \
    --cc=xlpang@redhat.com \
    --cc=yinghai@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).