Re: [PATCH v2 0/2] EFI chainloader improvement

[Daniel Kiper]

On Thu, Jun 01, 2023 at 09:33:25AM +0200, Ard Biesheuvel wrote:
> On Thu, 1 Jun 2023 at 06:17, Glenn Washburn <development@efficientek.com> 
> wrote:
> >
> > Changes since v1:
> >  * Rebase onto latest master
> >  * Change logic: The device path argument to image load changed back to the 
> > way
> >    it was originally where the argument is set to the device path that $root
> >    resolves to. If $root does not resolve or is not a device, the argument
> >    will be NULL as allowed for in the spec. By setting $root to the device 
> > of
> >    the chainloaded file, v1 behavior can be had. So this is more versatile
> >    behavior.
> >  * Minor rewording of metadata.
> >
> > This series improves the EFI chainloader. I've noticed for a while now that
> > chainloading would fail when root=memdisk. It didn't really make sense 
> > because
> > I was specifying the image to chainload as device+path, so why would it care
> > about what my root was. But I noticed that if I changed the root to the 
> > device
> > the image file was located on, then chainloading worked. The second patch
> > fixes this by removing some previous assumptions that I don't believe are
> > valid (eg. that LoadImage needs a valid device path).
> >
>
> I don't think it ultimately matters that much what the device path is
> when you provide the image via buffer+size, but I will note that the
> generic EFI linux loader handles this case by constructing a 'memory
> mapped' device path (in grub_arch_efi_linux_boot_image()), which just
> describes a range of memory.
>
> However, given that the device path in question is not installed on a
> handle, I'm not even sure whether creating that memory mapped device
> path serves any purpose tbh.
>
> So this approach seems perfectly reasonable to me.
>
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Thank you for fixing this issue!

Daniel