Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed

From: Baoquan He <bhe@redhat.com>
To: "H. Peter Anvin" <hpa@zytor.com>
Cc: Vivek Goyal <vgoyal@redhat.com>,
	Kees Cook <keescook@chromium.org>,
	linux-kernel@vger.kernel.org, tglx@linutronix.de,
	mingo@redhat.com, x86@kernel.org, ak@linux.intel.com,
	ebiederm@xmission.com, kexec@lists.infradead.org,
	whissi@whissi.de, kumagai-atsushi@mxc.nes.nec.co.jp,
	stable@vger.kernel.org
Subject: Re: [resend Patch v3 1/2] kaslr: check if kernel location is changed
Date: Sat, 11 Oct 2014 20:38:05 +0800	[thread overview]
Message-ID: <20141011123805.GC11560@dhcp-16-116.nay.redhat.com> (raw)
In-Reply-To: <543907B5.7060001@zytor.com>

On 10/11/14 at 03:34am, H. Peter Anvin wrote:
> On 10/10/2014 08:14 PM, Baoquan He wrote:
> >On 10/08/14 at 03:27pm, Vivek Goyal wrote:
> >>On Wed, Oct 08, 2014 at 08:09:59AM -0700, H. Peter Anvin wrote:
> >
> >>>Sorry... this makes no sense.
> >>>
> >>>For x86-64, there is no direct connection between the physical and
> >>>virtual address spaces that the kernel runs in...
> >>
> >>I am sorry I did not understand this one. I thought that initial
> >>relocatable kernel implementaion did not have any direct connection
> >>between virtual and physical address. One could load kernel anywhere
> >>and kernel virtual address will not change and we will just adjust
> >>page tables to map virtual address to right physical address.
> >>
> >>Now handle_relocation() stuff seems to introduce a close coupling
> >>between physical and virtual address. So if kernel shifts by 16MB
> >>in physical address space, then it will shift by equal amount
> >>in virtual address space. So there seems to be a direct connection
> >>between virtual and physical address space in this case.
> >
> >Yeah, it's exactly as Vivek said.
> >
> >Before kaslr was introduced, x86_64 kernel can be put anywhere, and
> >always _text is 0xffffffff81000000. Meanwhile phys_base contains the
> >offset between the compiled addr (namely 0x1000000) and kernel loaded
> >addr. After kaslr implementation was added, as long as kernel loaded
> >addr is different 0x1000000, it will call handle_relocations(). The
> >offset now is added onto each symbols including _text and phys_base
> >becomes 0.
> >
> >It's clearly showing that by checking /proc/kallsyms and value of
> >phys_base.
> >
> 
> This really shouldn't have happened this way on x86-64.  It has to
> happen this way on i386, but I worry that this may be a serious
> misdesign in kaslr on x86-64.  I'm also wondering if there is any
> other fallout of this?

Yes, this shouldn't happen this way on x86_64. With this patch, those
are fixed as expected. If kernel location is not chosen randomly, we
should not do the relocations handling. If and only if kaslr is enabled
and it relocated the kernel randomly as expected, we do the relocations
handling.

I think this patch really makes sense and it's simple and won't impact
i386 and other implementations.

Thanks
Baoquan