[PATCH v3 1/2] efi_loader: expose efi_image_parse() even if UEFI Secure Boot is disabled

[Takahiro Akashi <takahiro.akashi@linaro.org>]

On Mon, May 10, 2021 at 09:49:03AM +0900, Masahisa Kojima wrote:
> Hi Heinrich,
> 
> Sorry for the late reply.
> 
> On Sat, 8 May 2021 at 23:08, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > On 4/28/21 3:16 PM, Heinrich Schuchardt wrote:
> > > On 28.04.21 14:19, Masahisa Kojima wrote:
> > <snip />
> > >>   /**
> > >>    * cmp_pe_section() - compare virtual addresses of two PE image sections
> > >>    * @arg1:  pointer to pointer to first section header
> > >> @@ -504,6 +565,9 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)
> > >>
> > >>      EFI_PRINT("%s: Enter, %d\n", __func__, ret);
> > >>
> > >> +    if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT))
> > >> +            return true;
> > >> +
> > >
> > > Why is this needed? Doesn't efi_secure_boot_enabled() return false in
> > > this case?
> 
> The original code is as follows.

Heinrich's concern was, I guess, that

> > >> +    if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT))
> > >> +            return true;

and the succeeding check,

        if (!efi_secure_boot_enabled())
                        return true;

are somehow redundant.
But in the latter case, I'm afraid that a compiler cannot optimize out
the rest of the logic in efi_image_authenticate().

-Takahiro Akashi


> #ifdef CONFIG_EFI_SECURE_BOOT
> static bool efi_image_authenticate(void *efi, size_t efi_size) {
> 
>   < snip >
> 
>  }
> #else
> static bool efi_image_authenticate(void *efi, size_t efi_size)
> {
>        return true;
> }
> #endif /* CONFIG_EFI_SECURE_BOOT */
> 
> The purpose of this commit is removing #if compilation switch,
> so I keep the original implementation, always return true
> if CONFIG_EFI_SECURE_BOOT is disabled.
> 
> Thanks,
> Masahisa
> 
> >
> > Hello Masahisa,
> >
> > I did not see any reply yet. Was a mail lost?
> >
> > Best regards
> >
> > Heinrich