Ilias, On Fri, Sep 30, 2022 at 09:18:35AM +0300, Ilias Apalodimas wrote: > Akashi-san > > On Fri, 30 Sept 2022 at 04:47, AKASHI Takahiro > <[email protected]> wrote: > > > > On Fri, Sep 30, 2022 at 01:57:48AM +0200, Heinrich Schuchardt wrote: > > > Don't try to delete a non-existent handle. > > > > It is okay as a safe guard, but it doesn't fix underlying issues. > > I dont think we safeguard anything. That code path won't try to delete > anything regardless? > > > > > efi_initrd_register() is called only in efi_bootmgr_load(), and so > > efi_initrd_deregister() should be called only at the paired location. > > There's a reason for that. > > > > - Remove efi_initrd_deregister() from do_bootefi_exec() > > - do_efibootmgr() should look like > > efi_bootmgr_load() > > do_bootefi_exec() > > efi_initrd_deregister() > > Otherwise, do_bootefi_exec() always tries to free a handle in > > the other cases (i.e. bootefi <addr>). > > > > Another issue is there in try_load_entry() called by efi_bootmgr_load(). > > (after efi_initrd_register()) > > if (size) { > > *load_options = malloc(size); > > if (!*load_options) { > > ret = EFI_OUT_OF_RESOURCES; > > goto error; > > } > > ... > > > > If malloc() fails, we should call efi_initrd_deregister() within > > try_load_entry(). > > > > Should I submit a patch? > > The whole implementation on the *kernel* assumes the protocol is > present if the file it's pointing is real and existing. You also need > to have a single instance of the protocol installed. IOW if you > install the protocol and the initrd is not there, the kernel won't > fallback on the dt /chosen/ node or the initrd= in the cmdline.
Yes, I confirmed that before I made my comment. > The > whole initrd loading logic depends on BootCurrnent, which iirc is not > set yet on the flow you are proposing. I don't get your point. In do_efibootmgr(), what I suggested above is: - efi_bootmgr_load() installs LOAD_FILE2_PROTOCOL if initrd file exists, and if this function fails, LOAD_FILE2_PROTOCOL must be uninstalled any way. - after returning from UEFI app invoked by do_bootefi_exec(), - we should simply uninstall LOAD_FILE2_RPTOCOL by calling efi_initrd_deregister(). In "bootefi <addr>" case, efi_bootmgr_load() is not called, so LOAD_FILE2_PROTOCOL won't be installed for loading initrd file. Why do we have to call efi_initrd_deregister() in that case? Regarding BootCurrent, I don't think it has nothing to do with the discussion above. Anyhow it *is* set before reaching "if (size) ...". -Takahiro Akashi > Regards > /Ilias > > > > -Takahiro Akashi > > > > > Signed-off-by: Heinrich Schuchardt <[email protected]> > > > --- > > > lib/efi_loader/efi_load_initrd.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/lib/efi_loader/efi_load_initrd.c > > > b/lib/efi_loader/efi_load_initrd.c > > > index c5e6652e66..3d6044f760 100644 > > > --- a/lib/efi_loader/efi_load_initrd.c > > > +++ b/lib/efi_loader/efi_load_initrd.c > > > @@ -230,6 +230,9 @@ efi_status_t efi_initrd_register(void) > > > */ > > > void efi_initrd_deregister(void) > > > { > > > + if (!efi_initrd_handle) > > > + return; > > > + > > > efi_delete_handle(efi_initrd_handle); > > > efi_initrd_handle = NULL; > > > } > > > -- > > > 2.37.2 > > >
