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

[Michael Chang]

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
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.

diff --git a/grub-core/commands/efi/shim_lock.c 
b/grub-core/commands/efi/shim_lock.c
index 5259b27e8..b0c3cc178 100644
--- a/grub-core/commands/efi/shim_lock.c
+++ b/grub-core/commands/efi/shim_lock.c
@@ -24,6 +24,7 @@
 #include <grub/file.h>
 #include <grub/misc.h>
 #include <grub/verify.h>
+#include <grub/efi/sb.h>

 GRUB_MOD_LICENSE ("GPLv3+");

Thanks,
Michael

> 
> Daniel
>