From: Michael Chang <mchang@suse.com>
To: Daniel Kiper <dkiper@net-space.pl>
Cc: grub-devel@gnu.org, Javier Martinez Canillas <javierm@redhat.com>,
Ignat Korchagin <ignat@cloudflare.com>,
Peter Jones <pjones@redhat.com>,
Marco A Benatto <mbenatto@redhat.com>,
Leif Lindholm <leif@nuviainc.com>
Subject: Re: [PATCH 8/9] efi: Only register shim_lock verifier if shim_lock protocol is found and SB enabled
Date: Mon, 14 Dec 2020 21:50:45 +0800 [thread overview]
Message-ID: <20201214135045.GA22092@mercury> (raw)
In-Reply-To: <20201210165053.mj3bop5uu6pbcjlo@tomti.i.net-space.pl>
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
>
next prev parent reply other threads:[~2020-12-14 13:51 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-03 15:01 [PATCH 0/9] Add UEFI Secure Boot detection and report the status to Linux Javier Martinez Canillas
2020-12-03 15:01 ` [PATCH 1/9] i386: Don't include <grub/cpu/linux.h> in coreboot and ieee1275 startup.S Javier Martinez Canillas
2020-12-03 15:01 ` [PATCH 2/9] include/grub/i386/linux.h: Include missing <grub/types.h> header Javier Martinez Canillas
2020-12-03 15:01 ` [PATCH 3/9] arm/term: Fix linking error due multiple ps2_state definitions Javier Martinez Canillas
2020-12-03 15:01 ` [PATCH 4/9] efi: Make shim_lock GUID and protocol type public Javier Martinez Canillas
2020-12-03 15:01 ` [PATCH 5/9] efi: Return grub_efi_status_t from grub_efi_get_variable() Javier Martinez Canillas
2020-12-03 15:01 ` [PATCH 6/9] efi: Add a function to read EFI variables with attributes Javier Martinez Canillas
2020-12-03 15:01 ` [PATCH 7/9] efi: Add secure boot detection Javier Martinez Canillas
2020-12-03 15:01 ` [PATCH 8/9] efi: Only register shim_lock verifier if shim_lock protocol is found and SB enabled Javier Martinez Canillas
2020-12-08 2:20 ` Michael Chang
2020-12-10 16:50 ` Daniel Kiper
2020-12-14 13:50 ` Michael Chang [this message]
2020-12-15 12:36 ` Daniel Kiper
2020-12-16 9:29 ` Javier Martinez Canillas
2020-12-03 15:01 ` [PATCH 9/9] loader/linux: Report the UEFI Secure Boot status to the Linux kernel Javier Martinez Canillas
2020-12-04 12:27 ` [PATCH 0/9] Add UEFI Secure Boot detection and report the status to Linux Daniel Kiper
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201214135045.GA22092@mercury \
--to=mchang@suse.com \
--cc=dkiper@net-space.pl \
--cc=grub-devel@gnu.org \
--cc=ignat@cloudflare.com \
--cc=javierm@redhat.com \
--cc=leif@nuviainc.com \
--cc=mbenatto@redhat.com \
--cc=pjones@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.