Re: UEFI Secureboot not succeeding with Grub 2.06 and later version
Hello Sayanta,
On 7/15/21 7:26 AM, Sayanta Pattanayak wrote:
[snip]
>>> We understand LoadImage() interface is used in our platform, but if
>>> the error scenario is not expected with LoadImage() interface then we
>>> need further investigation. We are trying to look into it.
>>>
I agree with the assessment made by others that validating using the UEFI
firmware should be a supported configuration if the image is built with
the --disable-shim-lock option.
>>> What can we infer from the change you suggested and that it worked? Do
>>> we need to make certain changes in our platform?
>>
>> The change which I suggested was just a check for my theory. It is not real
>> fix.
>> We have to fix this issue in the GRUB in a different way. This is not your
>> fault.
>> When we have a fix we will ask you for some tests.
>
> Thanks for the information. Sure, will look forward for the change and
> further experiments to perform.
>
Could you please try the following patch? I've not tested it yet but I
think that should make GRUB to support your use case.
From 37157118e237f216866e185e53f8f7d6c9233407 Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas
Date: Thu, 15 Jul 2021 13:08:11 +0200
Subject: [RFC PATCH] 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
Signed-off-by: Javier Martinez Canillas
---
grub-core/kern/efi/sb.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c
index c52ec6226a6..51af1a21546 100644
--- a/grub-core/kern/efi/sb.c
+++ b/grub-core/kern/efi/sb.c
@@ -143,8 +143,17 @@ shim_lock_verifier_write (void *context __attribute__
((unused)), void *buf, gru
{
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)
-return grub_error (GRUB_ERR_ACCESS_DENIED, N_("shim_lock protocol not
found"));
+{
+ 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"));
@@ -166,16 +175,6 @@ grub_shim_lock_verifier_setup (void)
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
RE: UEFI Secureboot not succeeding with Grub 2.06 and later version
Hi Javier,
I tried with your suggested change, but observing Exception as following -
disk/efi/efidisk.c:531: opening hd0 succeeded
partmap/gpt.c:93: Read a valid GPT header
partmap/gpt.c:115: GPT entry 0: start=2048, length=40959
partmap/gpt.c:115: GPT entry 1: start=43008, length=409599
kern/fs.c:56: Detecting ext2...
kern/verifiers.c:88: file: /Image type: 3
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0xcc40 from hd0
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0xcc80 from hd0
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0xccc0 from hd0
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0xcd00 from hd0
.
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0x1dc00 from hd0
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0x1dc40 from hd0
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0x1dc80 from hd0
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0x1dcc0 from hd0
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0x1dd00 from hd0
Synchronous Exception at 0xF92699DC
Synchronous Exception at 0xF92699DC
PC 0xF92699DC
PC 0xF92699A8
PC 0xF92709B8
PC 0xF926D2BC
PC 0x0081FFF64644
PC 0x0081FFF87B84
PC 0x0081FFF86788
PC 0x0081FFF87614
PC 0x0081FFF86788
PC 0x0081FFF87D00
PC 0x0081FFF87D58
PC 0x0081FFF7E334
PC 0x0081FFF7E04C
PC 0x0081FFF7A398
PC 0x0081FFF7A7B4
PC 0x0081FFF7A890
PC 0xF926DE84
PC 0xFE955A78 (0xFE94E000+0x7A78) [ 1] DxeCore.dll
PC 0xFE74EF58 (0xFE747000+0x7F58) [ 2] BdsDxe.dll
PC 0xFE7514A8 (0xFE747000+0xA4A8) [ 2] BdsDxe.dll
PC 0xFE95922C (0xFE94E000+0xB22C) [ 3] DxeCore.dll
Another doubt, should the Image be detected as "UEFI stub kernel", as happened
with experimental suggestion by Daniel?
One minor addition in your patch, added below.
Thanks
>-Original Message-
>From: Javier Martinez Canillas
>Sent: Thursday, July 15, 2021 4:58 PM
>To: Sayanta Pattanayak ; Daniel Kiper
>
>Cc: grub-devel@gnu.org; nd ; x...@ubuntu.com;
>pjo...@redhat.com; l...@nuviainc.com
>Subject: Re: UEFI Secureboot not succeeding with Grub 2.06 and later version
>
>Hello Sayanta,
>
>On 7/15/21 7:26 AM, Sayanta Pattanayak wrote:
>
>[snip]
>
We understand LoadImage() interface is used in our platform, but if
the error scenario is not expected with LoadImage() interface then
we need further investigation. We are trying to look into it.
>
>I agree with the assessment made by others that validating using the UEFI
>firmware should be a supported configuration if the image is built with the --
>disable-shim-lock option.
>
What can we infer from the change you suggested and that it worked?
Do we need to make certain changes in our platform?
>>>
>>> The change which I suggested was just a check for my theory. It is not real
>fix.
>>> We have to fix this issue in the GRUB in a different way. This is not your
>fault.
>>> When we have a fix we will ask you for some tests.
>>
>> Thanks for the information. Sure, will look forward for the change and
>further experiments to perform.
>>
>
>Could you please try the following patch? I've not tested it yet but I think
>that
>should make GRUB to support your use case.
>
>From 37157118e237f216866e185e53f8f7d6c9233407 Mon Sep 17 00:00:00 2001
>From: Javier Martinez Canillas
>Date: Thu, 15 Jul 2021 13:08:11 +0200
>Subject: [RFC PATCH] 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
>Signed-off-by: Javier Martinez Canillas
>---
> grub-core/kern/efi/sb.c | 21 ++---
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
>diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c index
>c52ec6226a6..51af1a21546 100644
>--- a/grub-core/kern/efi/sb.c
>+++ b/grub-core/kern/efi/sb.c
>@@ -143,8 +143,17 @@ shim_lock_verifier_write (void *context
>__attribute__ ((unused)), void *buf, gru {
+ struct grub_module_header *header;
> grub_efi_shim_lock_protocol_t *sl = grub_efi_locate_protocol
>(&shim_lock_guid
Re: UEFI Secureboot not succeeding with Grub 2.06 and later version
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 0xF92699DC
> Synchronous Exception at 0xF92699DC
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
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
Signed-off-by: Javier Martinez Canillas
---
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
[PATCH v2 2/2] linux/hostdisk: use stat() instead of udevadm for partition lookup
From: Jeff Mahoney
sysfs_partition_path() calls udevadm to resolve the sysfs path for
a block device. That can be accomplished by stating the device node
and using the major/minor to follow the symlinks in /sys/dev/block/.
This cuts the execution time of grub-mkconfig to somewhere near 55% on
system without LVM (which uses libdevmapper instead sysfs_partition_path()).
Signed-off-by: Jeff Mahoney
[ pvorel: remove udevadm fallback as it does not help us more than
calling stat() directly; include , update commit
message. ]
Signed-off-by: Petr Vorel
---
changes v1->v2:
* remove udevadm fallback from sysfs_partition_path() as we agreed it
does not bring any advantage over plain stat() call.
* improve commit message
* fix code style (spaces)
grub-core/osdep/linux/hostdisk.c | 52
1 file changed, 6 insertions(+), 46 deletions(-)
diff --git a/grub-core/osdep/linux/hostdisk.c b/grub-core/osdep/linux/hostdisk.c
index da62f924e..d3326d095 100644
--- a/grub-core/osdep/linux/hostdisk.c
+++ b/grub-core/osdep/linux/hostdisk.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#include
#include
@@ -98,54 +99,13 @@ grub_util_get_fd_size_os (grub_util_fd_t fd, const char
*name, unsigned *log_sec
static char *
sysfs_partition_path (const char *dev, const char *entry)
{
- const char *argv[7];
- int fd;
- pid_t pid;
- FILE *udevadm;
- char *buf = NULL;
- size_t len = 0;
- char *path = NULL;
-
- argv[0] = "udevadm";
- argv[1] = "info";
- argv[2] = "--query";
- argv[3] = "path";
- argv[4] = "--name";
- argv[5] = dev;
- argv[6] = NULL;
-
- pid = grub_util_exec_pipe (argv, &fd);
-
- if (!pid)
-return NULL;
-
- /* Parent. Read udevadm's output. */
- udevadm = fdopen (fd, "r");
- if (!udevadm)
-{
- grub_util_warn (_("Unable to open stream from %s: %s"),
- "udevadm", strerror (errno));
- close (fd);
- goto out;
-}
-
- if (getline (&buf, &len, udevadm) > 0)
-{
- char *newline;
-
- newline = strchr (buf, '\n');
- if (newline)
- *newline = '\0';
- path = xasprintf ("/sys%s/%s", buf, entry);
-}
+ struct stat st;
-out:
- if (udevadm)
-fclose (udevadm);
- waitpid (pid, NULL, 0);
- free (buf);
+ if (stat (dev, &st) == 0 && S_ISBLK (st.st_mode))
+return xasprintf ("/sys/dev/block/%u:%u/%s",
+ major (st.st_rdev), minor (st.st_rdev), entry);
- return path;
+ return NULL;
}
static int
--
2.32.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 1/2] osdep: Introduce major.h and use it
to factor out fix for glibc 2.25 introduced in 7a5b301e3 ("build: Use
AC_HEADER_MAJOR to find device macros").
NOTE: Once glibc 2.25 is old enough and this fix is not needed also
AC_HEADER_MAJOR in configure.ac should be removed.
Signed-off-by: Petr Vorel
---
changes v1->v2:
* improve comment in major.h
* adjust comment in configure.ac
* improve commit message
configure.ac | 2 +-
grub-core/osdep/devmapper/getroot.c | 7 +--
grub-core/osdep/devmapper/hostdisk.c | 7 +--
grub-core/osdep/linux/getroot.c | 7 +--
grub-core/osdep/unix/getroot.c | 7 +--
include/grub/osdep/major.h | 30
6 files changed, 35 insertions(+), 25 deletions(-)
create mode 100644 include/grub/osdep/major.h
diff --git a/configure.ac b/configure.ac
index b025e1e84..09ba4ba2b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -424,7 +424,7 @@ AC_CHECK_HEADERS(sys/param.h sys/mount.h sys/mnttab.h
limits.h)
# glibc 2.25 still includes sys/sysmacros.h in sys/types.h but emits
deprecation
# warning which causes compilation failure later with -Werror. So use -Werror
here
-# as well to force proper sys/sysmacros.h detection.
+# as well to force proper sys/sysmacros.h detection. Used in major.h.
SAVED_CFLAGS="$CFLAGS"
CFLAGS="$HOST_CFLAGS -Werror"
AC_HEADER_MAJOR
diff --git a/grub-core/osdep/devmapper/getroot.c
b/grub-core/osdep/devmapper/getroot.c
index a13a39c96..9ba5c9865 100644
--- a/grub-core/osdep/devmapper/getroot.c
+++ b/grub-core/osdep/devmapper/getroot.c
@@ -40,12 +40,7 @@
#include
#endif
-#if defined(MAJOR_IN_MKDEV)
-#include
-#elif defined(MAJOR_IN_SYSMACROS)
-#include
-#endif
-
+#include
#include
#include
diff --git a/grub-core/osdep/devmapper/hostdisk.c
b/grub-core/osdep/devmapper/hostdisk.c
index a8afc0c94..c8053728b 100644
--- a/grub-core/osdep/devmapper/hostdisk.c
+++ b/grub-core/osdep/devmapper/hostdisk.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
@@ -24,12 +25,6 @@
#include
#include
-#if defined(MAJOR_IN_MKDEV)
-#include
-#elif defined(MAJOR_IN_SYSMACROS)
-#include
-#endif
-
#ifdef HAVE_DEVICE_MAPPER
# include
diff --git a/grub-core/osdep/linux/getroot.c b/grub-core/osdep/linux/getroot.c
index 001b818fe..cd588588e 100644
--- a/grub-core/osdep/linux/getroot.c
+++ b/grub-core/osdep/linux/getroot.c
@@ -35,12 +35,7 @@
#include
#endif
-#if defined(MAJOR_IN_MKDEV)
-#include
-#elif defined(MAJOR_IN_SYSMACROS)
-#include
-#endif
-
+#include
#include
#include /* ioctl */
#include
diff --git a/grub-core/osdep/unix/getroot.c b/grub-core/osdep/unix/getroot.c
index 46d7116c6..74f69116d 100644
--- a/grub-core/osdep/unix/getroot.c
+++ b/grub-core/osdep/unix/getroot.c
@@ -51,12 +51,7 @@
#endif /* ! FLOPPY_MAJOR */
#endif
-#include
-#if defined(MAJOR_IN_MKDEV)
-#include
-#elif defined(MAJOR_IN_SYSMACROS)
-#include
-#endif
+#include
#if defined(HAVE_LIBZFS) && defined(HAVE_LIBNVPAIR)
# include
diff --git a/include/grub/osdep/major.h b/include/grub/osdep/major.h
new file mode 100644
index 0..cb468351d
--- /dev/null
+++ b/include/grub/osdep/major.h
@@ -0,0 +1,30 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2021 Free Software Foundation, Inc.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see
[PATCH v2 0/2] linux: use stat instead of udevadm for partition lookup
Hi, changes v1->v2: * remove udevadm fallback from sysfs_partition_path() as we agreed it does not bring any advantage over plain stat() call. * improve comment in major.h * improve commit message * adjust comment in configure.ac * fix code style (spaces) Jeff Mahoney (1): linux/hostdisk: use stat() instead of udevadm for partition lookup Petr Vorel (1): osdep: Introduce major.h and use it configure.ac | 2 +- grub-core/osdep/devmapper/getroot.c | 7 +--- grub-core/osdep/devmapper/hostdisk.c | 7 +--- grub-core/osdep/linux/getroot.c | 7 +--- grub-core/osdep/linux/hostdisk.c | 52 grub-core/osdep/unix/getroot.c | 7 +--- include/grub/osdep/major.h | 30 7 files changed, 41 insertions(+), 71 deletions(-) create mode 100644 include/grub/osdep/major.h -- 2.32.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
RE: UEFI Secureboot not succeeding with Grub 2.06 and later version
Thanks for your quick response.
I did try with the latest change, but still observing "shim_lock protocol not
found" error. For " grub-mkimage", the option " --disable-shim-lock" is used.
disk/efi/efidisk.c:531: opening hd0 succeeded
partmap/gpt.c:93: Read a valid GPT header
partmap/gpt.c:115: GPT entry 0: start=2048, length=40959
partmap/gpt.c:115: GPT entry 1: start=43008, length=409599
kern/fs.c:56: Detecting ext2...
kern/verifiers.c:88: file: /Image type: 3
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0xcc40 from hd0
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0xcc80 from hd0
...
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0x1dcc0 from hd0
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0x1dd00 from hd0
kern/disk.c:295: Closing `hd0'.
disk/efi/efidisk.c:540: closing hd0
error: shim_lock protocol not found.
script/script.c:65: free 0x81fff4a6e0
>-Original Message-
>From: Javier Martinez Canillas
>Sent: Thursday, July 15, 2021 8:43 PM
>To: Sayanta Pattanayak ; Daniel Kiper
>
>Cc: grub-devel@gnu.org; nd ; x...@ubuntu.com;
>pjo...@redhat.com; l...@nuviainc.com
>Subject: Re: UEFI Secureboot not succeeding with Grub 2.06 and later version
>
>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 0xF92699DC Synchronous Exception at
>> 0xF92699DC
>
>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
>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
>Signed-off-by: Javier Martinez Canillas
>---
> 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.
>*/
Re: [PATCH v2 02/22] ieee1275: claim more memory
CC-in a few people who can be interested in this... On Wed, Jun 30, 2021 at 06:40:11PM +1000, Daniel Axtens wrote: > On powerpc-ieee1275, we are running out of memory trying to verify > anything. This is because: > > - we have to load an entire file into memory to verify it. This is >extremely difficult to change with appended signatures. > - We only have 32MB of heap. > - Distro kernels are now often around 30MB. > > So we want to claim more memory from OpenFirmware for our heap. AFAICT it is common problem in the GRUB right now. Please take a look at [1], [2]. It would be nice to find general solution for all. Of course if possible. So, if somebody could take a look at memory management in the GRUB and propose a solution for the problem that would be perfect. Any volunteers? Daniel [1] https://lists.gnu.org/archive/html/grub-devel/2020-06/msg9.html [2] https://lists.gnu.org/archive/html/grub-devel/2021-01/msg00031.html > There are some complications: > > - The grub mm code isn't the only thing that will make claims on >memory from OpenFirmware: > > * PFW/SLOF will have claimed some for their own use. > > * The ieee1275 loader will try to find other bits of memory that we > haven't claimed to place the kernel and initrd when we go to boot. > > * Once we load Linux, it will also try to claim memory. It claims > memory without any reference to /memory/available, it just starts > at min(top of RMO, 768MB) and works down. So we need to avoid this > area. See arch/powerpc/kernel/prom_init.c as of v5.11. > > - The smallest amount of memory a ppc64 KVM guest can have is 256MB. >It doesn't work with distro kernels but can work with custom kernels. >We should maintain support for that. (ppc32 can boot with even less, >and we shouldn't break that either.) > > - Even if a VM has more memory, the memory OpenFirmware makes available >as Real Memory Area can be restricted. A freshly created LPAR on a >PowerVM machine is likely to have only 256MB available to OpenFirmware >even if it has many gigabytes of memory allocated. > > EFI systems will attempt to allocate 1/4th of the available memory, > clamped to between 1M and 1600M. That seems like a good sort of > approach, we just need to figure out if 1/4 is the right fraction > for us. > > We don't know in advance how big the kernel and initrd are going to be, > which makes figuring out how much memory we can take a bit tricky. > > To figure out how much memory we should leave unused, I looked at: > > - an Ubuntu 20.04.1 ppc64le pseries KVM guest: > vmlinux: ~30MB > initrd: ~50MB > > - a RHEL8.2 ppc64le pseries KVM guest: > vmlinux: ~30MB > initrd: ~30MB > > Ubuntu VMs struggle to boot with just 256MB under SLOF. > RHEL likewise has a higher minimum supported memory figure. > So lets first consider a distro kernel and 512MB of addressible memory. > (This is the default case for anything booting under PFW.) Say we lose > 131MB to PFW (based on some tests). This leaves us 381MB. 1/4 of 381MB > is ~95MB. That should be enough to verify a 30MB vmlinux and should > leave plenty of space to load Linux and the initrd. > > If we consider 256MB of RMA under PFW, we have just 125MB remaining. 1/4 > of that is a smidge under 32MB, which gives us very poor odds of verifying > a distro-sized kernel. However, if we need 80MB just to put the kernel > and initrd in memory, we can't claim any more than 45MB anyway. So 1/4 > will do. We'll come back to this later. > > grub is always built as a 32-bit binary, even if it's loading a ppc64 > kernel. So we can't address memory beyond 4GB. This gives a natural cap > of 1GB for powerpc-ieee1275. > > Also apply this 1/4 approach to i386-ieee1275, but keep the 32MB cap. > > make check still works for both i386 and powerpc and I've booted > powerpc grub with this change under SLOF and PFW. > > Signed-off-by: Daniel Axtens > --- > docs/grub-dev.texi | 6 ++- > grub-core/kern/ieee1275/init.c | 81 +++--- > 2 files changed, 69 insertions(+), 18 deletions(-) > > diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi > index 6c629a23e2dc..c11f1ac46de2 100644 > --- a/docs/grub-dev.texi > +++ b/docs/grub-dev.texi > @@ -1047,7 +1047,9 @@ space is limited to 4GiB. GRUB allocates pages from EFI > for its heap, at most > 1.6 GiB. > > On i386-ieee1275 and powerpc-ieee1275 GRUB uses same stack as IEEE1275. > -It allocates at most 32MiB for its heap. > + > +On i386-ieee1275, GRUB allocates at most 32MiB for its heap. On > +powerpc-ieee1275, GRUB allocates up to 1GiB. > > On sparc64-ieee1275 stack is 256KiB and heap is 2MiB. > > @@ -1075,7 +1077,7 @@ In short: > @item i386-qemu @tab 60 KiB @tab < 4 GiB > @item *-efi @tab ? @tab < 1.6 GiB > @item i386-ieee1275 @tab ? @tab < 32 MiB > -@item powerpc-ieee1275@tab ? @tab < 32 MiB > +@item powerpc-ieee
Re: [PATCH v2 02/22] ieee1275: claim more memory
On Thu, Jul 15, 2021 at 11:51:04PM +0200, Daniel Kiper wrote: > CC-in a few people who can be interested in this... > > On Wed, Jun 30, 2021 at 06:40:11PM +1000, Daniel Axtens wrote: > > On powerpc-ieee1275, we are running out of memory trying to verify > > anything. This is because: > > > > - we have to load an entire file into memory to verify it. This is > >extremely difficult to change with appended signatures. > > - We only have 32MB of heap. > > - Distro kernels are now often around 30MB. > > > > So we want to claim more memory from OpenFirmware for our heap. > > AFAICT it is common problem in the GRUB right now. Please take a look at > [1], [2]. It would be nice to find general solution for all. Of course > if possible. So, if somebody could take a look at memory management in > the GRUB and propose a solution for the problem that would be perfect. > Any volunteers? > > Daniel > > [1] https://lists.gnu.org/archive/html/grub-devel/2020-06/msg9.html > [2] https://lists.gnu.org/archive/html/grub-devel/2021-01/msg00031.html I think that my [1] should solve the issue generically. Instead of bumping any of the static limits we have in place, we just drop them altogether in favor of dynamically requesting additional EFI regions whenever we realize that the currently mapped regions cannot satisfy our needs. Like this, we can lower the initially requested regions, but scale them to the specific needs if need be. I had planned to revisit this patch series much earlier, but somehow I didn't yet find the time. Any comments on my approach would be welcome though, and if we agree that this may be a viable route to go down then I'd be happy to further pursue it. Patrick signature.asc Description: PGP signature ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
