Hi Heinrich,
I'm about to send v4 patch series.
- keep if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) condition
I chose this option, but I reverted #ifdef statement instead of using "if (IS_ENABLED)" because I think it is better not to rely on compiler optimization.
- remove if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) condition and always include efi_signature.c as compilation target.
In this option, CONFIG_PKCS7_VERIFY is required for EFI_TCG2_PROTOCOL just for successful build. To minimize dependency, I did not proceed with 2).
Please kindly review v4.
Thanks, Masahisa
On Tue, 11 May 2021 at 07:06, Masahisa Kojima masahisa.kojima@linaro.org wrote:
On Mon, 10 May 2021 at 11:07, Takahiro Akashi takahiro.akashi@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.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().
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.
- keep if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) condition
- 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