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

From: Ingo Molnar
Date: Fri May 20 2016 - 02:41:38 EST



* 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