Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data

From: "H. Peter Anvin" <hpa@zytor.com>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: pbonzini@redhat.com, ebiggers@kernel.org, x86@kernel.org,
	linux-kernel@vger.kernel.org, qemu-devel@nongnu.org,
	ardb@kernel.org, kraxel@redhat.com, bp@alien8.de,
	philmd@linaro.org
Subject: Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
Date: Wed, 28 Dec 2022 15:58:12 -0800	[thread overview]
Message-ID: <9188EEE9-2759-4389-B39E-0FEBBA3FA57D@zytor.com> (raw)
In-Reply-To: <Y6x1knb8udpSyMSp@zx2c4.com>

On December 28, 2022 8:57:54 AM PST, "Jason A. Donenfeld" <Jason@zx2c4.com> wrote:
>HELLO H. PETER ANVIN,
>E
>L
>L
>O
>
>On Wed, Dec 28, 2022 at 05:30:30PM +0100, Jason A. Donenfeld wrote:
>> > Fix looks good, glad you figured out the problem.
>> 
>> I mean, kind of. The solution here sucks, especially given that in the
>> worst case, setup_data just gets dropped. I'm half inclined to consider
>> this a kernel bug instead, and add some code to relocate setup_data
>> prior to decompression, and then fix up all the links. It seems like
>> this would be a lot more robust.
>> 
>> I just wish the people who wrote this stuff would chime in. I've had
>> x86@kernel.org CC'd but so far, no input from them.
>
>Apparently you are the x86 boot guru. What do you want to happen here?
>Your input would be very instrumental.
>
>Jason

Hi!

Glad you asked.

So the kernel load addresses are parameterized in the kernel image setup header. One of the things that are so parameterized are the size and possible realignment of the kernel image in memory.

I'm very confused where you are getting the 64 MB number from. There should not be any such limitation.

In general, setup_data should be able to go anywhere the initrd can go, and so is subject to the same address cap (896 MB for old kernels, 4 GB on newer ones; this address too is enumerated in the header.)

If you want to put setup_data above 4 GB, it *should* be ok if and only if the kernel supports loading the initrd high, too (again, enumerated in the header.

TL;DR: put setup_data where you put the initrd (before or after doesn't matter.)

To be maximally conservative, link the setup_data list in order from lowest to highest address; currently there is no such item of relevance, but in the future there may be setup_data items needed by the BIOS part of the bootstrap in which case they would have to be < 1 MB and precede any items > 1 MB for obvious reasons. That being said, with BIOS dying it is not all that likely that such entries will ever be needed.