Re: [PATCH v2] hw/i386: place setup_data at fixed place in memory

From: Laszlo Ersek <lersek@redhat.com>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: "Ard Biesheuvel" <ardb@kernel.org>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Xiaoyao Li" <xiaoyao.li@intel.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	linux-efi <linux-efi@vger.kernel.org>
Subject: Re: [PATCH v2] hw/i386: place setup_data at fixed place in memory
Date: Thu, 4 Aug 2022 15:29:19 +0200	[thread overview]
Message-ID: <5f947ca6-b178-9943-ece6-d6844285f8ee@redhat.com> (raw)
In-Reply-To: <5528ca40-50fb-8e92-7f24-80e20c4c983e@redhat.com>

On 08/04/22 15:16, Laszlo Ersek wrote:
> On 08/04/22 14:47, Jason A. Donenfeld wrote:
>> On Thu, Aug 4, 2022 at 2:11 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>>>
>>> Hi Laszlo,
>>>
>>> On Thu, Aug 04, 2022 at 01:31:36PM +0200, Laszlo Ersek wrote:
>>>> None of the existing info passing methods seem early enough, generic
>>>> enough, and secure enough (at the same time)...
>>>
>>> Can you look at the v2 patch? It seems to work on every configuration I
>>> throw at it. Keep in mind that setup_data is only used very, very early.
>>> I can think of a few other places to put it too, looking at the x86
>>> memory map, that will survive long enough.
>>>
>>> I think this might actually be a straightforwardly solvable problem if
>>> you think about it more basically.
>>
>> And just to put things in perspective here... We only need like 48
>> bytes or something at some easy fixed address. That's not much. That's
>> *got* to be a fairly tractable problem. If v2 has issues, I can't see
>> why there wouldn't be a different easy place to put a meger 48 bytes
>> of stuff that then is allowed to be wiped out after early boot.
> 
> I've looked at v2. It still relies on passing information from QEMU to
> the guest kernel through guest RAM such that the whole firmware
> execution takes place in-between, without the firmware knowing anything
> about that particular area -- effectively treating it as free system
> RAM. Such exceptions are time bombs.
> 
> We *have* used hard-coded addresses, sometimes they are unavoidable, but
> then they are open-coded in both QEMU and the firmware, and some early
> part of the firmware takes care to either move the data to a "safe"
> place, or to cover it in-place with a kind of reservation that prevents
> other parts of the firmware from trampling over it. I've debugged
> mistakes (memory corruption) when such reservation was forgotten; it's
> not fun.

Reference:

https://github.com/tianocore/edk2/commit/ad43bc6b2e

> 
> In short, I have nothing against the QEMU patch, but then the current
> OvmfPkg maintainers should accept a patch for the firmware too, for
> protecting the area from later firmware components, as early as possible.
> 
> Laszlo
>