* [PATCH 1/9] i386: Don't include <grub/cpu/linux.h> in coreboot and ieee1275 startup.S
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 ` 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
` (8 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2020-12-03 15:01 UTC (permalink / raw)
To: grub-devel
Cc: Ignat Korchagin, Michael Chang, Peter Jones, Marco A Benatto,
Leif Lindholm, Daniel Kiper, Javier Martinez Canillas
Nothing defined in the header file is used in the assembly code but it
may lead to build errors if some headers are included through this and
contains definitions that are not recognized by the assembler, e.g.:
../include/grub/types.h: Assembler messages:
../include/grub/types.h:76: Error: no such instruction: `typedef signed char grub_int8_t'
../include/grub/types.h:77: Error: no such instruction: `typedef short grub_int16_t'
../include/grub/types.h:78: Error: no such instruction: `typedef int grub_int32_t'
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
grub-core/kern/i386/coreboot/startup.S | 1 -
grub-core/kern/i386/ieee1275/startup.S | 1 -
2 files changed, 2 deletions(-)
diff --git a/grub-core/kern/i386/coreboot/startup.S b/grub-core/kern/i386/coreboot/startup.S
index c8486548de0..df6adbabb5b 100644
--- a/grub-core/kern/i386/coreboot/startup.S
+++ b/grub-core/kern/i386/coreboot/startup.S
@@ -18,7 +18,6 @@
#include <grub/symbol.h>
#include <grub/machine/memory.h>
-#include <grub/cpu/linux.h>
#include <grub/offsets.h>
#include <multiboot.h>
#include <multiboot2.h>
diff --git a/grub-core/kern/i386/ieee1275/startup.S b/grub-core/kern/i386/ieee1275/startup.S
index 245583bdb71..62cf348e014 100644
--- a/grub-core/kern/i386/ieee1275/startup.S
+++ b/grub-core/kern/i386/ieee1275/startup.S
@@ -18,7 +18,6 @@
#include <grub/symbol.h>
#include <grub/offsets.h>
-#include <grub/cpu/linux.h>
#include <multiboot.h>
#include <multiboot2.h>
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 2/9] include/grub/i386/linux.h: Include missing <grub/types.h> header
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 ` Javier Martinez Canillas
2020-12-03 15:01 ` [PATCH 3/9] arm/term: Fix linking error due multiple ps2_state definitions Javier Martinez Canillas
` (7 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2020-12-03 15:01 UTC (permalink / raw)
To: grub-devel
Cc: Ignat Korchagin, Michael Chang, Peter Jones, Marco A Benatto,
Leif Lindholm, Daniel Kiper, Javier Martinez Canillas
This header uses types defined in <grub/types.h> but does not include it,
which leads to compile errors like the following:
In file included from ../include/grub/cpu/linux.h:19,
from kern/efi/sb.c:21:
../include/grub/i386/linux.h:80:3: error: unknown type name ‘grub_uint64_t’
80 | grub_uint64_t addr;
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
include/grub/i386/linux.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/grub/i386/linux.h b/include/grub/i386/linux.h
index ce30e7fb01b..6da5f030fd1 100644
--- a/include/grub/i386/linux.h
+++ b/include/grub/i386/linux.h
@@ -19,6 +19,8 @@
#ifndef GRUB_I386_LINUX_HEADER
#define GRUB_I386_LINUX_HEADER 1
+#include <grub/types.h>
+
#define GRUB_LINUX_I386_MAGIC_SIGNATURE 0x53726448 /* "HdrS" */
#define GRUB_LINUX_DEFAULT_SETUP_SECTS 4
#define GRUB_LINUX_INITRD_MAX_ADDRESS 0x37FFFFFF
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 3/9] arm/term: Fix linking error due multiple ps2_state definitions
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 ` Javier Martinez Canillas
2020-12-03 15:01 ` [PATCH 4/9] efi: Make shim_lock GUID and protocol type public Javier Martinez Canillas
` (6 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2020-12-03 15:01 UTC (permalink / raw)
To: grub-devel
Cc: Ignat Korchagin, Michael Chang, Peter Jones, Marco A Benatto,
Leif Lindholm, Daniel Kiper, Javier Martinez Canillas
When building with --target=arm-linux-gnu --with-platform=coreboot
a linking error occurs caused by multiple definitions of the
ps2_state variable.
Mark them as static since they aren't used outside their compilation unit.
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
grub-core/term/arm/cros.c | 2 +-
grub-core/term/arm/pl050.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/grub-core/term/arm/cros.c b/grub-core/term/arm/cros.c
index 1ff9f8ccfb8..a17e49c325c 100644
--- a/grub-core/term/arm/cros.c
+++ b/grub-core/term/arm/cros.c
@@ -30,7 +30,7 @@
#include <grub/fdtbus.h>
#include <grub/arm/cros_ec.h>
-struct grub_ps2_state ps2_state;
+static struct grub_ps2_state ps2_state;
struct grub_cros_ec_keyscan old_scan;
diff --git a/grub-core/term/arm/pl050.c b/grub-core/term/arm/pl050.c
index e4cda305666..b082243b032 100644
--- a/grub-core/term/arm/pl050.c
+++ b/grub-core/term/arm/pl050.c
@@ -29,7 +29,7 @@
static volatile grub_uint32_t *pl050_regs;
-struct grub_ps2_state ps2_state;
+static struct grub_ps2_state ps2_state;
static void
keyboard_controller_wait_until_ready (void)
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 4/9] efi: Make shim_lock GUID and protocol type public
2020-12-03 15:01 [PATCH 0/9] Add UEFI Secure Boot detection and report the status to Linux Javier Martinez Canillas
` (2 preceding siblings ...)
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 ` 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
` (5 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2020-12-03 15:01 UTC (permalink / raw)
To: grub-devel
Cc: Ignat Korchagin, Michael Chang, Peter Jones, Marco A Benatto,
Leif Lindholm, Daniel Kiper, Javier Martinez Canillas
From: Daniel Kiper <daniel.kiper@oracle.com>
The GUID will be used to properly detect and report UEFI Secure Boot
status to the x86 Linux kernel. The functionality will be added by
subsequent patches. The shim_lock protocol type is made public for
completeness.
Additionally, fix formatting of four preceding GUIDs.
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Marco A Benatto <mbenatto@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
grub-core/commands/efi/shim_lock.c | 12 ------------
include/grub/efi/api.h | 19 +++++++++++++++----
2 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/grub-core/commands/efi/shim_lock.c b/grub-core/commands/efi/shim_lock.c
index 764098cfc83..d8f52d721c3 100644
--- a/grub-core/commands/efi/shim_lock.c
+++ b/grub-core/commands/efi/shim_lock.c
@@ -27,18 +27,6 @@
GRUB_MOD_LICENSE ("GPLv3+");
-#define GRUB_EFI_SHIM_LOCK_GUID \
- { 0x605dab50, 0xe046, 0x4300, \
- { 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23 } \
- }
-
-struct grub_efi_shim_lock_protocol
-{
- grub_efi_status_t
- (*verify) (void *buffer, grub_uint32_t size);
-};
-typedef struct grub_efi_shim_lock_protocol grub_efi_shim_lock_protocol_t;
-
static grub_efi_guid_t shim_lock_guid = GRUB_EFI_SHIM_LOCK_GUID;
static grub_efi_shim_lock_protocol_t *sl;
diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
index 1dcaa12f59e..39733585b58 100644
--- a/include/grub/efi/api.h
+++ b/include/grub/efi/api.h
@@ -321,22 +321,27 @@
#define GRUB_EFI_SAL_TABLE_GUID \
{ 0xeb9d2d32, 0x2d88, 0x11d3, \
- { 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d } \
+ { 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d } \
}
#define GRUB_EFI_HCDP_TABLE_GUID \
{ 0xf951938d, 0x620b, 0x42ef, \
- { 0x82, 0x79, 0xa8, 0x4b, 0x79, 0x61, 0x78, 0x98 } \
+ { 0x82, 0x79, 0xa8, 0x4b, 0x79, 0x61, 0x78, 0x98 } \
}
#define GRUB_EFI_DEVICE_TREE_GUID \
{ 0xb1b621d5, 0xf19c, 0x41a5, \
- { 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0 } \
+ { 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0 } \
}
#define GRUB_EFI_VENDOR_APPLE_GUID \
{ 0x2B0585EB, 0xD8B8, 0x49A9, \
- { 0x8B, 0x8C, 0xE2, 0x1B, 0x01, 0xAE, 0xF2, 0xB7 } \
+ { 0x8B, 0x8C, 0xE2, 0x1B, 0x01, 0xAE, 0xF2, 0xB7 } \
+ }
+
+#define GRUB_EFI_SHIM_LOCK_GUID \
+ { 0x605dab50, 0xe046, 0x4300, \
+ { 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23 } \
}
struct grub_efi_sal_system_table
@@ -1694,6 +1699,12 @@ struct grub_efi_block_io
};
typedef struct grub_efi_block_io grub_efi_block_io_t;
+struct grub_efi_shim_lock_protocol
+{
+ grub_efi_status_t (*verify) (void *buffer, grub_uint32_t size);
+};
+typedef struct grub_efi_shim_lock_protocol grub_efi_shim_lock_protocol_t;
+
#if (GRUB_TARGET_SIZEOF_VOID_P == 4) || defined (__ia64__) \
|| defined (__aarch64__) || defined (__MINGW64__) || defined (__CYGWIN__) \
|| defined(__riscv)
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 5/9] efi: Return grub_efi_status_t from grub_efi_get_variable()
2020-12-03 15:01 [PATCH 0/9] Add UEFI Secure Boot detection and report the status to Linux Javier Martinez Canillas
` (3 preceding siblings ...)
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 ` Javier Martinez Canillas
2020-12-03 15:01 ` [PATCH 6/9] efi: Add a function to read EFI variables with attributes Javier Martinez Canillas
` (4 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2020-12-03 15:01 UTC (permalink / raw)
To: grub-devel
Cc: Ignat Korchagin, Michael Chang, Peter Jones, Marco A Benatto,
Leif Lindholm, Daniel Kiper, Javier Martinez Canillas
From: Daniel Kiper <daniel.kiper@oracle.com>
This is needed to properly detect and report UEFI Secure Boot status
to the x86 Linux kernel. The functionality will be added by subsequent
patches.
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Marco A Benatto <mbenatto@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
grub-core/commands/efi/efifwsetup.c | 8 ++++----
grub-core/kern/efi/efi.c | 16 +++++++++-------
grub-core/video/efi_gop.c | 2 +-
include/grub/efi/efi.h | 7 ++++---
4 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/grub-core/commands/efi/efifwsetup.c b/grub-core/commands/efi/efifwsetup.c
index 7a137a72a2f..eaca0328388 100644
--- a/grub-core/commands/efi/efifwsetup.c
+++ b/grub-core/commands/efi/efifwsetup.c
@@ -38,8 +38,8 @@ grub_cmd_fwsetup (grub_command_t cmd __attribute__ ((unused)),
grub_size_t oi_size;
grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
- old_os_indications = grub_efi_get_variable ("OsIndications", &global,
- &oi_size);
+ grub_efi_get_variable ("OsIndications", &global, &oi_size,
+ (void **) &old_os_indications);
if (old_os_indications != NULL && oi_size == sizeof (os_indications))
os_indications |= *old_os_indications;
@@ -63,8 +63,8 @@ efifwsetup_is_supported (void)
grub_size_t oi_size = 0;
grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
- os_indications_supported = grub_efi_get_variable ("OsIndicationsSupported",
- &global, &oi_size);
+ grub_efi_get_variable ("OsIndicationsSupported", &global, &oi_size,
+ (void **) &os_indications_supported);
if (!os_indications_supported)
return 0;
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index e0165e74c58..9403b12cd78 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -223,9 +223,9 @@ grub_efi_set_variable(const char *var, const grub_efi_guid_t *guid,
return grub_error (GRUB_ERR_IO, "could not set EFI variable `%s'", var);
}
-void *
+grub_efi_status_t
grub_efi_get_variable (const char *var, const grub_efi_guid_t *guid,
- grub_size_t *datasize_out)
+ grub_size_t *datasize_out, void **data_out)
{
grub_efi_status_t status;
grub_efi_uintn_t datasize = 0;
@@ -234,13 +234,14 @@ grub_efi_get_variable (const char *var, const grub_efi_guid_t *guid,
void *data;
grub_size_t len, len16;
+ *data_out = NULL;
*datasize_out = 0;
len = grub_strlen (var);
len16 = len * GRUB_MAX_UTF16_PER_UTF8;
var16 = grub_calloc (len16 + 1, sizeof (var16[0]));
if (!var16)
- return NULL;
+ return GRUB_EFI_OUT_OF_RESOURCES;
len16 = grub_utf8_to_utf16 (var16, len16, (grub_uint8_t *) var, len, NULL);
var16[len16] = 0;
@@ -251,14 +252,14 @@ grub_efi_get_variable (const char *var, const grub_efi_guid_t *guid,
if (status != GRUB_EFI_BUFFER_TOO_SMALL || !datasize)
{
grub_free (var16);
- return NULL;
+ return status;
}
data = grub_malloc (datasize);
if (!data)
{
grub_free (var16);
- return NULL;
+ return GRUB_EFI_OUT_OF_RESOURCES;
}
status = efi_call_5 (r->get_variable, var16, guid, NULL, &datasize, data);
@@ -266,12 +267,13 @@ grub_efi_get_variable (const char *var, const grub_efi_guid_t *guid,
if (status == GRUB_EFI_SUCCESS)
{
+ *data_out = data;
*datasize_out = datasize;
- return data;
+ return status;
}
grub_free (data);
- return NULL;
+ return status;
}
#pragma GCC diagnostic ignored "-Wcast-align"
diff --git a/grub-core/video/efi_gop.c b/grub-core/video/efi_gop.c
index be446f8d291..7fe0cdabf50 100644
--- a/grub-core/video/efi_gop.c
+++ b/grub-core/video/efi_gop.c
@@ -316,7 +316,7 @@ grub_video_gop_get_edid (struct grub_video_edid_info *edid_info)
char edidname[] = "agp-internal-edid";
grub_size_t datasize;
grub_uint8_t *data;
- data = grub_efi_get_variable (edidname, &efi_var_guid, &datasize);
+ grub_efi_get_variable (edidname, &efi_var_guid, &datasize, (void **) &data);
if (data && datasize > 16)
{
copy_size = datasize - 16;
diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
index e90e00dc431..8b2a0f1f590 100644
--- a/include/grub/efi/efi.h
+++ b/include/grub/efi/efi.h
@@ -74,9 +74,10 @@ grub_err_t EXPORT_FUNC (grub_efi_set_virtual_address_map) (grub_efi_uintn_t memo
grub_efi_uintn_t descriptor_size,
grub_efi_uint32_t descriptor_version,
grub_efi_memory_descriptor_t *virtual_map);
-void *EXPORT_FUNC (grub_efi_get_variable) (const char *variable,
- const grub_efi_guid_t *guid,
- grub_size_t *datasize_out);
+grub_efi_status_t EXPORT_FUNC (grub_efi_get_variable) (const char *variable,
+ const grub_efi_guid_t *guid,
+ grub_size_t *datasize_out,
+ void **data_out);
grub_err_t
EXPORT_FUNC (grub_efi_set_variable) (const char *var,
const grub_efi_guid_t *guid,
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 6/9] efi: Add a function to read EFI variables with attributes
2020-12-03 15:01 [PATCH 0/9] Add UEFI Secure Boot detection and report the status to Linux Javier Martinez Canillas
` (4 preceding siblings ...)
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 ` Javier Martinez Canillas
2020-12-03 15:01 ` [PATCH 7/9] efi: Add secure boot detection Javier Martinez Canillas
` (3 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2020-12-03 15:01 UTC (permalink / raw)
To: grub-devel
Cc: Ignat Korchagin, Michael Chang, Peter Jones, Marco A Benatto,
Leif Lindholm, Daniel Kiper, Javier Martinez Canillas
From: Daniel Kiper <daniel.kiper@oracle.com>
It will be used to properly detect and report UEFI Secure Boot status to
the x86 Linux kernel. The functionality will be added by subsequent patches.
Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Marco A Benatto <mbenatto@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
grub-core/kern/efi/efi.c | 16 +++++++++++++---
include/grub/efi/efi.h | 5 +++++
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index 9403b12cd78..2942b8e355d 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -224,8 +224,11 @@ grub_efi_set_variable(const char *var, const grub_efi_guid_t *guid,
}
grub_efi_status_t
-grub_efi_get_variable (const char *var, const grub_efi_guid_t *guid,
- grub_size_t *datasize_out, void **data_out)
+grub_efi_get_variable_with_attributes (const char *var,
+ const grub_efi_guid_t *guid,
+ grub_size_t *datasize_out,
+ void **data_out,
+ grub_efi_uint32_t *attributes)
{
grub_efi_status_t status;
grub_efi_uintn_t datasize = 0;
@@ -262,7 +265,7 @@ grub_efi_get_variable (const char *var, const grub_efi_guid_t *guid,
return GRUB_EFI_OUT_OF_RESOURCES;
}
- status = efi_call_5 (r->get_variable, var16, guid, NULL, &datasize, data);
+ status = efi_call_5 (r->get_variable, var16, guid, attributes, &datasize, data);
grub_free (var16);
if (status == GRUB_EFI_SUCCESS)
@@ -276,6 +279,13 @@ grub_efi_get_variable (const char *var, const grub_efi_guid_t *guid,
return status;
}
+grub_efi_status_t
+grub_efi_get_variable (const char *var, const grub_efi_guid_t *guid,
+ grub_size_t *datasize_out, void **data_out)
+{
+ return grub_efi_get_variable_with_attributes (var, guid, datasize_out, data_out, NULL);
+}
+
#pragma GCC diagnostic ignored "-Wcast-align"
/* Search the mods section from the PE32/PE32+ image. This code uses
diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
index 8b2a0f1f590..83d958f9945 100644
--- a/include/grub/efi/efi.h
+++ b/include/grub/efi/efi.h
@@ -74,6 +74,11 @@ grub_err_t EXPORT_FUNC (grub_efi_set_virtual_address_map) (grub_efi_uintn_t memo
grub_efi_uintn_t descriptor_size,
grub_efi_uint32_t descriptor_version,
grub_efi_memory_descriptor_t *virtual_map);
+grub_efi_status_t EXPORT_FUNC (grub_efi_get_variable_with_attributes) (const char *variable,
+ const grub_efi_guid_t *guid,
+ grub_size_t *datasize_out,
+ void **data_out,
+ grub_efi_uint32_t *attributes);
grub_efi_status_t EXPORT_FUNC (grub_efi_get_variable) (const char *variable,
const grub_efi_guid_t *guid,
grub_size_t *datasize_out,
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 7/9] efi: Add secure boot detection
2020-12-03 15:01 [PATCH 0/9] Add UEFI Secure Boot detection and report the status to Linux Javier Martinez Canillas
` (5 preceding siblings ...)
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 ` 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
` (2 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2020-12-03 15:01 UTC (permalink / raw)
To: grub-devel
Cc: Ignat Korchagin, Michael Chang, Peter Jones, Marco A Benatto,
Leif Lindholm, Daniel Kiper, Javier Martinez Canillas
From: Daniel Kiper <daniel.kiper@oracle.com>
Introduce grub_efi_get_secureboot() function which returns whether
UEFI Secure Boot is enabled or not on UEFI systems.
Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Marco A Benatto <mbenatto@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
grub-core/Makefile.am | 1 +
grub-core/Makefile.core.def | 1 +
grub-core/kern/efi/sb.c | 109 ++++++++++++++++++++++++++++++++++++
include/grub/efi/sb.h | 40 +++++++++++++
4 files changed, 151 insertions(+)
create mode 100644 grub-core/kern/efi/sb.c
create mode 100644 include/grub/efi/sb.h
diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
index 3ea8e7ff45f..c6ba5b2d763 100644
--- a/grub-core/Makefile.am
+++ b/grub-core/Makefile.am
@@ -71,6 +71,7 @@ KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/command.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/device.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/disk.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/dl.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/sb.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/env.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/env_private.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/err.h
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index b5f47fc41b5..68b9e9f68dc 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -203,6 +203,7 @@ kernel = {
efi = term/efi/console.c;
efi = kern/acpi.c;
efi = kern/efi/acpi.c;
+ efi = kern/efi/sb.c;
i386_coreboot = kern/i386/pc/acpi.c;
i386_multiboot = kern/i386/pc/acpi.c;
i386_coreboot = kern/acpi.c;
diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c
new file mode 100644
index 00000000000..19658d9626d
--- /dev/null
+++ b/grub-core/kern/efi/sb.c
@@ -0,0 +1,109 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2020 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 <http://www.gnu.org/licenses/>.
+ *
+ * UEFI Secure Boot related checkings.
+ */
+
+#include <grub/efi/efi.h>
+#include <grub/efi/pe32.h>
+#include <grub/efi/sb.h>
+#include <grub/err.h>
+#include <grub/i386/linux.h>
+#include <grub/mm.h>
+#include <grub/types.h>
+
+/*
+ * Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with the Linux kernel,
+ * drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
+ */
+grub_uint8_t
+grub_efi_get_secureboot (void)
+{
+ static grub_efi_guid_t efi_variable_guid = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+ static grub_efi_guid_t efi_shim_lock_guid = GRUB_EFI_SHIM_LOCK_GUID;
+ grub_efi_status_t status;
+ grub_efi_uint32_t attr = 0;
+ grub_size_t size = 0;
+ grub_uint8_t *secboot = NULL;
+ grub_uint8_t *setupmode = NULL;
+ grub_uint8_t *moksbstate = NULL;
+ grub_uint8_t secureboot = GRUB_EFI_SECUREBOOT_MODE_UNKNOWN;
+ const char *secureboot_str = "UNKNOWN";
+
+ status = grub_efi_get_variable ("SecureBoot", &efi_variable_guid,
+ &size, (void **) &secboot);
+
+ if (status == GRUB_EFI_NOT_FOUND)
+ {
+ secureboot = GRUB_EFI_SECUREBOOT_MODE_DISABLED;
+ goto out;
+ }
+
+ if (status != GRUB_EFI_SUCCESS)
+ goto out;
+
+ status = grub_efi_get_variable ("SetupMode", &efi_variable_guid,
+ &size, (void **) &setupmode);
+
+ if (status != GRUB_EFI_SUCCESS)
+ goto out;
+
+ if ((*secboot == 0) || (*setupmode == 1))
+ {
+ secureboot = GRUB_EFI_SECUREBOOT_MODE_DISABLED;
+ goto out;
+ }
+
+ /*
+ * See if a user has put the shim into insecure mode. If so, and if the
+ * variable doesn't have the runtime attribute set, we might as well
+ * honor that.
+ */
+ status = grub_efi_get_variable_with_attributes ("MokSBState", &efi_shim_lock_guid,
+ &size, (void **) &moksbstate, &attr);
+
+ /* If it fails, we don't care why. Default to secure. */
+ if (status != GRUB_EFI_SUCCESS)
+ {
+ secureboot = GRUB_EFI_SECUREBOOT_MODE_ENABLED;
+ goto out;
+ }
+
+ if (!(attr & GRUB_EFI_VARIABLE_RUNTIME_ACCESS) && *moksbstate == 1)
+ {
+ secureboot = GRUB_EFI_SECUREBOOT_MODE_DISABLED;
+ goto out;
+ }
+
+ secureboot = GRUB_EFI_SECUREBOOT_MODE_ENABLED;
+
+ out:
+ grub_free (moksbstate);
+ grub_free (setupmode);
+ grub_free (secboot);
+
+ if (secureboot == GRUB_EFI_SECUREBOOT_MODE_DISABLED)
+ secureboot_str = "Disabled";
+ else if (secureboot == GRUB_EFI_SECUREBOOT_MODE_ENABLED)
+ secureboot_str = "Enabled";
+
+ grub_dprintf ("efi", "UEFI Secure Boot state: %s\n", secureboot_str);
+
+ return secureboot;
+}
diff --git a/include/grub/efi/sb.h b/include/grub/efi/sb.h
new file mode 100644
index 00000000000..a33d985e3c4
--- /dev/null
+++ b/include/grub/efi/sb.h
@@ -0,0 +1,40 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2020 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 <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_EFI_SB_H
+#define GRUB_EFI_SB_H 1
+
+#include <grub/types.h>
+#include <grub/dl.h>
+
+#define GRUB_EFI_SECUREBOOT_MODE_UNSET 0
+#define GRUB_EFI_SECUREBOOT_MODE_UNKNOWN 1
+#define GRUB_EFI_SECUREBOOT_MODE_DISABLED 2
+#define GRUB_EFI_SECUREBOOT_MODE_ENABLED 3
+
+#ifdef GRUB_MACHINE_EFI
+extern grub_uint8_t
+EXPORT_FUNC (grub_efi_get_secureboot) (void);
+#else
+static inline grub_uint8_t
+grub_efi_get_secureboot (void)
+{
+ return GRUB_EFI_SECUREBOOT_MODE_UNSET;
+}
+#endif
+#endif /* GRUB_EFI_SB_H */
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 8/9] efi: Only register shim_lock verifier if shim_lock protocol is found and SB enabled
2020-12-03 15:01 [PATCH 0/9] Add UEFI Secure Boot detection and report the status to Linux Javier Martinez Canillas
` (6 preceding siblings ...)
2020-12-03 15:01 ` [PATCH 7/9] efi: Add secure boot detection Javier Martinez Canillas
@ 2020-12-03 15:01 ` Javier Martinez Canillas
2020-12-08 2:20 ` Michael Chang
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
9 siblings, 1 reply; 16+ messages in thread
From: Javier Martinez Canillas @ 2020-12-03 15:01 UTC (permalink / raw)
To: grub-devel
Cc: Ignat Korchagin, Michael Chang, Peter Jones, Marco A Benatto,
Leif Lindholm, Daniel Kiper, Javier Martinez Canillas
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>
Reported-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 8/9] efi: Only register shim_lock verifier if shim_lock protocol is found and SB enabled
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
0 siblings, 1 reply; 16+ messages in thread
From: Michael Chang @ 2020-12-08 2:20 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: grub-devel, Ignat Korchagin, Peter Jones, Marco A Benatto,
Leif Lindholm, Daniel Kiper
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.
Regards,
Michael
> Reported-by: Peter Jones <pjones@redhat.com>
> Signed-off-by: Javier Martinez Canillas <javierm@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
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 8/9] efi: Only register shim_lock verifier if shim_lock protocol is found and SB enabled
2020-12-08 2:20 ` Michael Chang
@ 2020-12-10 16:50 ` Daniel Kiper
2020-12-14 13:50 ` Michael Chang
0 siblings, 1 reply; 16+ messages in thread
From: Daniel Kiper @ 2020-12-10 16:50 UTC (permalink / raw)
To: mchang
Cc: grub-devel, Javier Martinez Canillas, Michael Chang,
Ignat Korchagin, Peter Jones, Marco A Benatto, Leif Lindholm
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?
Daniel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 8/9] efi: Only register shim_lock verifier if shim_lock protocol is found and SB enabled
2020-12-10 16:50 ` Daniel Kiper
@ 2020-12-14 13:50 ` Michael Chang
2020-12-15 12:36 ` Daniel Kiper
0 siblings, 1 reply; 16+ messages in thread
From: Michael Chang @ 2020-12-14 13:50 UTC (permalink / raw)
To: Daniel Kiper
Cc: grub-devel, Javier Martinez Canillas, Ignat Korchagin,
Peter Jones, Marco A Benatto, Leif Lindholm
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
>
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 8/9] efi: Only register shim_lock verifier if shim_lock protocol is found and SB enabled
2020-12-14 13:50 ` Michael Chang
@ 2020-12-15 12:36 ` Daniel Kiper
2020-12-16 9:29 ` Javier Martinez Canillas
0 siblings, 1 reply; 16+ messages in thread
From: Daniel Kiper @ 2020-12-15 12:36 UTC (permalink / raw)
To: mchang
Cc: grub-devel, Javier Martinez Canillas, Marco A Benatto,
Ignat Korchagin, Leif Lindholm
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 8/9] efi: Only register shim_lock verifier if shim_lock protocol is found and SB enabled
2020-12-15 12:36 ` Daniel Kiper
@ 2020-12-16 9:29 ` Javier Martinez Canillas
0 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2020-12-16 9:29 UTC (permalink / raw)
To: The development of GNU GRUB, Daniel Kiper, mchang
Cc: Leif Lindholm, Marco A Benatto, Ignat Korchagin
On 12/15/20 1:36 PM, Daniel Kiper wrote:
[snip]
>
>> 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.
>
Sorry about that. I did build and test the patches but it may be
that I reordered a little bit before posting and introduced this.
Best regards,
--
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 9/9] loader/linux: Report the UEFI Secure Boot status to the Linux kernel
2020-12-03 15:01 [PATCH 0/9] Add UEFI Secure Boot detection and report the status to Linux Javier Martinez Canillas
` (7 preceding siblings ...)
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-03 15:01 ` Javier Martinez Canillas
2020-12-04 12:27 ` [PATCH 0/9] Add UEFI Secure Boot detection and report the status to Linux Daniel Kiper
9 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2020-12-03 15:01 UTC (permalink / raw)
To: grub-devel
Cc: Ignat Korchagin, Michael Chang, Peter Jones, Marco A Benatto,
Leif Lindholm, Daniel Kiper, Javier Martinez Canillas
From: Daniel Kiper <daniel.kiper@oracle.com>
Now that the GRUB has a grub_efi_get_secureboot() function to check the
UEFI Secure Boot status, use it to report that to the Linux kernel.
Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Marco A Benatto <mbenatto@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
grub-core/loader/i386/linux.c | 6 +++++-
include/grub/i386/linux.h | 8 ++++++--
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
index 976af3fae87..d7e68658f43 100644
--- a/grub-core/loader/i386/linux.c
+++ b/grub-core/loader/i386/linux.c
@@ -46,6 +46,7 @@ GRUB_MOD_LICENSE ("GPLv3+");
#ifdef GRUB_MACHINE_EFI
#include <grub/efi/efi.h>
+#include <grub/efi/sb.h>
#define HAS_VGA_TEXT 0
#define DEFAULT_VIDEO_MODE "auto"
#define ACCEPTS_PURE_TEXT 0
@@ -583,6 +584,9 @@ grub_linux_boot (void)
grub_efi_uintn_t efi_desc_size;
grub_size_t efi_mmap_target;
grub_efi_uint32_t efi_desc_version;
+
+ ctx.params->secure_boot = grub_efi_get_secureboot ();
+
err = grub_efi_finish_boot_services (&efi_mmap_size, efi_mmap_buf, NULL,
&efi_desc_size, &efi_desc_version);
if (err)
@@ -794,7 +798,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
linux_params.code32_start = prot_mode_target + lh.code32_start - GRUB_LINUX_BZIMAGE_ADDR;
linux_params.kernel_alignment = (1 << align);
- linux_params.ps_mouse = linux_params.padding10 = 0;
+ linux_params.ps_mouse = linux_params.padding11 = 0;
linux_params.type_of_loader = GRUB_LINUX_BOOT_LOADER_TYPE;
/* These two are used (instead of cmd_line_ptr) by older versions of Linux,
diff --git a/include/grub/i386/linux.h b/include/grub/i386/linux.h
index 6da5f030fd1..eddf9251d9a 100644
--- a/include/grub/i386/linux.h
+++ b/include/grub/i386/linux.h
@@ -277,7 +277,11 @@ struct linux_kernel_params
grub_uint8_t mmap_size; /* 1e8 */
- grub_uint8_t padding9[0x1f1 - 0x1e9];
+ grub_uint8_t padding9[0x1ec - 0x1e9];
+
+ grub_uint8_t secure_boot; /* 1ec */
+
+ grub_uint8_t padding10[0x1f1 - 0x1ed];
/* Linux setup header copy - BEGIN. */
grub_uint8_t setup_sects; /* The size of the setup in sectors */
@@ -288,7 +292,7 @@ struct linux_kernel_params
grub_uint16_t vid_mode; /* Video mode control */
grub_uint16_t root_dev; /* Default root device number */
- grub_uint8_t padding10; /* 1fe */
+ grub_uint8_t padding11; /* 1fe */
grub_uint8_t ps_mouse; /* 1ff */
grub_uint16_t jump; /* Jump instruction */
--
2.28.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 0/9] Add UEFI Secure Boot detection and report the status to Linux
2020-12-03 15:01 [PATCH 0/9] Add UEFI Secure Boot detection and report the status to Linux Javier Martinez Canillas
` (8 preceding siblings ...)
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 ` Daniel Kiper
9 siblings, 0 replies; 16+ messages in thread
From: Daniel Kiper @ 2020-12-04 12:27 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: grub-devel, Ignat Korchagin, Michael Chang, Peter Jones,
Marco A Benatto, Leif Lindholm
On Thu, Dec 03, 2020 at 04:01:41PM +0100, Javier Martinez Canillas wrote:
>
> This patch series adds support for the GRUB to detect the UEFI Secure Boot
> status using the SecureBoot and SetupMode EFI variables. It also reports
> this to Linux by setting the .secure_boot field of struct boot_params.
>
> Besides that, it contains some cleanups and fixes mostly around EFI support.
For all the patches: Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
If there are no objections I will take them next week.
Daniel
^ permalink raw reply [flat|nested] 16+ messages in thread