On Thu, Dec 03, 2020 at 04:01:49PM +0100, Javier Martinez Canillas wrote: > The shim_lock module registers a verifier to call shim's verify, but the > handler is registered even when the shim_lock protocol was not installed. > > This doesn't cause a NULL pointer dereference in shim_lock_write() because > the shim_lock_init() function just returns GRUB_ERR_NONE if sl isn't set. > > But in that case there's no point to even register the shim_lock verifier > since won't do anything. Additionally, it is only useful when Secure Boot > is enabled. > > Finally, don't assume that the shim_lock protocol will always be present > when the shim_lock_write() function is called, and check for it on every > call to this function. > > Reported-by: Michael Chang <mch...@suse.com>
To complete the information here, this fixed the problem I tried to solve before, but in a more elegant way. :) https://www.mail-archive.com/grub-devel@gnu.org/msg30738.html Thank you to work on the patch. Regards, Michael > Reported-by: Peter Jones <pjo...@redhat.com> > Signed-off-by: Javier Martinez Canillas <javi...@redhat.com> > --- > > grub-core/commands/efi/shim_lock.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/grub-core/commands/efi/shim_lock.c > b/grub-core/commands/efi/shim_lock.c > index d8f52d721c3..5259b27e8fc 100644 > --- a/grub-core/commands/efi/shim_lock.c > +++ b/grub-core/commands/efi/shim_lock.c > @@ -28,7 +28,6 @@ > GRUB_MOD_LICENSE ("GPLv3+"); > > static grub_efi_guid_t shim_lock_guid = GRUB_EFI_SHIM_LOCK_GUID; > -static grub_efi_shim_lock_protocol_t *sl; > > /* List of modules which cannot be loaded if UEFI secure boot mode is > enabled. */ > static const char * const disabled_mods[] = {"iorw", "memrw", "wrmsr", NULL}; > @@ -43,9 +42,6 @@ shim_lock_init (grub_file_t io, enum grub_file_type type, > > *flags = GRUB_VERIFY_FLAGS_SKIP_VERIFICATION; > > - if (!sl) > - return GRUB_ERR_NONE; > - > switch (type & GRUB_FILE_TYPE_MASK) > { > case GRUB_FILE_TYPE_GRUB_MODULE: > @@ -100,6 +96,11 @@ shim_lock_init (grub_file_t io, enum grub_file_type type, > static grub_err_t > shim_lock_write (void *context __attribute__ ((unused)), void *buf, > grub_size_t size) > { > + grub_efi_shim_lock_protocol_t *sl = grub_efi_locate_protocol > (&shim_lock_guid, 0); > + > + if (sl == NULL) > + 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")); > > @@ -115,12 +116,13 @@ struct grub_file_verifier shim_lock = > > GRUB_MOD_INIT(shim_lock) > { > - sl = grub_efi_locate_protocol (&shim_lock_guid, 0); > - grub_verifier_register (&shim_lock); > + grub_efi_shim_lock_protocol_t *sl = grub_efi_locate_protocol > (&shim_lock_guid, 0); > > - if (!sl) > + if (sl == NULL || grub_efi_get_secureboot () != > GRUB_EFI_SECUREBOOT_MODE_ENABLED) > return; > > + grub_verifier_register (&shim_lock); > + > grub_dl_set_persistent (mod); > } > > -- > 2.28.0 > _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel