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

[Simon Glass]

Hi Heinrich,

On Thu, 13 Jun 2024 at 14:16, 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.

Sure, but this is running on the same CPU / thread, right? There isn't
anything actually complicated about that.

>
> >
> >>
> >> 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.

I don't believe the pool allocator allows that, though...not that I am
an expert on this.

Regards,
Simon