Re: [PATCH] x86/boot: Refuse to build with data relocations

[Ingo Molnar]

* Kees Cook <keescook@xxxxxxxxxxxx> wrote:

> On Wed, May 18, 2016 at 4:29 AM, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> >
> > * Kees Cook <keescook@xxxxxxxxxxxx> wrote:
> >
> >> > I think there is something way more subtle going on here, and it bothers me
> >> > exactly because it is subtle.  It may be that it is OK right now, but there
> >> > are alarm bells going on all over my brain on this.  I'm going to stare at
> >> > this for a bit and see if I can make sense of it; but if it turns out that
> >> > what we have is something really problematic it might be better to apply a big
> >> > hammer and avoid future breakage once and for all.
> >>
> >> Sounds good. I would just like to decouple this from the KASLR improvements.
> >> This fragility hasn't changed as a result of that work, but I'd really like to
> >> have that series put to bed -- I've spent a lot of time already cleaning up it
> >> and other areas of the compressed kernel code. :)
> >
> > So I disagree on that: while technically kASLR is independent of relocations, your
> > series already introduced such a relocation bug and I don't want to further
> > increase complexity via kASLR without first increasing robustness.
> 
> Well, in my defense, the bug was never actually reachable.

Hm, the changelog says a crash/reboot might happen:

commit 434a6c9f90f7ab5ade619455df01ef5ebea533ee
Author: Kees Cook <keescook@xxxxxxxxxxxx>
Date:   Mon May 9 13:22:04 2016 -0700

    x86/KASLR: Initialize mapping_info every time
    
    As it turns out, mapping_info DOES need to be initialized every
    time, because pgt_data address could be changed during kernel
    relocation. So it can not be build time assigned.
    
    Without this, page tables were not being corrected updated, which
    could cause reboots when a physical address beyond 2G was chosen.

is the changelog wrong?

> > So could we try something to either detect or avoid such subtle and hard to 
> > debug relocation bugs in very early boot code?
> 
> I've sent this (the readelf patch which detects the bug from the KASLR series), 
> but hpa wants to do a more comprehensive version. Could we temporarily use my 
> version of this, since it appears to accomplish at least a subset of the new 
> goal?

Yeah, that's fine with me.

> And on a related topic, how would you like me to send Thomas Garnier's memory 
> base randomization series? Pull request, or as a series like I've done with the 
> other KASLR improvements?

A series (size limited if necessary) would be nice!

Thanks,

	Ingo