Re: [PATCH] x86/boot/KASLR: Skip relocation handling in no kaslr case

[Baoquan He]

Hi Ingo,

Thanks for looking into this patch!

On 06/26/17 at 11:47am, Ingo Molnar wrote:
> 
> * Baoquan He <bhe@xxxxxxxxxx> wrote:
> 
> > Kdump kernel will reset to firmware after crash is trigered when
> > crashkernel=xxM,high is added to kernel command line. Kexec has the
> > same phenomenon. This only happened on system with kaslr code
> > compiled in and kernel option 'nokaslr'is added. Both of them works
> > well when kaslr is enabled.
> > 
> > When crashkernel high is set or kexec case, kexec/kdump kernel will be
> > put above 4G. Since we assign the original loading address of kernel to
> > virt_addr as initial value, the virt_addr will be larger than 1G if kaslr
> > is disabled, it exceeds the kernel mapping size which is only 1G. Then
> > it will cause relocation handling error in handle_relocations().
> 
> So instead of whacking yet another kexec mole, how could we turn this into a more 
> debuggable warning (either during build or during the failed bootup) instead of a 
> crash and reset (triple fault?) back to the BIOS screen?

This is a good question.

In fact this might not be kexe only problem. It's actually a code bug.
For x86_64, kernel text kaslr is separated into physical and virtual
address randomization. And here the virtual addr randmoization, can only
be done inside [0xffffffff80000000, 0xffffffffc0000000), the 1G area.
When we do the virtual address randomization, we only randomize to get
an offset between 0 and 1G, then add this offset onto the starting
address, 0xffffffff80000000. In the current code, virt_addr represents
the offset, which is a little confusing. In my original patch, it's
named as virt_offset(the link of original patch is pasted below). Kees
helped refactor the code, that corner case could be forgotton.

https://github.com/baoquan-he/linux/commit/034fbed941c60099368a40058cdfd0b4cac76040

If from the point of view of the offset of virtual address, which is among
kernel mapping area [0xffffffff80000000, 0xffffffffc0000000), the 'output'
which is the original loading address of kernel, absolutely should not be
assigned to virt_addr (better renamed as virt_offset or something else).

Here, kexec/kdump is only a pratical use case. I know someone ever
modifed bootloader to make kernel can be loaded arbitrary address, which
is not 16M, decided at compiled time. Then kernel will fail too.

As you suggested, we can add a checking to see if the virt_addr is
bigger than 1G, and print warning if exceed or hang there with error
message.

> 
> If kexec/kdump wants to do crazy things they should at least be _debuggable_ in a 
> straightforward manner.

So, it may not be fault of kexec/kdump, it's a code bug. I guess we
allow kernel to be loaded at any address, but not the address decided at
compiled time, 16M, right?

Thanks
Baoquan