On 7/15/21 4:43 PM, Sayanta Pattanayak wrote: > Hi Javier, > > I tried with your suggested change, but observing Exception as following - >
Thanks for testing. [snip] > > Synchronous Exception at 0x00000000F92699DC > Synchronous Exception at 0x00000000F92699DC Hmm, I found another bug in the patch since the error was returned inside the for loop and not after that. So may lead to a NULL pointer dereference error if not using the --disable-shim-lock option but booting without shim. [snip] > > Another doubt, should the Image be detected as "UEFI stub kernel", as > happened with experimental suggestion by Daniel? > I don't think is needed but I'll leave that to Daniel. > One minor addition in your patch, added below. > Thanks for that. That happen when I write a patch without even build testing it.... Can you give it a try to this one now? I built tested this time but still couldn't test it. I should be able to do that but no earlier than next week. From a7c205faef72df4dd6decb114b35b53941c17014 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas <javi...@redhat.com> Date: Thu, 15 Jul 2021 13:08:11 +0200 Subject: [RFC PATCH v2] kern/efi/sb: Allow validation to be done by the UEFI firmware The shim_lock protocol is used to delegate that PE32+ binaries have been signed with a trusted key. This is done because GRUB currently lacks the ability to do the validation itself. But in certain configurations a user may not want to use shim for this, and either delegate on a different verifier (i.e: pgp) or just leave it to the UEFI firmware. The latter can be done if both GRUB and the Linux kernel have been signed by a key trusted by the UEFI firmware. There's an grub-mkimage --disable-shim-lock option that could be used to avoid using he shim_lock protocol and rely on another verifier, but that will not work for the latter case. Since the lockdown verifier defers it to another verifier but no verifier validates the Linux kernel images. To workaround that, let's make the shim_lock verifier always validate a kernel file type if the --disable-shim-lock option has been enabled. Reported-by: Sayanta Pattanayak <sayanta.pattana...@arm.com> Signed-off-by: Javier Martinez Canillas <javi...@redhat.com> --- grub-core/kern/efi/sb.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c index c52ec6226a6..479f4adcba4 100644 --- a/grub-core/kern/efi/sb.c +++ b/grub-core/kern/efi/sb.c @@ -141,10 +141,19 @@ shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)), static grub_err_t shim_lock_verifier_write (void *context __attribute__ ((unused)), void *buf, grub_size_t size) { + struct grub_module_header *header; grub_efi_shim_lock_protocol_t *sl = grub_efi_locate_protocol (&shim_lock_guid, 0); if (!sl) - return grub_error (GRUB_ERR_ACCESS_DENIED, N_("shim_lock protocol not found")); + { + /* shim_lock is missing, check if GRUB image is built with --disable-shim-lock. */ + FOR_MODULES (header) + { + if (header->type == OBJ_TYPE_DISABLE_SHIM_LOCK) + return GRUB_ERR_NONE; + } + return grub_error (GRUB_ERR_ACCESS_DENIED, N_("shim_lock protocol not found")); + } if (sl->verify (buf, size) != GRUB_EFI_SUCCESS) return grub_error (GRUB_ERR_BAD_SIGNATURE, N_("bad shim signature")); @@ -162,20 +171,9 @@ struct grub_file_verifier shim_lock_verifier = void grub_shim_lock_verifier_setup (void) { - struct grub_module_header *header; grub_efi_shim_lock_protocol_t *sl = grub_efi_locate_protocol (&shim_lock_guid, 0); - /* shim_lock is missing, check if GRUB image is built with --disable-shim-lock. */ - if (!sl) - { - FOR_MODULES (header) - { - if (header->type == OBJ_TYPE_DISABLE_SHIM_LOCK) - return; - } - } - /* Secure Boot is off. Do not load shim_lock. */ if (grub_efi_get_secureboot () != GRUB_EFI_SECUREBOOT_MODE_ENABLED) return; -- 2.31.1 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel