On Mon, 10 May 2021 at 11:07, Takahiro Akashi
<takahiro.aka...@linaro.org> wrote:
>
> 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.g...@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().
Hi Heinrich, Takahiro,
Sorry for the late reply.
I now understand Takahiro's concern.
If I remove following check,
> + if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT))
> + return true;
compiler optimization does not work and link error occurs.
lib/built-in.o: In function `efi_image_authenticate':
/home/ubuntu/SynQuacer/ledge/u-boot/lib/efi_loader/efi_image_loader.c:601:
undefined reference to `efi_sigstore_parse_sigdb'
/home/ubuntu/SynQuacer/ledge/u-boot/lib/efi_loader/efi_image_loader.c:607:
undefined reference to `efi_sigstore_parse_sigdb'
/home/ubuntu/SynQuacer/ledge/u-boot/lib/efi_loader/efi_image_loader.c:613:
undefined reference to `efi_signature_lookup_digest'
I would like to propose two resolution.
1) keep if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) condition
2) remove if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) condition and
always include efi_signature.c as compilation target.
Please advise.
Thanks,
Masahisa
>
> -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
