Hi Heinrich, On Thu, 13 Jun 2024 at 11:32, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > > On 13.06.24 18:59, Simon Glass wrote: > > Hi Tom, > > > > On Thu, 13 Jun 2024 at 09:42, Tom Rini <tr...@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 <tr...@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 <tr...@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 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. > > 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 should not make any assumptions that conflict with the UEFI > specification. Indeed. > > In our initial discussion with Ilias one idea was to merge LMB and EFI > memory management. This merged system would have to consider the > requirements of the UEFI specifications like a finer grained memory type > system and page boundaries. > > Best regards > > Heinrich > > > > > This is all quite subtle and probably worthy of a VC discussion. > > > >> > >> But that also raised the more general problem, and why we need a > >> persistent reservation list, of allowing boards/SoCs to say they want to > >> reserve a block of memory for whatever, and have that obeyed, for real. > >> For example, the mach-apple logic of "just pick some memory locations to > >> use for kernel/dtb/initrd" isn't really as safe as it should be since > >> those reservations aren't really seen anywhere once the function > >> returns, it's just setting some environment variables. > > > > Yes, that part of it I understand. Somehow I either didn't see or > > forgot that board_late_init() code. With the script-based boot it > > makes some sort of sense, but with bootstd we should have allocation > > of addresses dealt with there. I have held off on retiring > > kernel_addr_r etc. as the scripts are still in use. But perhaps it > > would be a good time to convert bootstd to use lmb instead? > > > > Regards, > > Simon >
