Re: [PATCH] x86: fix q35 kernel measurements broken due to rng seeding

[H. Peter Anvin]

On February 2, 2023 7:17:01 AM PST, James Bottomley <jejb@linux.ibm.com> wrote:
>On Thu, 2023-02-02 at 07:03 -0800, H. Peter Anvin wrote:
>[...]
>> NAK. We need to fix the actual problem of the kernel stomping on
>> memory it shouldn't, not paper around it.
>
>This is a first boot situation, not kexec (I just updated kexec because
>it should use any new mechanism we propose).  Unlike kexec, for first
>boot we're very constrained by the amount of extra space QEMU has to do
>this.  The boot_params are the first page of the kernel load, but the
>kernel proper begins directly after it, so we can't expand it.  The two
>schemes tried: loading after the kernel and loading after the command
>line both tamper with integrity protected files, so we shouldn't use
>this mechanism.  This is the essence of the problem: If we add this
>area at boot, it has to go in an existing memory location; we can't
>steal random guest areas.  All current config parameters are passed
>through as fw_config files, so we can only use that mechanism *if* we
>know where the area ends up in the loaded kernel *and* the file isn't
>integrity protected (this latter is expanding over time).
>
>If we could wind back time, I'd have added the 32 byte random seed to
>boot_params properly not coded it as a setup_data addition, but now
>we're stuck with coping with existing behaviour, which is why I thought
>the retro fit to boot_params would be the better path forward, but if
>you have any alternatives, I'm sure we could look at them.
>
>James
>

The right thing to do is to fix the kernel so that it doesn't stomp on this 
memory, just as it cannot stomp on boot_params, initrd, or the command line. 
The kernel boot protocol defines a keep-out area, but physical kaslr violates 
it and so the kaslr code in the decompressor is responsible for keeping track 
of the keepout areas, and apparently noone every did.

Adding it to boot_params and bumping the version number is a hack that doesn't 
solve the backwards compatibility problem, so we should just fix the bug 
instead. Adding it to boot_params and adding a setup_data pointer MAY be 
backwards compatible, but it also enables an absolutely catastrophic failure 
mode: an unaware loader may end up relocating boot_params without knowing that 
that data has a secondary pointer into it, with the result that we now have a 
bogus pointer in a linked list. Not good.

Fixing the bug properly is also the only way forward for future setup_data 
users, and we are running low on space in the fixed-size structures.