[RFC PATCH 04/31] lmb: remove local instances of the lmb structure variable

[Simon Glass]

Hi Ilias,

On Thu, 13 Jun 2024 at 23:58, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> On Thu, 13 Jun 2024 at 23:11, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >
> >
> >
> > Am 13. Juni 2024 21:05:58 MESZ schrieb Simon Glass <sjg at chromium.org>:
> > >Hi Heinrich,
> > >
> > >On Thu, 13 Jun 2024 at 11:32, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > >>
> > >> On 13.06.24 18:59, Simon Glass wrote:
> > >> > Hi Tom,
> > >> >
> > >> > On Thu, 13 Jun 2024 at 09:42, Tom Rini <trini at konsulko.com> wrote:
> > >> >>
> > >> >> On Thu, Jun 13, 2024 at 09:22:15AM -0600, Simon Glass wrote:
> > >> >>> Hi Tom,
> > >> >>>
> > >> >>> On Wed, 12 Jun 2024 at 15:40, Tom Rini <trini at konsulko.com> wrote:
> > >> >>>>
> > >> >>>> On Wed, Jun 12, 2024 at 02:24:25PM -0600, Simon Glass wrote:
> > >> >>>>> Hi Tom,
> > >> >>>>>
> > >> >>>>> On Wed, 12 Jun 2024 at 11:22, Tom Rini <trini at konsulko.com> wrote:
> > >> >>>>>>
> > >> >>>>>> On Tue, Jun 11, 2024 at 08:41:39PM -0600, Simon Glass wrote:
> > >> >>>>>>
> > >> >>>>>> [snip]
> > >> >>>>>>> Also IMO there is only really one LMB list today. We create it at the
> > >> >>>>>>> start of bootm and then it is done when we boot. The file-loading
> > >> >>>>>>> stuff is what makes all this confusing...and with bootstd that is
> > >> >>>>>>> under control as well.
> > >> >>>>>>>
> > >> >>>>>>> At lot of this effort seems to be about dealing with random scripts
> > >> >>>>>>> which load things. We want to make sure we complain if something
> > >> >>>>>>> overlaps. But we should be making the bootstd case work nicely and
> > >> >>>>>>> doing things within that framework. Also EFI sort-of has its own
> > >> >>>>>>> thing, which it is very-much in control of.
> > >> >>>>>>>
> > >> >>>>>>> Overall I think this is a bit more subtle that just combining allocators.
> > >> >>>>>>
> > >> >>>>>> I think this gets to the main misunderstanding. The problem isn't
> > >> >>>>>> handling bootstd, or EFI boot, or even assorted scripts. Those are all
> > >> >>>>>> cases where things are otherwise (sufficiently) well-defined. The
> > >> >>>>>> problem is "security" and that a "carefully crafted payload" could do
> > >> >>>>>> something malicious. That's why we have to do all of this stuff sooner
> > >> >>>>>> rather than later in our boot process.
> > >> >>>>>
> > >> >>>>> That's the first I have heard of this, actually, but a bit more detail
> > >> >>>>> would help. How does the payload get loaded? I'm just not sure about
> > >> >>>>> the overall goals. It seems that everyone else is already familiar -
> > >> >>>>> can someone please take the time to point me to the details?
> > >> >>>>
> > >> >>>> Well, the short version I believe of the first CVE we got (and so
> > >> >>>> started abusing LMB) was along the lines of "load an image near where
> > >> >>>> the U-Boot stack is, smash things for fun and exploits".
> > >> >>>
> > >> >>> OK. I am surprised that LMB does not catch that. It is supposed to add
> > >> >>> the stack and various other things right at the start before loading
> > >> >>> any file. So even if it clears the LMB each time, it should not be
> > >> >>> able to do that. Having said this, the code may be buggy as I don't
> > >> >>> think we have tests for U-Boot's overall functional behaviour in these
> > >> >>> situations.
> > >> >>
> > >> >> Right, LMB does catch the example I gave (because we made all of the
> > >> >> load from storage/network functions init an lmb and we always make sure
> > >> >> a new lmb gets U-Boot stack/etc). The next thing we didn't catch was
> > >> >> "what if EFI does the loading?" and we've kludged around that, and in
> > >> >> turn had some of the thorny questions. Some of that is what I think
> > >> >> you're asking about in this part of the thread, to which the answer is
> > >> >> "EFI spec says you need to place X in memory", so we just need to
> > >> >> reserve it when it's asked for, so that something else can't come along
> > >> >> and smash it maliciously.
> > >> >
> > >> > OK I see. Of course it isn't just EFI that has this issue. I believe
> > >> > the answer (for small blocks) is to use malloc(), which I think we do
> > >> > with a few exceptions which Ilias pointed out. For things like the TPM
> > >> > log and ACPI tables we should probably use a bloblist, as we do on
> > >> > x86. For large things (like loading a kernel) we should use LMB. I've
> > >> > been thinking about how best to tie this to boot, as opposed to random
> > >> > allocations in U-Boot itself, which would lead to fragmentation and
> > >> > strange behaviour. I think bootstd is a great place to have a
> > >> > persistent LMB. It can be attached to bootstd_priv.
> > >> >
> > >> > My hope is that EFI is just another boot method, where
> > >> > already-allocating things are presented to the OS. Apart from the
> > >> > Ilias exceptions, I believe this is how it works today.
> > >> >
> > >> > Where I think this heads in the wrong direction is using
> > >> > EFI-allocation functions before we are booting an EFI image. EFI has
> > >> > no concept of what is 'in empty space' so it leads to the lmb
> > >> > conflict, the subject of this discussion.
> > >>
> > >> EFI binaries can return to the command line interface.
> > >> EFI binaries may be drivers that stay resident and run in the background
> > >> after returning to the command line interface. They might for instance
> > >> provide block devices.
> > >
> > >Hmm you mentioned that before but I'm still not quite understanding.
> > >Do you mean that the EFI app returns back to U-Boot, leaving the
> >
> > There are 3 types of of EFI binaries:
> >
> > * apps
> > * boot time drivers
> > * run time drivers
> >
> >
> > >driver active? If so, how does U-Boot use the driver? I'm just not
> > >familiar with how such a construct could work in a single-threaded
> > >U-Boot.
> >
> > We already have code for EFI block devices. See lib/efi_driver/
> >
> > When U-Boot reads from such a block device it may end up in an EFI driver that for instance uses U-Boots network drivers exposed as EFI simple network protocol to read from an NVMEoF or iSCSI server.
> >
> > >
> > >>
> > >> Device-paths must be created from EFI pool memory as they may be freed
> > >> via FreePool() according to the EFI specification. And these we create
> > >> whenever a block-device is probed.
> > >
> > >The implementation of pool memory rounds up to pages and uses that. It
> > >might be more efficient to use malloc()/free() instead, given that we
> > >mark the malloc() space as reserved when we call the EFI app.
> >
> > We have requirements on which EFI allocations may reside in the same page. See the ARM chapter in the UEFI specification.
>
> And apart from all Heinrich mentioned, EFI memory has to be marked and
> preserved at runtime for runtime services and data.
> Using malloc for the variables, the eventlog etc, would just
> complicate the EFI code, as we'd have to copy those just before
> ExitBootServices is called.

Since malloc() is marked as boot-time memory, you can just leave them
where they are. Lots of things are in the malloc() region anyway.

Regards,
Simon