Re: [PATCH 8/9] efi: Only register shim_lock verifier if shim_lock protocol is found and SB enabled

[Daniel Kiper]

On Mon, Dec 14, 2020 at 09:50:45PM +0800, Michael Chang via Grub-devel wrote:
> On Thu, Dec 10, 2020 at 05:50:53PM +0100, Daniel Kiper wrote:
> > On Tue, Dec 08, 2020 at 10:20:03AM +0800, Michael Chang via Grub-devel 
> > wrote:
> > > 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 <mchang@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.
> >
> > You are welcome!
> >
> > May I add your Tested-by do this patch?
>
> Sure you can. I have verified that it solved the problem, despite for

Thanks for confirmation but unfortunately I pushed the patches last week.

> the unexpected build error.
>
> ../../grub-core/commands/efi/shim_lock.c:121:21: error: implicit declaration 
> of function ‘grub_efi_get_secureboot’; did you mean 
> ‘grub_efi_get_device_path’? [-Werror=implicit-function-declaration]
>   121 |   if (sl == NULL || grub_efi_get_secureboot () != 
> GRUB_EFI_SECUREBOOT_MODE_ENABLED)
>
> FWIW, the trivial patch I use to get around above build error is
> included.

Yeah, I spotted the same and fixed it before pushing the patches.

Anyway, thank you for doing the tests.

Daniel