Re: [PATCH v2] x86/boot: Support uncompressed kernel

[Andy Lutomirski]

On Tue, Apr 4, 2017 at 12:14 PM, Kees Cook <keescook@xxxxxxxxxxxx> wrote:
> On Tue, Apr 4, 2017 at 1:52 AM, Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
>> -                       memmove(dest, output + phdr->p_offset, phdr->p_filesz);
>> +                       memmove(dest, buf + phdr->p_offset, phdr->p_filesz);
>
> With the renaming from "buf" to "input" this becomes much more
> comprehensible: the PT_LOAD segments from "input" are loaded into
> "output". However, does this mean that the RAW load uses parse_elf to
> move the kernel into place? Does this happen safely? If it's already
> safe, shouldn't we not need "input" at all, and leave this as-is, like
> how the decompressed kernel operate?

This is a bit of a brain dump of what I learned when I played with
this code a couple weeks ago:

parse_elf() is incredibly fragile.  It seems to work correct in two cases:

1. We're randomizing.  As far as I can tell, it just works.

2. We get lucky and all the memmoves are backwards.  The first segment
overwrites the headers, the second segment may overwrite the first,
etc.  This also requires that the (missing, ahem) memsets to zero out
the ends of segments don't overwrite anything.  Of course, nothing
accounts for them now and adding the missing memset breaks the boot.

#2 is very fragile, needless to say.

I'd love to see the code that sets up the decompressor figure out much
wiggle room is actually needed and allocate accordingly.