[PATCH V2 0/3] integrity: Load certs from EFI MOK config table

[PATCH V2 0/3] integrity: Load certs from EFI MOK config table

[Lenny Szubowicz]

Because of system-specific EFI firmware limitations, EFI volatile
variables may not be capable of holding the required contents of
the Machine Owner Key (MOK) certificate store when the certificate
list grows above some size. Therefore, an EFI boot loader may pass
the MOK certs via a EFI configuration table created specifically for
this purpose to avoid this firmware limitation.

An EFI configuration table is a simpler and more robust mechanism
compared to EFI variables and is well suited for one-way passage
of static information from a pre-OS environment to the kernel.

Entries in the MOK variable configuration table are named key/value
pairs. Therefore the shim boot loader can create a MokListRT named
entry in the MOK configuration table that contains exactly the same
data as the MokListRT UEFI variable does or would otherwise contain.
As such, the kernel can load certs from the data in the MokListRT
configuration table entry data in the same way that it loads certs
from the data returned by the EFI GetVariable() runtime call for the
MokListRT variable.

This patch set does not remove the support for loading certs from the
EFI MOK variables into the platform key ring. However, if both the EFI
MOK configuration table and corresponding EFI MOK variables are present,
the MOK table is used as the source of MOK certs.

The contents of the individual named MOK config table entries are
made available to user space as individual sysfs binary files,
which are read-only to root, under:

	/sys/firmware/efi/mok-variables/

This enables an updated mokutil to provide support for:

	mokutil --list-enrolled

such that it can provide accurate information regardless of whether
the MOK configuration table or MOK EFI variables were the source
for certs. Note that all modifications of MOK related state are still
initiated by mokutil via EFI variables.

V2: Incorporate feedback from V1
  Patch 01: efi: Support for MOK variable config table
  - Minor update to change log; no code changes
  Patch 02: integrity: Move import of MokListRT certs to a separate routine
  - Clean up code flow in code moved to load_moklist_certs()
  - Remove some unnecessary initialization of variables
  Patch 03: integrity: Load certs from the EFI MOK config table
  - Update required due to changes in patch 02.
  - Remove unnecessary init of mokvar_entry in load_moklist_certs() 

V1:
  https://lore.kernel.org/lkml/20200826034455.28707-1-lszubowi@redhat.com/

Lenny Szubowicz (3):
  efi: Support for MOK variable config table
  integrity: Move import of MokListRT certs to a separate routine
  integrity: Load certs from the EFI MOK config table

 arch/x86/kernel/setup.c                       |   1 +
 arch/x86/platform/efi/efi.c                   |   3 +
 drivers/firmware/efi/Makefile                 |   1 +
 drivers/firmware/efi/arm-init.c               |   1 +
 drivers/firmware/efi/efi.c                    |   6 +
 drivers/firmware/efi/mokvar-table.c           | 360 ++++++++++++++++++
 include/linux/efi.h                           |  34 ++
 security/integrity/platform_certs/load_uefi.c |  85 ++++-
 8 files changed, 472 insertions(+), 19 deletions(-)
 create mode 100644 drivers/firmware/efi/mokvar-table.c

-- 
2.27.0

[PATCH V2 1/3] efi: Support for MOK variable config table

[Lenny Szubowicz]

Because of system-specific EFI firmware limitations, EFI volatile
variables may not be capable of holding the required contents of
the Machine Owner Key (MOK) certificate store when the certificate
list grows above some size. Therefore, an EFI boot loader may pass
the MOK certs via a EFI configuration table created specifically for
this purpose to avoid this firmware limitation.

An EFI configuration table is a much more primitive mechanism
compared to EFI variables and is well suited for one-way passage
of static information from a pre-OS environment to the kernel.

This patch adds initial kernel support to recognize, parse,
and validate the EFI MOK configuration table, where named
entries contain the same data that would otherwise be provided
in similarly named EFI variables.

Additionally, this patch creates a sysfs binary file for each
EFI MOK configuration table entry found. These files are read-only
to root and are provided for use by user space utilities such as
mokutil.

A subsequent patch will load MOK certs into the trusted platform
key ring using this infrastructure.

Signed-off-by: Lenny Szubowicz <lszubowi@redhat.com>
---
 arch/x86/kernel/setup.c             |   1 +
 arch/x86/platform/efi/efi.c         |   3 +
 drivers/firmware/efi/Makefile       |   1 +
 drivers/firmware/efi/arm-init.c     |   1 +
 drivers/firmware/efi/efi.c          |   6 +
 drivers/firmware/efi/mokvar-table.c | 360 ++++++++++++++++++++++++++++
 include/linux/efi.h                 |  34 +++
 7 files changed, 406 insertions(+)
 create mode 100644 drivers/firmware/efi/mokvar-table.c

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 3511736fbc74..d41be0df72f8 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1077,6 +1077,7 @@ void __init setup_arch(char **cmdline_p)
 	efi_fake_memmap();
 	efi_find_mirror();
 	efi_esrt_init();
+	efi_mokvar_table_init();
 
 	/*
 	 * The EFI specification says that boot service code won't be
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index d37ebe6e70d7..8a26e705cb06 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -90,6 +90,9 @@ static const unsigned long * const efi_tables[] = {
 	&efi.tpm_log,
 	&efi.tpm_final_log,
 	&efi_rng_seed,
+#ifdef CONFIG_LOAD_UEFI_KEYS
+	&efi.mokvar_table,
+#endif
 };
 
 u64 efi_setup;		/* efi setup_data physical address */
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 7a216984552b..03964e2d27c5 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_EFI_DEV_PATH_PARSER)	+= dev-path-parser.o
 obj-$(CONFIG_APPLE_PROPERTIES)		+= apple-properties.o
 obj-$(CONFIG_EFI_RCI2_TABLE)		+= rci2-table.o
 obj-$(CONFIG_EFI_EMBEDDED_FIRMWARE)	+= embedded-firmware.o
+obj-$(CONFIG_LOAD_UEFI_KEYS)		+= mokvar-table.o
 
 fake_map-y				+= fake_mem.o
 fake_map-$(CONFIG_X86)			+= x86_fake_mem.o
diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index 71c445d20258..f55a92ff12c0 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -236,6 +236,7 @@ void __init efi_init(void)
 
 	reserve_regions();
 	efi_esrt_init();
+	efi_mokvar_table_init();
 
 	memblock_reserve(data.phys_map & PAGE_MASK,
 			 PAGE_ALIGN(data.size + (data.phys_map & ~PAGE_MASK)));
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 3aa07c3b5136..3d4daf215e19 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -43,6 +43,9 @@ struct efi __read_mostly efi = {
 	.esrt			= EFI_INVALID_TABLE_ADDR,
 	.tpm_log		= EFI_INVALID_TABLE_ADDR,
 	.tpm_final_log		= EFI_INVALID_TABLE_ADDR,
+#ifdef CONFIG_LOAD_UEFI_KEYS
+	.mokvar_table		= EFI_INVALID_TABLE_ADDR,
+#endif
 };
 EXPORT_SYMBOL(efi);
 
@@ -518,6 +521,9 @@ static const efi_config_table_type_t common_tables[] __initconst = {
 	{EFI_RT_PROPERTIES_TABLE_GUID,		&rt_prop,		"RTPROP"	},
 #ifdef CONFIG_EFI_RCI2_TABLE
 	{DELLEMC_EFI_RCI2_TABLE_GUID,		&rci2_table_phys			},
+#endif
+#ifdef CONFIG_LOAD_UEFI_KEYS
+	{LINUX_EFI_MOK_VARIABLE_TABLE_GUID,	&efi.mokvar_table,	"MOKvar"	},
 #endif
 	{},
 };
diff --git a/drivers/firmware/efi/mokvar-table.c b/drivers/firmware/efi/mokvar-table.c
new file mode 100644
index 000000000000..f12f1710f5d9
--- /dev/null
+++ b/drivers/firmware/efi/mokvar-table.c
@@ -0,0 +1,360 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mokvar-table.c
+ *
+ * Copyright (c) 2020 Red Hat
+ * Author: Lenny Szubowicz <lszubowi@redhat.com>
+ *
+ * This module contains the kernel support for the Linux EFI Machine
+ * Owner Key (MOK) variable configuration table, which is identified by
+ * the LINUX_EFI_MOK_VARIABLE_TABLE_GUID.
+ *
+ * This EFI configuration table provides a more robust alternative to
+ * EFI volatile variables by which an EFI boot loader can pass the
+ * contents of the Machine Owner Key (MOK) certificate stores to the
+ * kernel during boot. If both the EFI MOK config table and corresponding
+ * EFI MOK variables are present, the table should be considered as
+ * more authoritative.
+ *
+ * This module includes code that validates and maps the EFI MOK table,
+ * if it's presence was detected very early in boot.
+ *
+ * Kernel interface routines are provided to walk through all the
+ * entries in the MOK config table or to search for a specific named
+ * entry.
+ *
+ * The contents of the individual named MOK config table entries are
+ * made available to user space via read-only sysfs binary files under:
+ *
+ * /sys/firmware/efi/mok-variables/
+ *
+ */
+#define pr_fmt(fmt) "mokvar: " fmt
+
+#include <linux/capability.h>
+#include <linux/efi.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/kobject.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+
+/*
+ * The LINUX_EFI_MOK_VARIABLE_TABLE_GUID config table is a packed
+ * sequence of struct efi_mokvar_table_entry, one for each named
+ * MOK variable. The sequence is terminated by an entry with a
+ * completely NULL name and 0 data size.
+ *
+ * efi_mokvar_table_size is set to the computed size of the
+ * MOK config table by efi_mokvar_table_init(). This will be
+ * non-zero if and only if the table if present and has been
+ * validated by efi_mokvar_table_init().
+ */
+static size_t efi_mokvar_table_size;
+
+/*
+ * efi_mokvar_table_va is the kernel virtual address at which the
+ * EFI MOK config table has been mapped by efi_mokvar_sysfs_init().
+ */
+static struct efi_mokvar_table_entry *efi_mokvar_table_va;
+
+/*
+ * Each /sys/firmware/efi/mok-variables/ sysfs file is represented by
+ * an instance of struct efi_mokvar_sysfs_attr on efi_mokvar_sysfs_list.
+ * bin_attr.private points to the associated EFI MOK config table entry.
+ *
+ * This list is created during boot and then remains unchanged.
+ * So no sychronization is currently required to walk the list.
+ */
+struct efi_mokvar_sysfs_attr {
+	struct bin_attribute bin_attr;
+	struct list_head node;
+};
+
+static LIST_HEAD(efi_mokvar_sysfs_list);
+static struct kobject *mokvar_kobj;
+
+/*
+ * efi_mokvar_table_init() - Early boot validation of EFI MOK config table
+ *
+ * If present, validate and compute the size of the EFI MOK variable
+ * configuration table. This table may be provided by an EFI boot loader
+ * as an alternative to ordinary EFI variables, due to platform-dependent
+ * limitations. The memory occupied by this table is marked as reserved.
+ *
+ * This routine must be called before efi_free_boot_services() in order
+ * to guarantee that it can mark the table as reserved.
+ *
+ * Implicit inputs:
+ * efi.mokvar_table:	Physical address of EFI MOK variable config table
+ *			or special value that indicates no such table.
+ *
+ * Implicit outputs:
+ * efi_mokvar_table_size: Computed size of EFI MOK variable config table.
+ *			The table is considered present and valid if this
+ *			is non-zero.
+ */
+void __init efi_mokvar_table_init(void)
+{
+	efi_memory_desc_t md;
+	u64 end_pa;
+	void *va = NULL;
+	size_t cur_offset = 0;
+	size_t offset_limit;
+	size_t map_size = 0;
+	size_t map_size_needed = 0;
+	size_t size;
+	struct efi_mokvar_table_entry *mokvar_entry;
+	int err = -EINVAL;
+
+	if (!efi_enabled(EFI_MEMMAP))
+		return;
+
+	if (efi.mokvar_table == EFI_INVALID_TABLE_ADDR)
+		return;
+	/*
+	 * The EFI MOK config table must fit within a single EFI memory
+	 * descriptor range.
+	 */
+	err = efi_mem_desc_lookup(efi.mokvar_table, &md);
+	if (err) {
+		pr_warn("EFI MOKvar config table is not within the EFI memory map\n");
+		return;
+	}
+	end_pa = efi_mem_desc_end(&md);
+	if (efi.mokvar_table >= end_pa) {
+		pr_err("EFI memory descriptor containing MOKvar config table is invalid\n");
+		return;
+	}
+	offset_limit = end_pa - efi.mokvar_table;
+	/*
+	 * Validate the MOK config table. Since there is no table header
+	 * from which we could get the total size of the MOK config table,
+	 * we compute the total size as we validate each variably sized
+	 * entry, remapping as necessary.
+	 */
+	while (cur_offset + sizeof(*mokvar_entry) <= offset_limit) {
+		mokvar_entry = va + cur_offset;
+		map_size_needed = cur_offset + sizeof(*mokvar_entry);
+		if (map_size_needed > map_size) {
+			if (va)
+				early_memunmap(va, map_size);
+			/*
+			 * Map a little more than the fixed size entry
+			 * header, anticipating some data. It's safe to
+			 * do so as long as we stay within current memory
+			 * descriptor.
+			 */
+			map_size = min(map_size_needed + 2*EFI_PAGE_SIZE,
+				       offset_limit);
+			va = early_memremap(efi.mokvar_table, map_size);
+			if (!va) {
+				pr_err("Failed to map EFI MOKvar config table pa=0x%lx, size=%zu.\n",
+				       efi.mokvar_table, map_size);
+				return;
+			}
+			mokvar_entry = va + cur_offset;
+		}
+
+		/* Check for last sentinel entry */
+		if (mokvar_entry->name[0] == '\0') {
+			if (mokvar_entry->data_size != 0)
+				break;
+			err = 0;
+			break;
+		}
+
+		/* Sanity check that the name is null terminated */
+		size = strnlen(mokvar_entry->name,
+			       sizeof(mokvar_entry->name));
+		if (size >= sizeof(mokvar_entry->name))
+			break;
+
+		/* Advance to the next entry */
+		cur_offset = map_size_needed + mokvar_entry->data_size;
+	}
+
+	if (va)
+		early_memunmap(va, map_size);
+	if (err) {
+		pr_err("EFI MOKvar config table is not valid\n");
+		return;
+	}
+	efi_mem_reserve(efi.mokvar_table, map_size_needed);
+	efi_mokvar_table_size = map_size_needed;
+}
+
+/*
+ * efi_mokvar_entry_next() - Get next entry in the EFI MOK config table
+ *
+ * mokvar_entry:	Pointer to current EFI MOK config table entry
+ *			or null. Null indicates get first entry.
+ *			Passed by reference. This is updated to the
+ *			same value as the return value.
+ *
+ * Returns:		Pointer to next EFI MOK config table entry
+ *			or null, if there are no more entries.
+ *			Same value is returned in the mokvar_entry
+ *			parameter.
+ *
+ * This routine depends on the EFI MOK config table being entirely
+ * mapped with it's starting virtual address in efi_mokvar_table_va.
+ */
+struct efi_mokvar_table_entry *efi_mokvar_entry_next(
+			struct efi_mokvar_table_entry **mokvar_entry)
+{
+	struct efi_mokvar_table_entry *mokvar_cur;
+	struct efi_mokvar_table_entry *mokvar_next;
+	size_t size_cur;
+
+	mokvar_cur = *mokvar_entry;
+	*mokvar_entry = NULL;
+
+	if (efi_mokvar_table_va == NULL)
+		return NULL;
+
+	if (mokvar_cur == NULL) {
+		mokvar_next = efi_mokvar_table_va;
+	} else {
+		if (mokvar_cur->name[0] == '\0')
+			return NULL;
+		size_cur = sizeof(*mokvar_cur) + mokvar_cur->data_size;
+		mokvar_next = (void *)mokvar_cur + size_cur;
+	}
+
+	if (mokvar_next->name[0] == '\0')
+		return NULL;
+
+	*mokvar_entry = mokvar_next;
+	return mokvar_next;
+}
+
+/*
+ * efi_mokvar_entry_find() - Find EFI MOK config entry by name
+ *
+ * name:	Name of the entry to look for.
+ *
+ * Returns:	Pointer to EFI MOK config table entry if found;
+ *		null otherwise.
+ *
+ * This routine depends on the EFI MOK config table being entirely
+ * mapped with it's starting virtual address in efi_mokvar_table_va.
+ */
+struct efi_mokvar_table_entry *efi_mokvar_entry_find(const char *name)
+{
+	struct efi_mokvar_table_entry *mokvar_entry = NULL;
+
+	while (efi_mokvar_entry_next(&mokvar_entry)) {
+		if (!strncmp(name, mokvar_entry->name,
+			     sizeof(mokvar_entry->name)))
+			return mokvar_entry;
+	}
+	return NULL;
+}
+
+/*
+ * efi_mokvar_sysfs_read() - sysfs binary file read routine
+ *
+ * Returns:	Count of bytes read.
+ *
+ * Copy EFI MOK config table entry data for this mokvar sysfs binary file
+ * to the supplied buffer, starting at the specified offset into mokvar table
+ * entry data, for the specified count bytes. The copy is limited by the
+ * amount of data in this mokvar config table entry.
+ */
+static ssize_t efi_mokvar_sysfs_read(struct file *file, struct kobject *kobj,
+				 struct bin_attribute *bin_attr, char *buf,
+				 loff_t off, size_t count)
+{
+	struct efi_mokvar_table_entry *mokvar_entry = bin_attr->private;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return 0;
+
+	if (off >= mokvar_entry->data_size)
+		return 0;
+	if (count >  mokvar_entry->data_size - off)
+		count = mokvar_entry->data_size - off;
+
+	memcpy(buf, mokvar_entry->data + off, count);
+	return count;
+}
+
+/*
+ * efi_mokvar_sysfs_init() - Map EFI MOK config table and create sysfs
+ *
+ * Map the EFI MOK variable config table for run-time use by the kernel
+ * and create the sysfs entries in /sys/firmware/efi/mok-variables/
+ *
+ * This routine just returns if a valid EFI MOK variable config table
+ * was not found earlier during boot.
+ *
+ * This routine must be called during a "middle" initcall phase, i.e.
+ * after efi_mokvar_table_init() but before UEFI certs are loaded
+ * during late init.
+ *
+ * Implicit inputs:
+ * efi.mokvar_table:	Physical address of EFI MOK variable config table
+ *			or special value that indicates no such table.
+ *
+ * efi_mokvar_table_size: Computed size of EFI MOK variable config table.
+ *			The table is considered present and valid if this
+ *			is non-zero.
+ *
+ * Implicit outputs:
+ * efi_mokvar_table_va:	Start virtual address of the EFI MOK config table.
+ */
+static int __init efi_mokvar_sysfs_init(void)
+{
+	void *config_va;
+	struct efi_mokvar_table_entry *mokvar_entry = NULL;
+	struct efi_mokvar_sysfs_attr *mokvar_sysfs = NULL;
+	int err = 0;
+
+	if (efi_mokvar_table_size == 0)
+		return -ENOENT;
+
+	config_va = memremap(efi.mokvar_table, efi_mokvar_table_size,
+			     MEMREMAP_WB);
+	if (!config_va) {
+		pr_err("Failed to map EFI MOKvar config table\n");
+		return -ENOMEM;
+	}
+	efi_mokvar_table_va = config_va;
+
+	mokvar_kobj = kobject_create_and_add("mok-variables", efi_kobj);
+	if (!mokvar_kobj) {
+		pr_err("Failed to create EFI mok-variables sysfs entry\n");
+		return -ENOMEM;
+	}
+
+	while (efi_mokvar_entry_next(&mokvar_entry)) {
+		mokvar_sysfs = kzalloc(sizeof(*mokvar_sysfs), GFP_KERNEL);
+		if (!mokvar_sysfs) {
+			err = -ENOMEM;
+			break;
+		}
+
+		sysfs_bin_attr_init(&mokvar_sysfs->bin_attr);
+		mokvar_sysfs->bin_attr.private = mokvar_entry;
+		mokvar_sysfs->bin_attr.attr.name = mokvar_entry->name;
+		mokvar_sysfs->bin_attr.attr.mode = 0400;
+		mokvar_sysfs->bin_attr.size = mokvar_entry->data_size;
+		mokvar_sysfs->bin_attr.read = efi_mokvar_sysfs_read;
+
+		err = sysfs_create_bin_file(mokvar_kobj,
+					   &mokvar_sysfs->bin_attr);
+		if (err)
+			break;
+
+		list_add_tail(&mokvar_sysfs->node, &efi_mokvar_sysfs_list);
+	}
+
+	if (err) {
+		pr_err("Failed to create some EFI mok-variables sysfs entries\n");
+		kfree(mokvar_sysfs);
+	}
+	return err;
+}
+device_initcall(efi_mokvar_sysfs_init);
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 73db1ae04cef..4a2332f146eb 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -357,6 +357,7 @@ void efi_native_runtime_setup(void);
 #define LINUX_EFI_TPM_FINAL_LOG_GUID		EFI_GUID(0x1e2ed096, 0x30e2, 0x4254,  0xbd, 0x89, 0x86, 0x3b, 0xbe, 0xf8, 0x23, 0x25)
 #define LINUX_EFI_MEMRESERVE_TABLE_GUID		EFI_GUID(0x888eb0c6, 0x8ede, 0x4ff5,  0xa8, 0xf0, 0x9a, 0xee, 0x5c, 0xb9, 0x77, 0xc2)
 #define LINUX_EFI_INITRD_MEDIA_GUID		EFI_GUID(0x5568e427, 0x68fc, 0x4f3d,  0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68)
+#define LINUX_EFI_MOK_VARIABLE_TABLE_GUID	EFI_GUID(0xc451ed2b, 0x9694, 0x45d3,  0xba, 0xba, 0xed, 0x9f, 0x89, 0x88, 0xa3, 0x89)
 
 /* OEM GUIDs */
 #define DELLEMC_EFI_RCI2_TABLE_GUID		EFI_GUID(0x2d9f28a2, 0xa886, 0x456a,  0x97, 0xa8, 0xf1, 0x1e, 0xf2, 0x4f, 0xf4, 0x55)
@@ -546,6 +547,7 @@ extern struct efi {
 	unsigned long			esrt;			/* ESRT table */
 	unsigned long			tpm_log;		/* TPM2 Event Log table */
 	unsigned long			tpm_final_log;		/* TPM2 Final Events Log table */
+	unsigned long			mokvar_table;		/* MOK variable config table */
 
 	efi_get_time_t			*get_time;
 	efi_set_time_t			*set_time;
@@ -1252,4 +1254,36 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size);
 
 char *efi_systab_show_arch(char *str);
 
+/*
+ * The LINUX_EFI_MOK_VARIABLE_TABLE_GUID config table can be provided
+ * to the kernel by an EFI boot loader. The table contains a packed
+ * sequence of these entries, one for each named MOK variable.
+ * The sequence is terminated by an entry with a completely NULL
+ * name and 0 data size.
+ */
+struct efi_mokvar_table_entry {
+	char name[256];
+	u64 data_size;
+	u8 data[];
+} __attribute((packed));
+
+#ifdef CONFIG_LOAD_UEFI_KEYS
+extern void __init efi_mokvar_table_init(void);
+extern struct efi_mokvar_table_entry *efi_mokvar_entry_next(
+			struct efi_mokvar_table_entry **mokvar_entry);
+extern struct efi_mokvar_table_entry *efi_mokvar_entry_find(const char *name);
+#else
+static inline void efi_mokvar_table_init(void) { }
+static inline struct efi_mokvar_table_entry *efi_mokvar_entry_next(
+			struct efi_mokvar_table_entry **mokvar_entry)
+{
+	return NULL;
+}
+static inline struct efi_mokvar_table_entry *efi_mokvar_entry_find(
+			const char *name)
+{
+	return NULL;
+}
+#endif
+
 #endif /* _LINUX_EFI_H */
-- 
2.27.0

[PATCH V2 2/3] integrity: Move import of MokListRT certs to a separate routine

[Lenny Szubowicz]

Move the loading of certs from the UEFI MokListRT into a separate
routine to facilitate additional MokList functionality.

There is no visible functional change as a result of this patch.
Although the UEFI dbx certs are now loaded before the MokList certs,
they are loaded onto different key rings. So the order of the keys
on their respective key rings is the same.

Signed-off-by: Lenny Szubowicz <lszubowi@redhat.com>
---
 security/integrity/platform_certs/load_uefi.c | 63 +++++++++++++------
 1 file changed, 44 insertions(+), 19 deletions(-)

diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index 253fb9a7fc98..c1c622b4dc78 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -66,6 +66,43 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
 }
 
 /*
+ * load_moklist_certs() - Load MokList certs
+ *
+ * Load the certs contained in the UEFI MokListRT database into the
+ * platform trusted keyring.
+ *
+ * Return:	Status
+ */
+static int __init load_moklist_certs(void)
+{
+	efi_guid_t mok_var = EFI_SHIM_LOCK_GUID;
+	void *mok;
+	unsigned long moksize;
+	efi_status_t status;
+	int rc;
+
+	/* Get MokListRT. It might not exist, so it isn't an error
+	 * if we can't get it.
+	 */
+	mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status);
+	if (mok) {
+		rc = parse_efi_signature_list("UEFI:MokListRT",
+					      mok, moksize, get_handler_for_db);
+		kfree(mok);
+		if (rc)
+			pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
+		return rc;
+	}
+	if (status == EFI_NOT_FOUND)
+		pr_debug("MokListRT variable wasn't found\n");
+	else
+		pr_info("Couldn't get UEFI MokListRT\n");
+	return 0;
+}
+
+/*
+ * load_uefi_certs() - Load certs from UEFI sources
+ *
  * Load the certs contained in the UEFI databases into the platform trusted
  * keyring and the UEFI blacklisted X.509 cert SHA256 hashes into the blacklist
  * keyring.
@@ -73,17 +110,16 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
 static int __init load_uefi_certs(void)
 {
 	efi_guid_t secure_var = EFI_IMAGE_SECURITY_DATABASE_GUID;
-	efi_guid_t mok_var = EFI_SHIM_LOCK_GUID;
-	void *db = NULL, *dbx = NULL, *mok = NULL;
-	unsigned long dbsize = 0, dbxsize = 0, moksize = 0;
+	void *db = NULL, *dbx = NULL;
+	unsigned long dbsize = 0, dbxsize = 0;
 	efi_status_t status;
 	int rc = 0;
 
 	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
 		return false;
 
-	/* Get db, MokListRT, and dbx.  They might not exist, so it isn't
-	 * an error if we can't get them.
+	/* Get db and dbx.  They might not exist, so it isn't an error
+	 * if we can't get them.
 	 */
 	if (!uefi_check_ignore_db()) {
 		db = get_cert_list(L"db", &secure_var, &dbsize, &status);
@@ -102,20 +138,6 @@ static int __init load_uefi_certs(void)
 		}
 	}
 
-	mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status);
-	if (!mok) {
-		if (status == EFI_NOT_FOUND)
-			pr_debug("MokListRT variable wasn't found\n");
-		else
-			pr_info("Couldn't get UEFI MokListRT\n");
-	} else {
-		rc = parse_efi_signature_list("UEFI:MokListRT",
-					      mok, moksize, get_handler_for_db);
-		if (rc)
-			pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
-		kfree(mok);
-	}
-
 	dbx = get_cert_list(L"dbx", &secure_var, &dbxsize, &status);
 	if (!dbx) {
 		if (status == EFI_NOT_FOUND)
@@ -131,6 +153,9 @@ static int __init load_uefi_certs(void)
 		kfree(dbx);
 	}
 
+	/* Load the MokListRT certs */
+	rc = load_moklist_certs();
+
 	return rc;
 }
 late_initcall(load_uefi_certs);
-- 
2.27.0

[PATCH V2 3/3] integrity: Load certs from the EFI MOK config table

[Lenny Szubowicz]

Because of system-specific EFI firmware limitations, EFI volatile
variables may not be capable of holding the required contents of
the Machine Owner Key (MOK) certificate store when the certificate
list grows above some size. Therefore, an EFI boot loader may pass
the MOK certs via a EFI configuration table created specifically for
this purpose to avoid this firmware limitation.

An EFI configuration table is a much more primitive mechanism
compared to EFI variables and is well suited for one-way passage
of static information from a pre-OS environment to the kernel.

This patch adds the support to load certs from the MokListRT
entry in the MOK variable configuration table, if it's present.
The pre-existing support to load certs from the MokListRT EFI
variable remains and is used if the EFI MOK configuration table
isn't present or can't be successfully used.

Signed-off-by: Lenny Szubowicz <lszubowi@redhat.com>
---
 security/integrity/platform_certs/load_uefi.c | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index c1c622b4dc78..ee4b4c666854 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -71,16 +71,38 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
  * Load the certs contained in the UEFI MokListRT database into the
  * platform trusted keyring.
  *
+ * This routine checks the EFI MOK config table first. If and only if
+ * that fails, this routine uses the MokListRT ordinary UEFI variable.
+ *
  * Return:	Status
  */
 static int __init load_moklist_certs(void)
 {
+	struct efi_mokvar_table_entry *mokvar_entry;
 	efi_guid_t mok_var = EFI_SHIM_LOCK_GUID;
 	void *mok;
 	unsigned long moksize;
 	efi_status_t status;
 	int rc;
 
+	/* First try to load certs from the EFI MOKvar config table.
+	 * It's not an error if the MOKvar config table doesn't exist
+	 * or the MokListRT entry is not found in it.
+	 */
+	mokvar_entry = efi_mokvar_entry_find("MokListRT");
+	if (mokvar_entry) {
+		rc = parse_efi_signature_list("UEFI:MokListRT (MOKvar table)",
+					      mokvar_entry->data,
+					      mokvar_entry->data_size,
+					      get_handler_for_db);
+		/* All done if that worked. */
+		if (!rc)
+			return rc;
+
+		pr_err("Couldn't parse MokListRT signatures from EFI MOKvar config table: %d\n",
+		       rc);
+	}
+
 	/* Get MokListRT. It might not exist, so it isn't an error
 	 * if we can't get it.
 	 */
-- 
2.27.0

Re: [PATCH V2 2/3] integrity: Move import of MokListRT certs to a separate routine

[Ard Biesheuvel]

On Sat, 5 Sep 2020 at 04:31, Lenny Szubowicz <lszubowi@redhat.com> wrote:
>
> Move the loading of certs from the UEFI MokListRT into a separate
> routine to facilitate additional MokList functionality.
>
> There is no visible functional change as a result of this patch.
> Although the UEFI dbx certs are now loaded before the MokList certs,
> they are loaded onto different key rings. So the order of the keys
> on their respective key rings is the same.
>
> Signed-off-by: Lenny Szubowicz <lszubowi@redhat.com>

Why did you drop Mimi's reviewed-by from this patch?

> ---
>  security/integrity/platform_certs/load_uefi.c | 63 +++++++++++++------
>  1 file changed, 44 insertions(+), 19 deletions(-)
>
> diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
> index 253fb9a7fc98..c1c622b4dc78 100644
> --- a/security/integrity/platform_certs/load_uefi.c
> +++ b/security/integrity/platform_certs/load_uefi.c
> @@ -66,6 +66,43 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
>  }
>
>  /*
> + * load_moklist_certs() - Load MokList certs
> + *
> + * Load the certs contained in the UEFI MokListRT database into the
> + * platform trusted keyring.
> + *
> + * Return:     Status
> + */
> +static int __init load_moklist_certs(void)
> +{
> +       efi_guid_t mok_var = EFI_SHIM_LOCK_GUID;
> +       void *mok;
> +       unsigned long moksize;
> +       efi_status_t status;
> +       int rc;
> +
> +       /* Get MokListRT. It might not exist, so it isn't an error
> +        * if we can't get it.
> +        */
> +       mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status);
> +       if (mok) {
> +               rc = parse_efi_signature_list("UEFI:MokListRT",
> +                                             mok, moksize, get_handler_for_db);
> +               kfree(mok);
> +               if (rc)
> +                       pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
> +               return rc;
> +       }
> +       if (status == EFI_NOT_FOUND)
> +               pr_debug("MokListRT variable wasn't found\n");
> +       else
> +               pr_info("Couldn't get UEFI MokListRT\n");
> +       return 0;
> +}
> +
> +/*
> + * load_uefi_certs() - Load certs from UEFI sources
> + *
>   * Load the certs contained in the UEFI databases into the platform trusted
>   * keyring and the UEFI blacklisted X.509 cert SHA256 hashes into the blacklist
>   * keyring.
> @@ -73,17 +110,16 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
>  static int __init load_uefi_certs(void)
>  {
>         efi_guid_t secure_var = EFI_IMAGE_SECURITY_DATABASE_GUID;
> -       efi_guid_t mok_var = EFI_SHIM_LOCK_GUID;
> -       void *db = NULL, *dbx = NULL, *mok = NULL;
> -       unsigned long dbsize = 0, dbxsize = 0, moksize = 0;
> +       void *db = NULL, *dbx = NULL;
> +       unsigned long dbsize = 0, dbxsize = 0;
>         efi_status_t status;
>         int rc = 0;
>
>         if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
>                 return false;
>
> -       /* Get db, MokListRT, and dbx.  They might not exist, so it isn't
> -        * an error if we can't get them.
> +       /* Get db and dbx.  They might not exist, so it isn't an error
> +        * if we can't get them.
>          */
>         if (!uefi_check_ignore_db()) {
>                 db = get_cert_list(L"db", &secure_var, &dbsize, &status);
> @@ -102,20 +138,6 @@ static int __init load_uefi_certs(void)
>                 }
>         }
>
> -       mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status);
> -       if (!mok) {
> -               if (status == EFI_NOT_FOUND)
> -                       pr_debug("MokListRT variable wasn't found\n");
> -               else
> -                       pr_info("Couldn't get UEFI MokListRT\n");
> -       } else {
> -               rc = parse_efi_signature_list("UEFI:MokListRT",
> -                                             mok, moksize, get_handler_for_db);
> -               if (rc)
> -                       pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
> -               kfree(mok);
> -       }
> -
>         dbx = get_cert_list(L"dbx", &secure_var, &dbxsize, &status);
>         if (!dbx) {
>                 if (status == EFI_NOT_FOUND)
> @@ -131,6 +153,9 @@ static int __init load_uefi_certs(void)
>                 kfree(dbx);
>         }
>
> +       /* Load the MokListRT certs */
> +       rc = load_moklist_certs();
> +
>         return rc;
>  }
>  late_initcall(load_uefi_certs);
> --
> 2.27.0
>

Re: [PATCH V2 0/3] integrity: Load certs from EFI MOK config table

[Ard Biesheuvel]

On Sat, 5 Sep 2020 at 04:31, Lenny Szubowicz <lszubowi@redhat.com> wrote:
>
> Because of system-specific EFI firmware limitations, EFI volatile
> variables may not be capable of holding the required contents of
> the Machine Owner Key (MOK) certificate store when the certificate
> list grows above some size. Therefore, an EFI boot loader may pass
> the MOK certs via a EFI configuration table created specifically for
> this purpose to avoid this firmware limitation.
>
> An EFI configuration table is a simpler and more robust mechanism
> compared to EFI variables and is well suited for one-way passage
> of static information from a pre-OS environment to the kernel.
>
> Entries in the MOK variable configuration table are named key/value
> pairs. Therefore the shim boot loader can create a MokListRT named
> entry in the MOK configuration table that contains exactly the same
> data as the MokListRT UEFI variable does or would otherwise contain.
> As such, the kernel can load certs from the data in the MokListRT
> configuration table entry data in the same way that it loads certs
> from the data returned by the EFI GetVariable() runtime call for the
> MokListRT variable.
>
> This patch set does not remove the support for loading certs from the
> EFI MOK variables into the platform key ring. However, if both the EFI
> MOK configuration table and corresponding EFI MOK variables are present,
> the MOK table is used as the source of MOK certs.
>
> The contents of the individual named MOK config table entries are
> made available to user space as individual sysfs binary files,
> which are read-only to root, under:
>
>         /sys/firmware/efi/mok-variables/
>
> This enables an updated mokutil to provide support for:
>
>         mokutil --list-enrolled
>
> such that it can provide accurate information regardless of whether
> the MOK configuration table or MOK EFI variables were the source
> for certs. Note that all modifications of MOK related state are still
> initiated by mokutil via EFI variables.
>
> V2: Incorporate feedback from V1
>   Patch 01: efi: Support for MOK variable config table
>   - Minor update to change log; no code changes
>   Patch 02: integrity: Move import of MokListRT certs to a separate routine
>   - Clean up code flow in code moved to load_moklist_certs()
>   - Remove some unnecessary initialization of variables
>   Patch 03: integrity: Load certs from the EFI MOK config table
>   - Update required due to changes in patch 02.
>   - Remove unnecessary init of mokvar_entry in load_moklist_certs()
>
> V1:
>   https://lore.kernel.org/lkml/20200826034455.28707-1-lszubowi@redhat.com/
>
> Lenny Szubowicz (3):
>   efi: Support for MOK variable config table
>   integrity: Move import of MokListRT certs to a separate routine
>   integrity: Load certs from the EFI MOK config table
>
>  arch/x86/kernel/setup.c                       |   1 +
>  arch/x86/platform/efi/efi.c                   |   3 +
>  drivers/firmware/efi/Makefile                 |   1 +
>  drivers/firmware/efi/arm-init.c               |   1 +
>  drivers/firmware/efi/efi.c                    |   6 +
>  drivers/firmware/efi/mokvar-table.c           | 360 ++++++++++++++++++
>  include/linux/efi.h                           |  34 ++
>  security/integrity/platform_certs/load_uefi.c |  85 ++++-
>  8 files changed, 472 insertions(+), 19 deletions(-)
>  create mode 100644 drivers/firmware/efi/mokvar-table.c
>

Thanks. I have tentatively queued these up in efi/next.

Mimi, please let me know if you have any thoughts on 3/3, and whether
your R-b on 2/3 [v1] implies that you are ok with the series going
through the EFI tree.

-- 
Ard.

Re: [PATCH V2 2/3] integrity: Move import of MokListRT certs to a separate routine

[Lenny Szubowicz]

On 9/11/20 11:02 AM, Ard Biesheuvel wrote:
> On Sat, 5 Sep 2020 at 04:31, Lenny Szubowicz <lszubowi@redhat.com> wrote:
>>
>> Move the loading of certs from the UEFI MokListRT into a separate
>> routine to facilitate additional MokList functionality.
>>
>> There is no visible functional change as a result of this patch.
>> Although the UEFI dbx certs are now loaded before the MokList certs,
>> they are loaded onto different key rings. So the order of the keys
>> on their respective key rings is the same.
>>
>> Signed-off-by: Lenny Szubowicz <lszubowi@redhat.com>
> 
> Why did you drop Mimi's reviewed-by from this patch?

It was not intentional. I was just not aware that I needed to propagate
Mimi Zohar's reviewed-by from V1 of the patch to V2.

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

V2 includes changes in that patch to incorporate suggestions from
Andy Shevchenko. My assumption was that the maintainer would
gather up the reviewed-by and add any signed-off-by as appropriate,
but it sounds like my assumption was incorrect. In retrospect, I
could see that having the maintainer dig through prior versions
of a patch set for prior reviewed-by tags could be burdensome.

Advice on the expected handling of this would be appreciated.

                     -Lenny.

> 
>> ---
>>   security/integrity/platform_certs/load_uefi.c | 63 +++++++++++++------
>>   1 file changed, 44 insertions(+), 19 deletions(-)
>>
>> diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
>> index 253fb9a7fc98..c1c622b4dc78 100644
>> --- a/security/integrity/platform_certs/load_uefi.c
>> +++ b/security/integrity/platform_certs/load_uefi.c
>> @@ -66,6 +66,43 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
>>   }
>>
>>   /*
>> + * load_moklist_certs() - Load MokList certs
>> + *
>> + * Load the certs contained in the UEFI MokListRT database into the
>> + * platform trusted keyring.
>> + *
>> + * Return:     Status
>> + */
>> +static int __init load_moklist_certs(void)
>> +{
>> +       efi_guid_t mok_var = EFI_SHIM_LOCK_GUID;
>> +       void *mok;
>> +       unsigned long moksize;
>> +       efi_status_t status;
>> +       int rc;
>> +
>> +       /* Get MokListRT. It might not exist, so it isn't an error
>> +        * if we can't get it.
>> +        */
>> +       mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status);
>> +       if (mok) {
>> +               rc = parse_efi_signature_list("UEFI:MokListRT",
>> +                                             mok, moksize, get_handler_for_db);
>> +               kfree(mok);
>> +               if (rc)
>> +                       pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
>> +               return rc;
>> +       }
>> +       if (status == EFI_NOT_FOUND)
>> +               pr_debug("MokListRT variable wasn't found\n");
>> +       else
>> +               pr_info("Couldn't get UEFI MokListRT\n");
>> +       return 0;
>> +}
>> +
>> +/*
>> + * load_uefi_certs() - Load certs from UEFI sources
>> + *
>>    * Load the certs contained in the UEFI databases into the platform trusted
>>    * keyring and the UEFI blacklisted X.509 cert SHA256 hashes into the blacklist
>>    * keyring.
>> @@ -73,17 +110,16 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
>>   static int __init load_uefi_certs(void)
>>   {
>>          efi_guid_t secure_var = EFI_IMAGE_SECURITY_DATABASE_GUID;
>> -       efi_guid_t mok_var = EFI_SHIM_LOCK_GUID;
>> -       void *db = NULL, *dbx = NULL, *mok = NULL;
>> -       unsigned long dbsize = 0, dbxsize = 0, moksize = 0;
>> +       void *db = NULL, *dbx = NULL;
>> +       unsigned long dbsize = 0, dbxsize = 0;
>>          efi_status_t status;
>>          int rc = 0;
>>
>>          if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
>>                  return false;
>>
>> -       /* Get db, MokListRT, and dbx.  They might not exist, so it isn't
>> -        * an error if we can't get them.
>> +       /* Get db and dbx.  They might not exist, so it isn't an error
>> +        * if we can't get them.
>>           */
>>          if (!uefi_check_ignore_db()) {
>>                  db = get_cert_list(L"db", &secure_var, &dbsize, &status);
>> @@ -102,20 +138,6 @@ static int __init load_uefi_certs(void)
>>                  }
>>          }
>>
>> -       mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status);
>> -       if (!mok) {
>> -               if (status == EFI_NOT_FOUND)
>> -                       pr_debug("MokListRT variable wasn't found\n");
>> -               else
>> -                       pr_info("Couldn't get UEFI MokListRT\n");
>> -       } else {
>> -               rc = parse_efi_signature_list("UEFI:MokListRT",
>> -                                             mok, moksize, get_handler_for_db);
>> -               if (rc)
>> -                       pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
>> -               kfree(mok);
>> -       }
>> -
>>          dbx = get_cert_list(L"dbx", &secure_var, &dbxsize, &status);
>>          if (!dbx) {
>>                  if (status == EFI_NOT_FOUND)
>> @@ -131,6 +153,9 @@ static int __init load_uefi_certs(void)
>>                  kfree(dbx);
>>          }
>>
>> +       /* Load the MokListRT certs */
>> +       rc = load_moklist_certs();
>> +
>>          return rc;
>>   }
>>   late_initcall(load_uefi_certs);
>> --
>> 2.27.0
>>
> 

Re: [PATCH V2 2/3] integrity: Move import of MokListRT certs to a separate routine

[Mimi Zohar]

On Fri, 2020-09-11 at 11:54 -0400, Lenny Szubowicz wrote:
> On 9/11/20 11:02 AM, Ard Biesheuvel wrote:
> > On Sat, 5 Sep 2020 at 04:31, Lenny Szubowicz <lszubowi@redhat.com> wrote:
> >>
> >> Move the loading of certs from the UEFI MokListRT into a separate
> >> routine to facilitate additional MokList functionality.
> >>
> >> There is no visible functional change as a result of this patch.
> >> Although the UEFI dbx certs are now loaded before the MokList certs,
> >> they are loaded onto different key rings. So the order of the keys
> >> on their respective key rings is the same.
> >>
> >> Signed-off-by: Lenny Szubowicz <lszubowi@redhat.com>
> > 
> > Why did you drop Mimi's reviewed-by from this patch?
> 
> It was not intentional. I was just not aware that I needed to propagate
> Mimi Zohar's reviewed-by from V1 of the patch to V2.
> 
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> 
> V2 includes changes in that patch to incorporate suggestions from
> Andy Shevchenko. My assumption was that the maintainer would
> gather up the reviewed-by and add any signed-off-by as appropriate,
> but it sounds like my assumption was incorrect. In retrospect, I
> could see that having the maintainer dig through prior versions
> of a patch set for prior reviewed-by tags could be burdensome.

As much as possible moving code should be done without making changes,
simpler for code review.   Then as a separate patch you make changes.  
That way you could also have retained my Reviewed-by.

Mimi

> 
> Advice on the expected handling of this would be appreciated.

Re: [PATCH V2 0/3] integrity: Load certs from EFI MOK config table

[Mimi Zohar]

On Fri, 2020-09-11 at 18:17 +0300, Ard Biesheuvel wrote:
> On Sat, 5 Sep 2020 at 04:31, Lenny Szubowicz <lszubowi@redhat.com> wrote:
> >
> > Because of system-specific EFI firmware limitations, EFI volatile
> > variables may not be capable of holding the required contents of
> > the Machine Owner Key (MOK) certificate store when the certificate
> > list grows above some size. Therefore, an EFI boot loader may pass
> > the MOK certs via a EFI configuration table created specifically for
> > this purpose to avoid this firmware limitation.
> >
> > An EFI configuration table is a simpler and more robust mechanism
> > compared to EFI variables and is well suited for one-way passage
> > of static information from a pre-OS environment to the kernel.
> >
> > Entries in the MOK variable configuration table are named key/value
> > pairs. Therefore the shim boot loader can create a MokListRT named
> > entry in the MOK configuration table that contains exactly the same
> > data as the MokListRT UEFI variable does or would otherwise contain.
> > As such, the kernel can load certs from the data in the MokListRT
> > configuration table entry data in the same way that it loads certs
> > from the data returned by the EFI GetVariable() runtime call for the
> > MokListRT variable.
> >
> > This patch set does not remove the support for loading certs from the
> > EFI MOK variables into the platform key ring. However, if both the EFI
> > MOK configuration table and corresponding EFI MOK variables are present,
> > the MOK table is used as the source of MOK certs.
> >
> > The contents of the individual named MOK config table entries are
> > made available to user space as individual sysfs binary files,
> > which are read-only to root, under:
> >
> >         /sys/firmware/efi/mok-variables/
> >
> > This enables an updated mokutil to provide support for:
> >
> >         mokutil --list-enrolled
> >
> > such that it can provide accurate information regardless of whether
> > the MOK configuration table or MOK EFI variables were the source
> > for certs. Note that all modifications of MOK related state are still
> > initiated by mokutil via EFI variables.
> >
> > V2: Incorporate feedback from V1
> >   Patch 01: efi: Support for MOK variable config table
> >   - Minor update to change log; no code changes
> >   Patch 02: integrity: Move import of MokListRT certs to a separate routine
> >   - Clean up code flow in code moved to load_moklist_certs()
> >   - Remove some unnecessary initialization of variables
> >   Patch 03: integrity: Load certs from the EFI MOK config table
> >   - Update required due to changes in patch 02.
> >   - Remove unnecessary init of mokvar_entry in load_moklist_certs()
> >
> > V1:
> >   https://lore.kernel.org/lkml/20200826034455.28707-1-lszubowi@redhat.com/
> >
> > Lenny Szubowicz (3):
> >   efi: Support for MOK variable config table
> >   integrity: Move import of MokListRT certs to a separate routine
> >   integrity: Load certs from the EFI MOK config table
> >
> >  arch/x86/kernel/setup.c                       |   1 +
> >  arch/x86/platform/efi/efi.c                   |   3 +
> >  drivers/firmware/efi/Makefile                 |   1 +
> >  drivers/firmware/efi/arm-init.c               |   1 +
> >  drivers/firmware/efi/efi.c                    |   6 +
> >  drivers/firmware/efi/mokvar-table.c           | 360 ++++++++++++++++++
> >  include/linux/efi.h                           |  34 ++
> >  security/integrity/platform_certs/load_uefi.c |  85 ++++-
> >  8 files changed, 472 insertions(+), 19 deletions(-)
> >  create mode 100644 drivers/firmware/efi/mokvar-table.c
> >
> 
> Thanks. I have tentatively queued these up in efi/next.
> 
> Mimi, please let me know if you have any thoughts on 3/3, and whether
> your R-b on 2/3 [v1] implies that you are ok with the series going
> through the EFI tree.

Yes, Ard, that was the intent.  I haven't reviewed the most recent
version.

Mimi

Re: [PATCH V2 2/3] integrity: Move import of MokListRT certs to a separate routine

[Lenny Szubowicz]

On 9/11/20 11:59 AM, Mimi Zohar wrote:
> On Fri, 2020-09-11 at 11:54 -0400, Lenny Szubowicz wrote:
>> On 9/11/20 11:02 AM, Ard Biesheuvel wrote:
>>> On Sat, 5 Sep 2020 at 04:31, Lenny Szubowicz <lszubowi@redhat.com> wrote:
>>>>
>>>> Move the loading of certs from the UEFI MokListRT into a separate
>>>> routine to facilitate additional MokList functionality.
>>>>
>>>> There is no visible functional change as a result of this patch.
>>>> Although the UEFI dbx certs are now loaded before the MokList certs,
>>>> they are loaded onto different key rings. So the order of the keys
>>>> on their respective key rings is the same.
>>>>
>>>> Signed-off-by: Lenny Szubowicz <lszubowi@redhat.com>
>>>
>>> Why did you drop Mimi's reviewed-by from this patch?
>>
>> It was not intentional. I was just not aware that I needed to propagate
>> Mimi Zohar's reviewed-by from V1 of the patch to V2.
>>
>> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
>>
>> V2 includes changes in that patch to incorporate suggestions from
>> Andy Shevchenko. My assumption was that the maintainer would
>> gather up the reviewed-by and add any signed-off-by as appropriate,
>> but it sounds like my assumption was incorrect. In retrospect, I
>> could see that having the maintainer dig through prior versions
>> of a patch set for prior reviewed-by tags could be burdensome.
> 
> As much as possible moving code should be done without making changes,
> simpler for code review.   Then as a separate patch you make changes.
> That way you could also have retained my Reviewed-by.
> 
> Mimi

If you or Ard think I should, I can do a V3 with:

   Patch V3 01: Unchanged from V2
   Patch V3 02: Goes back to V1 of patch 02 that Mimi reviewed
   Patch V3 03: New. Has Andy's cleanup suggestions separated from patch 02
   Patch V3 04: This would most probably just be the V2 of patch 03 with no changes

                  -Lenny.

> 
>>
>> Advice on the expected handling of this would be appreciated.
> 
> 

Re: [PATCH V2 2/3] integrity: Move import of MokListRT certs to a separate routine

[Ard Biesheuvel]

On Fri, 11 Sep 2020 at 20:18, Lenny Szubowicz <lszubowi@redhat.com> wrote:
>
> On 9/11/20 11:59 AM, Mimi Zohar wrote:
> > On Fri, 2020-09-11 at 11:54 -0400, Lenny Szubowicz wrote:
> >> On 9/11/20 11:02 AM, Ard Biesheuvel wrote:
> >>> On Sat, 5 Sep 2020 at 04:31, Lenny Szubowicz <lszubowi@redhat.com> wrote:
> >>>>
> >>>> Move the loading of certs from the UEFI MokListRT into a separate
> >>>> routine to facilitate additional MokList functionality.
> >>>>
> >>>> There is no visible functional change as a result of this patch.
> >>>> Although the UEFI dbx certs are now loaded before the MokList certs,
> >>>> they are loaded onto different key rings. So the order of the keys
> >>>> on their respective key rings is the same.
> >>>>
> >>>> Signed-off-by: Lenny Szubowicz <lszubowi@redhat.com>
> >>>
> >>> Why did you drop Mimi's reviewed-by from this patch?
> >>
> >> It was not intentional. I was just not aware that I needed to propagate
> >> Mimi Zohar's reviewed-by from V1 of the patch to V2.
> >>
> >> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> >>
> >> V2 includes changes in that patch to incorporate suggestions from
> >> Andy Shevchenko. My assumption was that the maintainer would
> >> gather up the reviewed-by and add any signed-off-by as appropriate,
> >> but it sounds like my assumption was incorrect. In retrospect, I
> >> could see that having the maintainer dig through prior versions
> >> of a patch set for prior reviewed-by tags could be burdensome.
> >
> > As much as possible moving code should be done without making changes,
> > simpler for code review.   Then as a separate patch you make changes.
> > That way you could also have retained my Reviewed-by.
> >
> > Mimi
>
> If you or Ard think I should, I can do a V3 with:
>
>    Patch V3 01: Unchanged from V2
>    Patch V3 02: Goes back to V1 of patch 02 that Mimi reviewed
>    Patch V3 03: New. Has Andy's cleanup suggestions separated from patch 02
>    Patch V3 04: This would most probably just be the V2 of patch 03 with no changes
>

I think we can just merge the patches as they are, with Mimi's R-b carried over.

Re: [PATCH V2 2/3] integrity: Move import of MokListRT certs to a separate routine

[Mimi Zohar]

On Fri, 2020-09-11 at 21:16 +0300, Ard Biesheuvel wrote:
> I think we can just merge the patches as they are, with Mimi's R-b carried over.

Other than the comments beginning on the "/*" line as opposed to the
subsequent line, the updated 2/2 and 3/3 patches look fine.

thanks,

Mimi

Re: [PATCH V2 2/3] integrity: Move import of MokListRT certs to a separate routine

[Lenny Szubowicz]

On 9/11/20 3:08 PM, Mimi Zohar wrote:
> On Fri, 2020-09-11 at 21:16 +0300, Ard Biesheuvel wrote:
>> I think we can just merge the patches as they are, with Mimi's R-b carried over.
> 
> Other than the comments beginning on the "/*" line as opposed to the
> subsequent line, the updated 2/2 and 3/3 patches look fine.
> 
> thanks,
> 
> Mimi
> 

I also prefer the block comment style that you are suggesting. However, I
kept to the style used by the load_uefi.c source file. If checkpatch.pl
considers it acceptable, I deferred to consistency within the source module.

                       -Lenny.

Re: [PATCH V2 1/3] efi: Support for MOK variable config table

[Arvind Sankar]

On Fri, Sep 04, 2020 at 09:31:05PM -0400, Lenny Szubowicz wrote:
> Because of system-specific EFI firmware limitations, EFI volatile
> variables may not be capable of holding the required contents of
> the Machine Owner Key (MOK) certificate store when the certificate
> list grows above some size. Therefore, an EFI boot loader may pass
> the MOK certs via a EFI configuration table created specifically for
> this purpose to avoid this firmware limitation.
> 
> An EFI configuration table is a much more primitive mechanism
> compared to EFI variables and is well suited for one-way passage
> of static information from a pre-OS environment to the kernel.
> 
> This patch adds initial kernel support to recognize, parse,
> and validate the EFI MOK configuration table, where named
> entries contain the same data that would otherwise be provided
> in similarly named EFI variables.
> 
> Additionally, this patch creates a sysfs binary file for each
> EFI MOK configuration table entry found. These files are read-only
> to root and are provided for use by user space utilities such as
> mokutil.
> 
> A subsequent patch will load MOK certs into the trusted platform
> key ring using this infrastructure.
> 
> Signed-off-by: Lenny Szubowicz <lszubowi@redhat.com>
> ---
>  arch/x86/kernel/setup.c             |   1 +
>  arch/x86/platform/efi/efi.c         |   3 +
>  drivers/firmware/efi/Makefile       |   1 +
>  drivers/firmware/efi/arm-init.c     |   1 +
>  drivers/firmware/efi/efi.c          |   6 +
>  drivers/firmware/efi/mokvar-table.c | 360 ++++++++++++++++++++++++++++
>  include/linux/efi.h                 |  34 +++
>  7 files changed, 406 insertions(+)
>  create mode 100644 drivers/firmware/efi/mokvar-table.c
> 
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 3511736fbc74..d41be0df72f8 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1077,6 +1077,7 @@ void __init setup_arch(char **cmdline_p)
>  	efi_fake_memmap();
>  	efi_find_mirror();
>  	efi_esrt_init();
> +	efi_mokvar_table_init();
>  
>  	/*
>  	 * The EFI specification says that boot service code won't be
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index d37ebe6e70d7..8a26e705cb06 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -90,6 +90,9 @@ static const unsigned long * const efi_tables[] = {
>  	&efi.tpm_log,
>  	&efi.tpm_final_log,
>  	&efi_rng_seed,
> +#ifdef CONFIG_LOAD_UEFI_KEYS
> +	&efi.mokvar_table,
> +#endif
>  };
>  
>  u64 efi_setup;		/* efi setup_data physical address */
> diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
> index 7a216984552b..03964e2d27c5 100644
> --- a/drivers/firmware/efi/Makefile
> +++ b/drivers/firmware/efi/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_EFI_DEV_PATH_PARSER)	+= dev-path-parser.o
>  obj-$(CONFIG_APPLE_PROPERTIES)		+= apple-properties.o
>  obj-$(CONFIG_EFI_RCI2_TABLE)		+= rci2-table.o
>  obj-$(CONFIG_EFI_EMBEDDED_FIRMWARE)	+= embedded-firmware.o
> +obj-$(CONFIG_LOAD_UEFI_KEYS)		+= mokvar-table.o
>  
>  fake_map-y				+= fake_mem.o
>  fake_map-$(CONFIG_X86)			+= x86_fake_mem.o
> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> index 71c445d20258..f55a92ff12c0 100644
> --- a/drivers/firmware/efi/arm-init.c
> +++ b/drivers/firmware/efi/arm-init.c
> @@ -236,6 +236,7 @@ void __init efi_init(void)
>  
>  	reserve_regions();
>  	efi_esrt_init();
> +	efi_mokvar_table_init();
>  
>  	memblock_reserve(data.phys_map & PAGE_MASK,
>  			 PAGE_ALIGN(data.size + (data.phys_map & ~PAGE_MASK)));
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 3aa07c3b5136..3d4daf215e19 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -43,6 +43,9 @@ struct efi __read_mostly efi = {
>  	.esrt			= EFI_INVALID_TABLE_ADDR,
>  	.tpm_log		= EFI_INVALID_TABLE_ADDR,
>  	.tpm_final_log		= EFI_INVALID_TABLE_ADDR,
> +#ifdef CONFIG_LOAD_UEFI_KEYS
> +	.mokvar_table		= EFI_INVALID_TABLE_ADDR,
> +#endif
>  };
>  EXPORT_SYMBOL(efi);
>  
> @@ -518,6 +521,9 @@ static const efi_config_table_type_t common_tables[] __initconst = {
>  	{EFI_RT_PROPERTIES_TABLE_GUID,		&rt_prop,		"RTPROP"	},
>  #ifdef CONFIG_EFI_RCI2_TABLE
>  	{DELLEMC_EFI_RCI2_TABLE_GUID,		&rci2_table_phys			},
> +#endif
> +#ifdef CONFIG_LOAD_UEFI_KEYS
> +	{LINUX_EFI_MOK_VARIABLE_TABLE_GUID,	&efi.mokvar_table,	"MOKvar"	},
>  #endif
>  	{},
>  };
> diff --git a/drivers/firmware/efi/mokvar-table.c b/drivers/firmware/efi/mokvar-table.c
> new file mode 100644
> index 000000000000..f12f1710f5d9
> --- /dev/null
> +++ b/drivers/firmware/efi/mokvar-table.c
> @@ -0,0 +1,360 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * mokvar-table.c
> + *
> + * Copyright (c) 2020 Red Hat
> + * Author: Lenny Szubowicz <lszubowi@redhat.com>
> + *
> + * This module contains the kernel support for the Linux EFI Machine
> + * Owner Key (MOK) variable configuration table, which is identified by
> + * the LINUX_EFI_MOK_VARIABLE_TABLE_GUID.
> + *
> + * This EFI configuration table provides a more robust alternative to
> + * EFI volatile variables by which an EFI boot loader can pass the
> + * contents of the Machine Owner Key (MOK) certificate stores to the
> + * kernel during boot. If both the EFI MOK config table and corresponding
> + * EFI MOK variables are present, the table should be considered as
> + * more authoritative.
> + *
> + * This module includes code that validates and maps the EFI MOK table,
> + * if it's presence was detected very early in boot.
> + *
> + * Kernel interface routines are provided to walk through all the
> + * entries in the MOK config table or to search for a specific named
> + * entry.
> + *
> + * The contents of the individual named MOK config table entries are
> + * made available to user space via read-only sysfs binary files under:
> + *
> + * /sys/firmware/efi/mok-variables/
> + *
> + */
> +#define pr_fmt(fmt) "mokvar: " fmt
> +
> +#include <linux/capability.h>
> +#include <linux/efi.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/kobject.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +
> +/*
> + * The LINUX_EFI_MOK_VARIABLE_TABLE_GUID config table is a packed
> + * sequence of struct efi_mokvar_table_entry, one for each named
> + * MOK variable. The sequence is terminated by an entry with a
> + * completely NULL name and 0 data size.
> + *
> + * efi_mokvar_table_size is set to the computed size of the
> + * MOK config table by efi_mokvar_table_init(). This will be
> + * non-zero if and only if the table if present and has been
> + * validated by efi_mokvar_table_init().
> + */
> +static size_t efi_mokvar_table_size;
> +
> +/*
> + * efi_mokvar_table_va is the kernel virtual address at which the
> + * EFI MOK config table has been mapped by efi_mokvar_sysfs_init().
> + */
> +static struct efi_mokvar_table_entry *efi_mokvar_table_va;
> +
> +/*
> + * Each /sys/firmware/efi/mok-variables/ sysfs file is represented by
> + * an instance of struct efi_mokvar_sysfs_attr on efi_mokvar_sysfs_list.
> + * bin_attr.private points to the associated EFI MOK config table entry.
> + *
> + * This list is created during boot and then remains unchanged.
> + * So no sychronization is currently required to walk the list.
> + */
> +struct efi_mokvar_sysfs_attr {
> +	struct bin_attribute bin_attr;
> +	struct list_head node;
> +};
> +
> +static LIST_HEAD(efi_mokvar_sysfs_list);
> +static struct kobject *mokvar_kobj;
> +
> +/*
> + * efi_mokvar_table_init() - Early boot validation of EFI MOK config table
> + *
> + * If present, validate and compute the size of the EFI MOK variable
> + * configuration table. This table may be provided by an EFI boot loader
> + * as an alternative to ordinary EFI variables, due to platform-dependent
> + * limitations. The memory occupied by this table is marked as reserved.
> + *
> + * This routine must be called before efi_free_boot_services() in order
> + * to guarantee that it can mark the table as reserved.
> + *
> + * Implicit inputs:
> + * efi.mokvar_table:	Physical address of EFI MOK variable config table
> + *			or special value that indicates no such table.
> + *
> + * Implicit outputs:
> + * efi_mokvar_table_size: Computed size of EFI MOK variable config table.
> + *			The table is considered present and valid if this
> + *			is non-zero.
> + */
> +void __init efi_mokvar_table_init(void)
> +{
> +	efi_memory_desc_t md;
> +	u64 end_pa;
> +	void *va = NULL;
> +	size_t cur_offset = 0;
> +	size_t offset_limit;
> +	size_t map_size = 0;
> +	size_t map_size_needed = 0;
> +	size_t size;
> +	struct efi_mokvar_table_entry *mokvar_entry;
> +	int err = -EINVAL;
> +
> +	if (!efi_enabled(EFI_MEMMAP))
> +		return;
> +
> +	if (efi.mokvar_table == EFI_INVALID_TABLE_ADDR)
> +		return;
> +	/*
> +	 * The EFI MOK config table must fit within a single EFI memory
> +	 * descriptor range.
> +	 */
> +	err = efi_mem_desc_lookup(efi.mokvar_table, &md);
> +	if (err) {
> +		pr_warn("EFI MOKvar config table is not within the EFI memory map\n");
> +		return;
> +	}
> +	end_pa = efi_mem_desc_end(&md);
> +	if (efi.mokvar_table >= end_pa) {
> +		pr_err("EFI memory descriptor containing MOKvar config table is invalid\n");
> +		return;
> +	}

efi_mem_desc_lookup() can't return success if efi.mokvar_table >= end_pa, 
why check it again?

> +	offset_limit = end_pa - efi.mokvar_table;
> +	/*
> +	 * Validate the MOK config table. Since there is no table header
> +	 * from which we could get the total size of the MOK config table,
> +	 * we compute the total size as we validate each variably sized
> +	 * entry, remapping as necessary.
> +	 */
> +	while (cur_offset + sizeof(*mokvar_entry) <= offset_limit) {
> +		mokvar_entry = va + cur_offset;
> +		map_size_needed = cur_offset + sizeof(*mokvar_entry);
> +		if (map_size_needed > map_size) {
> +			if (va)
> +				early_memunmap(va, map_size);
> +			/*
> +			 * Map a little more than the fixed size entry
> +			 * header, anticipating some data. It's safe to
> +			 * do so as long as we stay within current memory
> +			 * descriptor.
> +			 */
> +			map_size = min(map_size_needed + 2*EFI_PAGE_SIZE,
> +				       offset_limit);
> +			va = early_memremap(efi.mokvar_table, map_size);

Can't we just map the entire region from efi.mokvar_table to end_pa in
one early_memremap call before the loop and avoid all the remapping
logic?

> +			if (!va) {
> +				pr_err("Failed to map EFI MOKvar config table pa=0x%lx, size=%zu.\n",
> +				       efi.mokvar_table, map_size);
> +				return;
> +			}
> +			mokvar_entry = va + cur_offset;
> +		}
> +
> +		/* Check for last sentinel entry */
> +		if (mokvar_entry->name[0] == '\0') {
> +			if (mokvar_entry->data_size != 0)
> +				break;
> +			err = 0;
> +			break;
> +		}
> +
> +		/* Sanity check that the name is null terminated */
> +		size = strnlen(mokvar_entry->name,
> +			       sizeof(mokvar_entry->name));
> +		if (size >= sizeof(mokvar_entry->name))
> +			break;
> +
> +		/* Advance to the next entry */
> +		cur_offset = map_size_needed + mokvar_entry->data_size;
> +	}
> +
> +	if (va)
> +		early_memunmap(va, map_size);
> +	if (err) {
> +		pr_err("EFI MOKvar config table is not valid\n");
> +		return;
> +	}

err will never be non-zero here: it was cleared when the
efi_mem_desc_lookup() was done. I think the initialization of err to
-EINVAL needs to be moved just prior to the loop.

> +	efi_mem_reserve(efi.mokvar_table, map_size_needed);
> +	efi_mokvar_table_size = map_size_needed;
> +}
> +
> +/*
> + * efi_mokvar_entry_next() - Get next entry in the EFI MOK config table
> + *
> + * mokvar_entry:	Pointer to current EFI MOK config table entry
> + *			or null. Null indicates get first entry.
> + *			Passed by reference. This is updated to the
> + *			same value as the return value.
> + *
> + * Returns:		Pointer to next EFI MOK config table entry
> + *			or null, if there are no more entries.
> + *			Same value is returned in the mokvar_entry
> + *			parameter.
> + *
> + * This routine depends on the EFI MOK config table being entirely
> + * mapped with it's starting virtual address in efi_mokvar_table_va.
> + */
> +struct efi_mokvar_table_entry *efi_mokvar_entry_next(
> +			struct efi_mokvar_table_entry **mokvar_entry)
> +{
> +	struct efi_mokvar_table_entry *mokvar_cur;
> +	struct efi_mokvar_table_entry *mokvar_next;
> +	size_t size_cur;
> +
> +	mokvar_cur = *mokvar_entry;
> +	*mokvar_entry = NULL;
> +
> +	if (efi_mokvar_table_va == NULL)
> +		return NULL;
> +
> +	if (mokvar_cur == NULL) {
> +		mokvar_next = efi_mokvar_table_va;
> +	} else {
> +		if (mokvar_cur->name[0] == '\0')
> +			return NULL;
> +		size_cur = sizeof(*mokvar_cur) + mokvar_cur->data_size;
> +		mokvar_next = (void *)mokvar_cur + size_cur;
> +	}
> +
> +	if (mokvar_next->name[0] == '\0')
> +		return NULL;
> +
> +	*mokvar_entry = mokvar_next;
> +	return mokvar_next;
> +}
> +
> +/*
> + * efi_mokvar_entry_find() - Find EFI MOK config entry by name
> + *
> + * name:	Name of the entry to look for.
> + *
> + * Returns:	Pointer to EFI MOK config table entry if found;
> + *		null otherwise.
> + *
> + * This routine depends on the EFI MOK config table being entirely
> + * mapped with it's starting virtual address in efi_mokvar_table_va.
> + */
> +struct efi_mokvar_table_entry *efi_mokvar_entry_find(const char *name)
> +{
> +	struct efi_mokvar_table_entry *mokvar_entry = NULL;
> +
> +	while (efi_mokvar_entry_next(&mokvar_entry)) {
> +		if (!strncmp(name, mokvar_entry->name,
> +			     sizeof(mokvar_entry->name)))
> +			return mokvar_entry;
> +	}
> +	return NULL;
> +}
> +
> +/*
> + * efi_mokvar_sysfs_read() - sysfs binary file read routine
> + *
> + * Returns:	Count of bytes read.
> + *
> + * Copy EFI MOK config table entry data for this mokvar sysfs binary file
> + * to the supplied buffer, starting at the specified offset into mokvar table
> + * entry data, for the specified count bytes. The copy is limited by the
> + * amount of data in this mokvar config table entry.
> + */
> +static ssize_t efi_mokvar_sysfs_read(struct file *file, struct kobject *kobj,
> +				 struct bin_attribute *bin_attr, char *buf,
> +				 loff_t off, size_t count)
> +{
> +	struct efi_mokvar_table_entry *mokvar_entry = bin_attr->private;
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return 0;
> +
> +	if (off >= mokvar_entry->data_size)
> +		return 0;
> +	if (count >  mokvar_entry->data_size - off)
> +		count = mokvar_entry->data_size - off;
> +
> +	memcpy(buf, mokvar_entry->data + off, count);
> +	return count;
> +}
> +
> +/*
> + * efi_mokvar_sysfs_init() - Map EFI MOK config table and create sysfs
> + *
> + * Map the EFI MOK variable config table for run-time use by the kernel
> + * and create the sysfs entries in /sys/firmware/efi/mok-variables/
> + *
> + * This routine just returns if a valid EFI MOK variable config table
> + * was not found earlier during boot.
> + *
> + * This routine must be called during a "middle" initcall phase, i.e.
> + * after efi_mokvar_table_init() but before UEFI certs are loaded
> + * during late init.
> + *
> + * Implicit inputs:
> + * efi.mokvar_table:	Physical address of EFI MOK variable config table
> + *			or special value that indicates no such table.
> + *
> + * efi_mokvar_table_size: Computed size of EFI MOK variable config table.
> + *			The table is considered present and valid if this
> + *			is non-zero.
> + *
> + * Implicit outputs:
> + * efi_mokvar_table_va:	Start virtual address of the EFI MOK config table.
> + */
> +static int __init efi_mokvar_sysfs_init(void)
> +{
> +	void *config_va;
> +	struct efi_mokvar_table_entry *mokvar_entry = NULL;
> +	struct efi_mokvar_sysfs_attr *mokvar_sysfs = NULL;
> +	int err = 0;
> +
> +	if (efi_mokvar_table_size == 0)
> +		return -ENOENT;
> +
> +	config_va = memremap(efi.mokvar_table, efi_mokvar_table_size,
> +			     MEMREMAP_WB);
> +	if (!config_va) {
> +		pr_err("Failed to map EFI MOKvar config table\n");
> +		return -ENOMEM;
> +	}
> +	efi_mokvar_table_va = config_va;
> +
> +	mokvar_kobj = kobject_create_and_add("mok-variables", efi_kobj);
> +	if (!mokvar_kobj) {
> +		pr_err("Failed to create EFI mok-variables sysfs entry\n");
> +		return -ENOMEM;
> +	}
> +
> +	while (efi_mokvar_entry_next(&mokvar_entry)) {
> +		mokvar_sysfs = kzalloc(sizeof(*mokvar_sysfs), GFP_KERNEL);
> +		if (!mokvar_sysfs) {
> +			err = -ENOMEM;
> +			break;
> +		}
> +
> +		sysfs_bin_attr_init(&mokvar_sysfs->bin_attr);
> +		mokvar_sysfs->bin_attr.private = mokvar_entry;
> +		mokvar_sysfs->bin_attr.attr.name = mokvar_entry->name;
> +		mokvar_sysfs->bin_attr.attr.mode = 0400;
> +		mokvar_sysfs->bin_attr.size = mokvar_entry->data_size;
> +		mokvar_sysfs->bin_attr.read = efi_mokvar_sysfs_read;
> +
> +		err = sysfs_create_bin_file(mokvar_kobj,
> +					   &mokvar_sysfs->bin_attr);
> +		if (err)
> +			break;
> +
> +		list_add_tail(&mokvar_sysfs->node, &efi_mokvar_sysfs_list);
> +	}
> +
> +	if (err) {
> +		pr_err("Failed to create some EFI mok-variables sysfs entries\n");
> +		kfree(mokvar_sysfs);
> +	}
> +	return err;
> +}
> +device_initcall(efi_mokvar_sysfs_init);
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 73db1ae04cef..4a2332f146eb 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -357,6 +357,7 @@ void efi_native_runtime_setup(void);
>  #define LINUX_EFI_TPM_FINAL_LOG_GUID		EFI_GUID(0x1e2ed096, 0x30e2, 0x4254,  0xbd, 0x89, 0x86, 0x3b, 0xbe, 0xf8, 0x23, 0x25)
>  #define LINUX_EFI_MEMRESERVE_TABLE_GUID		EFI_GUID(0x888eb0c6, 0x8ede, 0x4ff5,  0xa8, 0xf0, 0x9a, 0xee, 0x5c, 0xb9, 0x77, 0xc2)
>  #define LINUX_EFI_INITRD_MEDIA_GUID		EFI_GUID(0x5568e427, 0x68fc, 0x4f3d,  0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68)
> +#define LINUX_EFI_MOK_VARIABLE_TABLE_GUID	EFI_GUID(0xc451ed2b, 0x9694, 0x45d3,  0xba, 0xba, 0xed, 0x9f, 0x89, 0x88, 0xa3, 0x89)
>  
>  /* OEM GUIDs */
>  #define DELLEMC_EFI_RCI2_TABLE_GUID		EFI_GUID(0x2d9f28a2, 0xa886, 0x456a,  0x97, 0xa8, 0xf1, 0x1e, 0xf2, 0x4f, 0xf4, 0x55)
> @@ -546,6 +547,7 @@ extern struct efi {
>  	unsigned long			esrt;			/* ESRT table */
>  	unsigned long			tpm_log;		/* TPM2 Event Log table */
>  	unsigned long			tpm_final_log;		/* TPM2 Final Events Log table */
> +	unsigned long			mokvar_table;		/* MOK variable config table */
>  
>  	efi_get_time_t			*get_time;
>  	efi_set_time_t			*set_time;
> @@ -1252,4 +1254,36 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size);
>  
>  char *efi_systab_show_arch(char *str);
>  
> +/*
> + * The LINUX_EFI_MOK_VARIABLE_TABLE_GUID config table can be provided
> + * to the kernel by an EFI boot loader. The table contains a packed
> + * sequence of these entries, one for each named MOK variable.
> + * The sequence is terminated by an entry with a completely NULL
> + * name and 0 data size.
> + */
> +struct efi_mokvar_table_entry {
> +	char name[256];
> +	u64 data_size;
> +	u8 data[];
> +} __attribute((packed));
> +
> +#ifdef CONFIG_LOAD_UEFI_KEYS
> +extern void __init efi_mokvar_table_init(void);
> +extern struct efi_mokvar_table_entry *efi_mokvar_entry_next(
> +			struct efi_mokvar_table_entry **mokvar_entry);
> +extern struct efi_mokvar_table_entry *efi_mokvar_entry_find(const char *name);
> +#else
> +static inline void efi_mokvar_table_init(void) { }
> +static inline struct efi_mokvar_table_entry *efi_mokvar_entry_next(
> +			struct efi_mokvar_table_entry **mokvar_entry)
> +{
> +	return NULL;
> +}
> +static inline struct efi_mokvar_table_entry *efi_mokvar_entry_find(
> +			const char *name)
> +{
> +	return NULL;
> +}
> +#endif
> +
>  #endif /* _LINUX_EFI_H */
> -- 
> 2.27.0
> 

Re: [PATCH V2 1/3] efi: Support for MOK variable config table

[Ard Biesheuvel]

On Mon, 21 Sep 2020 at 18:19, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Fri, Sep 04, 2020 at 09:31:05PM -0400, Lenny Szubowicz wrote:
> > Because of system-specific EFI firmware limitations, EFI volatile
> > variables may not be capable of holding the required contents of
> > the Machine Owner Key (MOK) certificate store when the certificate
> > list grows above some size. Therefore, an EFI boot loader may pass
> > the MOK certs via a EFI configuration table created specifically for
> > this purpose to avoid this firmware limitation.
> >
> > An EFI configuration table is a much more primitive mechanism
> > compared to EFI variables and is well suited for one-way passage
> > of static information from a pre-OS environment to the kernel.
> >
> > This patch adds initial kernel support to recognize, parse,
> > and validate the EFI MOK configuration table, where named
> > entries contain the same data that would otherwise be provided
> > in similarly named EFI variables.
> >
> > Additionally, this patch creates a sysfs binary file for each
> > EFI MOK configuration table entry found. These files are read-only
> > to root and are provided for use by user space utilities such as
> > mokutil.
> >
> > A subsequent patch will load MOK certs into the trusted platform
> > key ring using this infrastructure.
> >
> > Signed-off-by: Lenny Szubowicz <lszubowi@redhat.com>
> > ---
> >  arch/x86/kernel/setup.c             |   1 +
> >  arch/x86/platform/efi/efi.c         |   3 +
> >  drivers/firmware/efi/Makefile       |   1 +
> >  drivers/firmware/efi/arm-init.c     |   1 +
> >  drivers/firmware/efi/efi.c          |   6 +
> >  drivers/firmware/efi/mokvar-table.c | 360 ++++++++++++++++++++++++++++
> >  include/linux/efi.h                 |  34 +++
> >  7 files changed, 406 insertions(+)
> >  create mode 100644 drivers/firmware/efi/mokvar-table.c
> >
> > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > index 3511736fbc74..d41be0df72f8 100644
> > --- a/arch/x86/kernel/setup.c
> > +++ b/arch/x86/kernel/setup.c
> > @@ -1077,6 +1077,7 @@ void __init setup_arch(char **cmdline_p)
> >       efi_fake_memmap();
> >       efi_find_mirror();
> >       efi_esrt_init();
> > +     efi_mokvar_table_init();
> >
> >       /*
> >        * The EFI specification says that boot service code won't be
> > diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> > index d37ebe6e70d7..8a26e705cb06 100644
> > --- a/arch/x86/platform/efi/efi.c
> > +++ b/arch/x86/platform/efi/efi.c
> > @@ -90,6 +90,9 @@ static const unsigned long * const efi_tables[] = {
> >       &efi.tpm_log,
> >       &efi.tpm_final_log,
> >       &efi_rng_seed,
> > +#ifdef CONFIG_LOAD_UEFI_KEYS
> > +     &efi.mokvar_table,
> > +#endif
> >  };
> >
> >  u64 efi_setup;               /* efi setup_data physical address */
> > diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
> > index 7a216984552b..03964e2d27c5 100644
> > --- a/drivers/firmware/efi/Makefile
> > +++ b/drivers/firmware/efi/Makefile
> > @@ -28,6 +28,7 @@ obj-$(CONFIG_EFI_DEV_PATH_PARSER)   += dev-path-parser.o
> >  obj-$(CONFIG_APPLE_PROPERTIES)               += apple-properties.o
> >  obj-$(CONFIG_EFI_RCI2_TABLE)         += rci2-table.o
> >  obj-$(CONFIG_EFI_EMBEDDED_FIRMWARE)  += embedded-firmware.o
> > +obj-$(CONFIG_LOAD_UEFI_KEYS)         += mokvar-table.o
> >
> >  fake_map-y                           += fake_mem.o
> >  fake_map-$(CONFIG_X86)                       += x86_fake_mem.o
> > diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> > index 71c445d20258..f55a92ff12c0 100644
> > --- a/drivers/firmware/efi/arm-init.c
> > +++ b/drivers/firmware/efi/arm-init.c
> > @@ -236,6 +236,7 @@ void __init efi_init(void)
> >
> >       reserve_regions();
> >       efi_esrt_init();
> > +     efi_mokvar_table_init();
> >
> >       memblock_reserve(data.phys_map & PAGE_MASK,
> >                        PAGE_ALIGN(data.size + (data.phys_map & ~PAGE_MASK)));
> > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> > index 3aa07c3b5136..3d4daf215e19 100644
> > --- a/drivers/firmware/efi/efi.c
> > +++ b/drivers/firmware/efi/efi.c
> > @@ -43,6 +43,9 @@ struct efi __read_mostly efi = {
> >       .esrt                   = EFI_INVALID_TABLE_ADDR,
> >       .tpm_log                = EFI_INVALID_TABLE_ADDR,
> >       .tpm_final_log          = EFI_INVALID_TABLE_ADDR,
> > +#ifdef CONFIG_LOAD_UEFI_KEYS
> > +     .mokvar_table           = EFI_INVALID_TABLE_ADDR,
> > +#endif
> >  };
> >  EXPORT_SYMBOL(efi);
> >
> > @@ -518,6 +521,9 @@ static const efi_config_table_type_t common_tables[] __initconst = {
> >       {EFI_RT_PROPERTIES_TABLE_GUID,          &rt_prop,               "RTPROP"        },
> >  #ifdef CONFIG_EFI_RCI2_TABLE
> >       {DELLEMC_EFI_RCI2_TABLE_GUID,           &rci2_table_phys                        },
> > +#endif
> > +#ifdef CONFIG_LOAD_UEFI_KEYS
> > +     {LINUX_EFI_MOK_VARIABLE_TABLE_GUID,     &efi.mokvar_table,      "MOKvar"        },
> >  #endif
> >       {},
> >  };
> > diff --git a/drivers/firmware/efi/mokvar-table.c b/drivers/firmware/efi/mokvar-table.c
> > new file mode 100644
> > index 000000000000..f12f1710f5d9
> > --- /dev/null
> > +++ b/drivers/firmware/efi/mokvar-table.c
> > @@ -0,0 +1,360 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * mokvar-table.c
> > + *
> > + * Copyright (c) 2020 Red Hat
> > + * Author: Lenny Szubowicz <lszubowi@redhat.com>
> > + *
> > + * This module contains the kernel support for the Linux EFI Machine
> > + * Owner Key (MOK) variable configuration table, which is identified by
> > + * the LINUX_EFI_MOK_VARIABLE_TABLE_GUID.
> > + *
> > + * This EFI configuration table provides a more robust alternative to
> > + * EFI volatile variables by which an EFI boot loader can pass the
> > + * contents of the Machine Owner Key (MOK) certificate stores to the
> > + * kernel during boot. If both the EFI MOK config table and corresponding
> > + * EFI MOK variables are present, the table should be considered as
> > + * more authoritative.
> > + *
> > + * This module includes code that validates and maps the EFI MOK table,
> > + * if it's presence was detected very early in boot.
> > + *
> > + * Kernel interface routines are provided to walk through all the
> > + * entries in the MOK config table or to search for a specific named
> > + * entry.
> > + *
> > + * The contents of the individual named MOK config table entries are
> > + * made available to user space via read-only sysfs binary files under:
> > + *
> > + * /sys/firmware/efi/mok-variables/
> > + *
> > + */
> > +#define pr_fmt(fmt) "mokvar: " fmt
> > +
> > +#include <linux/capability.h>
> > +#include <linux/efi.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/kobject.h>
> > +#include <linux/list.h>
> > +#include <linux/slab.h>
> > +
> > +/*
> > + * The LINUX_EFI_MOK_VARIABLE_TABLE_GUID config table is a packed
> > + * sequence of struct efi_mokvar_table_entry, one for each named
> > + * MOK variable. The sequence is terminated by an entry with a
> > + * completely NULL name and 0 data size.
> > + *
> > + * efi_mokvar_table_size is set to the computed size of the
> > + * MOK config table by efi_mokvar_table_init(). This will be
> > + * non-zero if and only if the table if present and has been
> > + * validated by efi_mokvar_table_init().
> > + */
> > +static size_t efi_mokvar_table_size;
> > +
> > +/*
> > + * efi_mokvar_table_va is the kernel virtual address at which the
> > + * EFI MOK config table has been mapped by efi_mokvar_sysfs_init().
> > + */
> > +static struct efi_mokvar_table_entry *efi_mokvar_table_va;
> > +
> > +/*
> > + * Each /sys/firmware/efi/mok-variables/ sysfs file is represented by
> > + * an instance of struct efi_mokvar_sysfs_attr on efi_mokvar_sysfs_list.
> > + * bin_attr.private points to the associated EFI MOK config table entry.
> > + *
> > + * This list is created during boot and then remains unchanged.
> > + * So no sychronization is currently required to walk the list.
> > + */
> > +struct efi_mokvar_sysfs_attr {
> > +     struct bin_attribute bin_attr;
> > +     struct list_head node;
> > +};
> > +
> > +static LIST_HEAD(efi_mokvar_sysfs_list);
> > +static struct kobject *mokvar_kobj;
> > +
> > +/*
> > + * efi_mokvar_table_init() - Early boot validation of EFI MOK config table
> > + *
> > + * If present, validate and compute the size of the EFI MOK variable
> > + * configuration table. This table may be provided by an EFI boot loader
> > + * as an alternative to ordinary EFI variables, due to platform-dependent
> > + * limitations. The memory occupied by this table is marked as reserved.
> > + *
> > + * This routine must be called before efi_free_boot_services() in order
> > + * to guarantee that it can mark the table as reserved.
> > + *
> > + * Implicit inputs:
> > + * efi.mokvar_table: Physical address of EFI MOK variable config table
> > + *                   or special value that indicates no such table.
> > + *
> > + * Implicit outputs:
> > + * efi_mokvar_table_size: Computed size of EFI MOK variable config table.
> > + *                   The table is considered present and valid if this
> > + *                   is non-zero.
> > + */
> > +void __init efi_mokvar_table_init(void)
> > +{
> > +     efi_memory_desc_t md;
> > +     u64 end_pa;
> > +     void *va = NULL;
> > +     size_t cur_offset = 0;
> > +     size_t offset_limit;
> > +     size_t map_size = 0;
> > +     size_t map_size_needed = 0;
> > +     size_t size;
> > +     struct efi_mokvar_table_entry *mokvar_entry;
> > +     int err = -EINVAL;
> > +
> > +     if (!efi_enabled(EFI_MEMMAP))
> > +             return;
> > +
> > +     if (efi.mokvar_table == EFI_INVALID_TABLE_ADDR)
> > +             return;
> > +     /*
> > +      * The EFI MOK config table must fit within a single EFI memory
> > +      * descriptor range.
> > +      */
> > +     err = efi_mem_desc_lookup(efi.mokvar_table, &md);
> > +     if (err) {
> > +             pr_warn("EFI MOKvar config table is not within the EFI memory map\n");
> > +             return;
> > +     }
> > +     end_pa = efi_mem_desc_end(&md);
> > +     if (efi.mokvar_table >= end_pa) {
> > +             pr_err("EFI memory descriptor containing MOKvar config table is invalid\n");
> > +             return;
> > +     }
>
> efi_mem_desc_lookup() can't return success if efi.mokvar_table >= end_pa,
> why check it again?
>
> > +     offset_limit = end_pa - efi.mokvar_table;
> > +     /*
> > +      * Validate the MOK config table. Since there is no table header
> > +      * from which we could get the total size of the MOK config table,
> > +      * we compute the total size as we validate each variably sized
> > +      * entry, remapping as necessary.
> > +      */
> > +     while (cur_offset + sizeof(*mokvar_entry) <= offset_limit) {
> > +             mokvar_entry = va + cur_offset;
> > +             map_size_needed = cur_offset + sizeof(*mokvar_entry);
> > +             if (map_size_needed > map_size) {
> > +                     if (va)
> > +                             early_memunmap(va, map_size);
> > +                     /*
> > +                      * Map a little more than the fixed size entry
> > +                      * header, anticipating some data. It's safe to
> > +                      * do so as long as we stay within current memory
> > +                      * descriptor.
> > +                      */
> > +                     map_size = min(map_size_needed + 2*EFI_PAGE_SIZE,
> > +                                    offset_limit);
> > +                     va = early_memremap(efi.mokvar_table, map_size);
>
> Can't we just map the entire region from efi.mokvar_table to end_pa in
> one early_memremap call before the loop and avoid all the remapping
> logic?
>

I suppose that depends on whether there is a reasonable upper bound on
the size which is guaranteed to be mappable using early_memremap()
(e.g., 128 KB on 32-bit ARM, or 256 KB on other architectures)


> > +                     if (!va) {
> > +                             pr_err("Failed to map EFI MOKvar config table pa=0x%lx, size=%zu.\n",
> > +                                    efi.mokvar_table, map_size);
> > +                             return;
> > +                     }
> > +                     mokvar_entry = va + cur_offset;
> > +             }
> > +
> > +             /* Check for last sentinel entry */
> > +             if (mokvar_entry->name[0] == '\0') {
> > +                     if (mokvar_entry->data_size != 0)
> > +                             break;
> > +                     err = 0;
> > +                     break;
> > +             }
> > +
> > +             /* Sanity check that the name is null terminated */
> > +             size = strnlen(mokvar_entry->name,
> > +                            sizeof(mokvar_entry->name));
> > +             if (size >= sizeof(mokvar_entry->name))
> > +                     break;
> > +
> > +             /* Advance to the next entry */
> > +             cur_offset = map_size_needed + mokvar_entry->data_size;
> > +     }
> > +
> > +     if (va)
> > +             early_memunmap(va, map_size);
> > +     if (err) {
> > +             pr_err("EFI MOKvar config table is not valid\n");
> > +             return;
> > +     }
>
> err will never be non-zero here: it was cleared when the
> efi_mem_desc_lookup() was done. I think the initialization of err to
> -EINVAL needs to be moved just prior to the loop.
>
> > +     efi_mem_reserve(efi.mokvar_table, map_size_needed);
> > +     efi_mokvar_table_size = map_size_needed;
> > +}
> > +
> > +/*
> > + * efi_mokvar_entry_next() - Get next entry in the EFI MOK config table
> > + *
> > + * mokvar_entry:     Pointer to current EFI MOK config table entry
> > + *                   or null. Null indicates get first entry.
> > + *                   Passed by reference. This is updated to the
> > + *                   same value as the return value.
> > + *
> > + * Returns:          Pointer to next EFI MOK config table entry
> > + *                   or null, if there are no more entries.
> > + *                   Same value is returned in the mokvar_entry
> > + *                   parameter.
> > + *
> > + * This routine depends on the EFI MOK config table being entirely
> > + * mapped with it's starting virtual address in efi_mokvar_table_va.
> > + */
> > +struct efi_mokvar_table_entry *efi_mokvar_entry_next(
> > +                     struct efi_mokvar_table_entry **mokvar_entry)
> > +{
> > +     struct efi_mokvar_table_entry *mokvar_cur;
> > +     struct efi_mokvar_table_entry *mokvar_next;
> > +     size_t size_cur;
> > +
> > +     mokvar_cur = *mokvar_entry;
> > +     *mokvar_entry = NULL;
> > +
> > +     if (efi_mokvar_table_va == NULL)
> > +             return NULL;
> > +
> > +     if (mokvar_cur == NULL) {
> > +             mokvar_next = efi_mokvar_table_va;
> > +     } else {
> > +             if (mokvar_cur->name[0] == '\0')
> > +                     return NULL;
> > +             size_cur = sizeof(*mokvar_cur) + mokvar_cur->data_size;
> > +             mokvar_next = (void *)mokvar_cur + size_cur;
> > +     }
> > +
> > +     if (mokvar_next->name[0] == '\0')
> > +             return NULL;
> > +
> > +     *mokvar_entry = mokvar_next;
> > +     return mokvar_next;
> > +}
> > +
> > +/*
> > + * efi_mokvar_entry_find() - Find EFI MOK config entry by name
> > + *
> > + * name:     Name of the entry to look for.
> > + *
> > + * Returns:  Pointer to EFI MOK config table entry if found;
> > + *           null otherwise.
> > + *
> > + * This routine depends on the EFI MOK config table being entirely
> > + * mapped with it's starting virtual address in efi_mokvar_table_va.
> > + */
> > +struct efi_mokvar_table_entry *efi_mokvar_entry_find(const char *name)
> > +{
> > +     struct efi_mokvar_table_entry *mokvar_entry = NULL;
> > +
> > +     while (efi_mokvar_entry_next(&mokvar_entry)) {
> > +             if (!strncmp(name, mokvar_entry->name,
> > +                          sizeof(mokvar_entry->name)))
> > +                     return mokvar_entry;
> > +     }
> > +     return NULL;
> > +}
> > +
> > +/*
> > + * efi_mokvar_sysfs_read() - sysfs binary file read routine
> > + *
> > + * Returns:  Count of bytes read.
> > + *
> > + * Copy EFI MOK config table entry data for this mokvar sysfs binary file
> > + * to the supplied buffer, starting at the specified offset into mokvar table
> > + * entry data, for the specified count bytes. The copy is limited by the
> > + * amount of data in this mokvar config table entry.
> > + */
> > +static ssize_t efi_mokvar_sysfs_read(struct file *file, struct kobject *kobj,
> > +                              struct bin_attribute *bin_attr, char *buf,
> > +                              loff_t off, size_t count)
> > +{
> > +     struct efi_mokvar_table_entry *mokvar_entry = bin_attr->private;
> > +
> > +     if (!capable(CAP_SYS_ADMIN))
> > +             return 0;
> > +
> > +     if (off >= mokvar_entry->data_size)
> > +             return 0;
> > +     if (count >  mokvar_entry->data_size - off)
> > +             count = mokvar_entry->data_size - off;
> > +
> > +     memcpy(buf, mokvar_entry->data + off, count);
> > +     return count;
> > +}
> > +
> > +/*
> > + * efi_mokvar_sysfs_init() - Map EFI MOK config table and create sysfs
> > + *
> > + * Map the EFI MOK variable config table for run-time use by the kernel
> > + * and create the sysfs entries in /sys/firmware/efi/mok-variables/
> > + *
> > + * This routine just returns if a valid EFI MOK variable config table
> > + * was not found earlier during boot.
> > + *
> > + * This routine must be called during a "middle" initcall phase, i.e.
> > + * after efi_mokvar_table_init() but before UEFI certs are loaded
> > + * during late init.
> > + *
> > + * Implicit inputs:
> > + * efi.mokvar_table: Physical address of EFI MOK variable config table
> > + *                   or special value that indicates no such table.
> > + *
> > + * efi_mokvar_table_size: Computed size of EFI MOK variable config table.
> > + *                   The table is considered present and valid if this
> > + *                   is non-zero.
> > + *
> > + * Implicit outputs:
> > + * efi_mokvar_table_va:      Start virtual address of the EFI MOK config table.
> > + */
> > +static int __init efi_mokvar_sysfs_init(void)
> > +{
> > +     void *config_va;
> > +     struct efi_mokvar_table_entry *mokvar_entry = NULL;
> > +     struct efi_mokvar_sysfs_attr *mokvar_sysfs = NULL;
> > +     int err = 0;
> > +
> > +     if (efi_mokvar_table_size == 0)
> > +             return -ENOENT;
> > +
> > +     config_va = memremap(efi.mokvar_table, efi_mokvar_table_size,
> > +                          MEMREMAP_WB);
> > +     if (!config_va) {
> > +             pr_err("Failed to map EFI MOKvar config table\n");
> > +             return -ENOMEM;
> > +     }
> > +     efi_mokvar_table_va = config_va;
> > +
> > +     mokvar_kobj = kobject_create_and_add("mok-variables", efi_kobj);
> > +     if (!mokvar_kobj) {
> > +             pr_err("Failed to create EFI mok-variables sysfs entry\n");
> > +             return -ENOMEM;
> > +     }
> > +
> > +     while (efi_mokvar_entry_next(&mokvar_entry)) {
> > +             mokvar_sysfs = kzalloc(sizeof(*mokvar_sysfs), GFP_KERNEL);
> > +             if (!mokvar_sysfs) {
> > +                     err = -ENOMEM;
> > +                     break;
> > +             }
> > +
> > +             sysfs_bin_attr_init(&mokvar_sysfs->bin_attr);
> > +             mokvar_sysfs->bin_attr.private = mokvar_entry;
> > +             mokvar_sysfs->bin_attr.attr.name = mokvar_entry->name;
> > +             mokvar_sysfs->bin_attr.attr.mode = 0400;
> > +             mokvar_sysfs->bin_attr.size = mokvar_entry->data_size;
> > +             mokvar_sysfs->bin_attr.read = efi_mokvar_sysfs_read;
> > +
> > +             err = sysfs_create_bin_file(mokvar_kobj,
> > +                                        &mokvar_sysfs->bin_attr);
> > +             if (err)
> > +                     break;
> > +
> > +             list_add_tail(&mokvar_sysfs->node, &efi_mokvar_sysfs_list);
> > +     }
> > +
> > +     if (err) {
> > +             pr_err("Failed to create some EFI mok-variables sysfs entries\n");
> > +             kfree(mokvar_sysfs);
> > +     }
> > +     return err;
> > +}
> > +device_initcall(efi_mokvar_sysfs_init);
> > diff --git a/include/linux/efi.h b/include/linux/efi.h
> > index 73db1ae04cef..4a2332f146eb 100644
> > --- a/include/linux/efi.h
> > +++ b/include/linux/efi.h
> > @@ -357,6 +357,7 @@ void efi_native_runtime_setup(void);
> >  #define LINUX_EFI_TPM_FINAL_LOG_GUID         EFI_GUID(0x1e2ed096, 0x30e2, 0x4254,  0xbd, 0x89, 0x86, 0x3b, 0xbe, 0xf8, 0x23, 0x25)
> >  #define LINUX_EFI_MEMRESERVE_TABLE_GUID              EFI_GUID(0x888eb0c6, 0x8ede, 0x4ff5,  0xa8, 0xf0, 0x9a, 0xee, 0x5c, 0xb9, 0x77, 0xc2)
> >  #define LINUX_EFI_INITRD_MEDIA_GUID          EFI_GUID(0x5568e427, 0x68fc, 0x4f3d,  0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68)
> > +#define LINUX_EFI_MOK_VARIABLE_TABLE_GUID    EFI_GUID(0xc451ed2b, 0x9694, 0x45d3,  0xba, 0xba, 0xed, 0x9f, 0x89, 0x88, 0xa3, 0x89)
> >
> >  /* OEM GUIDs */
> >  #define DELLEMC_EFI_RCI2_TABLE_GUID          EFI_GUID(0x2d9f28a2, 0xa886, 0x456a,  0x97, 0xa8, 0xf1, 0x1e, 0xf2, 0x4f, 0xf4, 0x55)
> > @@ -546,6 +547,7 @@ extern struct efi {
> >       unsigned long                   esrt;                   /* ESRT table */
> >       unsigned long                   tpm_log;                /* TPM2 Event Log table */
> >       unsigned long                   tpm_final_log;          /* TPM2 Final Events Log table */
> > +     unsigned long                   mokvar_table;           /* MOK variable config table */
> >
> >       efi_get_time_t                  *get_time;
> >       efi_set_time_t                  *set_time;
> > @@ -1252,4 +1254,36 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size);
> >
> >  char *efi_systab_show_arch(char *str);
> >
> > +/*
> > + * The LINUX_EFI_MOK_VARIABLE_TABLE_GUID config table can be provided
> > + * to the kernel by an EFI boot loader. The table contains a packed
> > + * sequence of these entries, one for each named MOK variable.
> > + * The sequence is terminated by an entry with a completely NULL
> > + * name and 0 data size.
> > + */
> > +struct efi_mokvar_table_entry {
> > +     char name[256];
> > +     u64 data_size;
> > +     u8 data[];
> > +} __attribute((packed));
> > +
> > +#ifdef CONFIG_LOAD_UEFI_KEYS
> > +extern void __init efi_mokvar_table_init(void);
> > +extern struct efi_mokvar_table_entry *efi_mokvar_entry_next(
> > +                     struct efi_mokvar_table_entry **mokvar_entry);
> > +extern struct efi_mokvar_table_entry *efi_mokvar_entry_find(const char *name);
> > +#else
> > +static inline void efi_mokvar_table_init(void) { }
> > +static inline struct efi_mokvar_table_entry *efi_mokvar_entry_next(
> > +                     struct efi_mokvar_table_entry **mokvar_entry)
> > +{
> > +     return NULL;
> > +}
> > +static inline struct efi_mokvar_table_entry *efi_mokvar_entry_find(
> > +                     const char *name)
> > +{
> > +     return NULL;
> > +}
> > +#endif
> > +
> >  #endif /* _LINUX_EFI_H */
> > --
> > 2.27.0
> >

Re: [PATCH V2 1/3] efi: Support for MOK variable config table

[Arvind Sankar]

On Mon, Sep 21, 2020 at 06:27:17PM +0200, Ard Biesheuvel wrote:
> On Mon, 21 Sep 2020 at 18:19, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > On Fri, Sep 04, 2020 at 09:31:05PM -0400, Lenny Szubowicz wrote:
> > > +     /*
> > > +      * The EFI MOK config table must fit within a single EFI memory
> > > +      * descriptor range.
> > > +      */
> > > +     err = efi_mem_desc_lookup(efi.mokvar_table, &md);
> > > +     if (err) {
> > > +             pr_warn("EFI MOKvar config table is not within the EFI memory map\n");
> > > +             return;
> > > +     }
> > > +     end_pa = efi_mem_desc_end(&md);
> > > +     if (efi.mokvar_table >= end_pa) {
> > > +             pr_err("EFI memory descriptor containing MOKvar config table is invalid\n");
> > > +             return;
> > > +     }
> >
> > efi_mem_desc_lookup() can't return success if efi.mokvar_table >= end_pa,
> > why check it again?
> >
> > > +     offset_limit = end_pa - efi.mokvar_table;
> > > +     /*
> > > +      * Validate the MOK config table. Since there is no table header
> > > +      * from which we could get the total size of the MOK config table,
> > > +      * we compute the total size as we validate each variably sized
> > > +      * entry, remapping as necessary.
> > > +      */
> > > +     while (cur_offset + sizeof(*mokvar_entry) <= offset_limit) {
> > > +             mokvar_entry = va + cur_offset;
> > > +             map_size_needed = cur_offset + sizeof(*mokvar_entry);
> > > +             if (map_size_needed > map_size) {
> > > +                     if (va)
> > > +                             early_memunmap(va, map_size);
> > > +                     /*
> > > +                      * Map a little more than the fixed size entry
> > > +                      * header, anticipating some data. It's safe to
> > > +                      * do so as long as we stay within current memory
> > > +                      * descriptor.
> > > +                      */
> > > +                     map_size = min(map_size_needed + 2*EFI_PAGE_SIZE,
> > > +                                    offset_limit);
> > > +                     va = early_memremap(efi.mokvar_table, map_size);
> >
> > Can't we just map the entire region from efi.mokvar_table to end_pa in
> > one early_memremap call before the loop and avoid all the remapping
> > logic?
> >
> 
> I suppose that depends on whether there is a reasonable upper bound on
> the size which is guaranteed to be mappable using early_memremap()
> (e.g., 128 KB on 32-bit ARM, or 256 KB on other architectures)

Ah, sorry, I thought only the number of early mappings was limited, not
the size as well. We could still just map the maximum possible
(NR_FIX_BTMAPS * PAGE_SIZE), since it will fail anyway if the config
table turns out to be bigger than that?

> 
> 
> > > +     if (va)
> > > +             early_memunmap(va, map_size);
> > > +     if (err) {
> > > +             pr_err("EFI MOKvar config table is not valid\n");
> > > +             return;
> > > +     }
> >
> > err will never be non-zero here: it was cleared when the
> > efi_mem_desc_lookup() was done. I think the initialization of err to
> > -EINVAL needs to be moved just prior to the loop.
> >
> > > +     efi_mem_reserve(efi.mokvar_table, map_size_needed);
> > > +     efi_mokvar_table_size = map_size_needed;
> > > +}

Re: [PATCH V2 1/3] efi: Support for MOK variable config table

[Lenny Szubowicz]

On 9/21/20 12:55 PM, Arvind Sankar wrote:
> On Mon, Sep 21, 2020 at 06:27:17PM +0200, Ard Biesheuvel wrote:
>> On Mon, 21 Sep 2020 at 18:19, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>>>
>>> On Fri, Sep 04, 2020 at 09:31:05PM -0400, Lenny Szubowicz wrote:
>>>> +     /*
>>>> +      * The EFI MOK config table must fit within a single EFI memory
>>>> +      * descriptor range.
>>>> +      */
>>>> +     err = efi_mem_desc_lookup(efi.mokvar_table, &md);
>>>> +     if (err) {
>>>> +             pr_warn("EFI MOKvar config table is not within the EFI memory map\n");
>>>> +             return;
>>>> +     }
>>>> +     end_pa = efi_mem_desc_end(&md);
>>>> +     if (efi.mokvar_table >= end_pa) {
>>>> +             pr_err("EFI memory descriptor containing MOKvar config table is invalid\n");
>>>> +             return;
>>>> +     }
>>>
>>> efi_mem_desc_lookup() can't return success if efi.mokvar_table >= end_pa,
>>> why check it again?

I agree it's unnecessary and I see that Ard has addressed this in a patch.

>>>
>>>> +     offset_limit = end_pa - efi.mokvar_table;
>>>> +     /*
>>>> +      * Validate the MOK config table. Since there is no table header
>>>> +      * from which we could get the total size of the MOK config table,
>>>> +      * we compute the total size as we validate each variably sized
>>>> +      * entry, remapping as necessary.
>>>> +      */
>>>> +     while (cur_offset + sizeof(*mokvar_entry) <= offset_limit) {
>>>> +             mokvar_entry = va + cur_offset;
>>>> +             map_size_needed = cur_offset + sizeof(*mokvar_entry);
>>>> +             if (map_size_needed > map_size) {
>>>> +                     if (va)
>>>> +                             early_memunmap(va, map_size);
>>>> +                     /*
>>>> +                      * Map a little more than the fixed size entry
>>>> +                      * header, anticipating some data. It's safe to
>>>> +                      * do so as long as we stay within current memory
>>>> +                      * descriptor.
>>>> +                      */
>>>> +                     map_size = min(map_size_needed + 2*EFI_PAGE_SIZE,
>>>> +                                    offset_limit);
>>>> +                     va = early_memremap(efi.mokvar_table, map_size);
>>>
>>> Can't we just map the entire region from efi.mokvar_table to end_pa in
>>> one early_memremap call before the loop and avoid all the remapping
>>> logic?
>>>
>>
>> I suppose that depends on whether there is a reasonable upper bound on
>> the size which is guaranteed to be mappable using early_memremap()
>> (e.g., 128 KB on 32-bit ARM, or 256 KB on other architectures)
> 
> Ah, sorry, I thought only the number of early mappings was limited, not
> the size as well. We could still just map the maximum possible
> (NR_FIX_BTMAPS * PAGE_SIZE), since it will fail anyway if the config
> table turns out to be bigger than that?

In practice, the loop will only do one or two mappings. That's because of
two factors.

First the code attempts to map a little more than it needs (2*EFI_PAGE_SIZE).
Secondly, right now only one entry in the MOKvar config table might be quite
large, i.e. the entry named MoKListRT. It's extremely likely that the header
of MokListRT entry will be encountered on the first mapping. If that entry
goes beyond what was originally mapped, then a second mapping will cover all
the data in that large entry as well as the remaining small entries that might
follow it.

> 
>>
>>
>>>> +     if (va)
>>>> +             early_memunmap(va, map_size);
>>>> +     if (err) {
>>>> +             pr_err("EFI MOKvar config table is not valid\n");
>>>> +             return;
>>>> +     }
>>>
>>> err will never be non-zero here: it was cleared when the
>>> efi_mem_desc_lookup() was done. I think the initialization of err to
>>> -EINVAL needs to be moved just prior to the loop.
>>>
>>>> +     efi_mem_reserve(efi.mokvar_table, map_size_needed);
>>>> +     efi_mokvar_table_size = map_size_needed;
>>>> +}
> 

Re: [PATCH V2 1/3] efi: Support for MOK variable config table

[Nathan Chancellor]

On Fri, Sep 04, 2020 at 09:31:05PM -0400, Lenny Szubowicz wrote:
> Because of system-specific EFI firmware limitations, EFI volatile
> variables may not be capable of holding the required contents of
> the Machine Owner Key (MOK) certificate store when the certificate
> list grows above some size. Therefore, an EFI boot loader may pass
> the MOK certs via a EFI configuration table created specifically for
> this purpose to avoid this firmware limitation.
> 
> An EFI configuration table is a much more primitive mechanism
> compared to EFI variables and is well suited for one-way passage
> of static information from a pre-OS environment to the kernel.
> 
> This patch adds initial kernel support to recognize, parse,
> and validate the EFI MOK configuration table, where named
> entries contain the same data that would otherwise be provided
> in similarly named EFI variables.
> 
> Additionally, this patch creates a sysfs binary file for each
> EFI MOK configuration table entry found. These files are read-only
> to root and are provided for use by user space utilities such as
> mokutil.
> 
> A subsequent patch will load MOK certs into the trusted platform
> key ring using this infrastructure.
> 
> Signed-off-by: Lenny Szubowicz <lszubowi@redhat.com>

I have not seen this reported yet but this breaks arm allyesconfig and
allmodconfig when CPU_LITTLE_ENDIAN is force selected (because CONFIG_EFI
will actually be enabled):

$ cat le.config
CONFIG_CPU_BIG_ENDIAN=n

$ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- KCONFIG_ALLCONFIG=le.config allyesconfig drivers/firmware/efi/mokvar-table.o
drivers/firmware/efi/mokvar-table.c: In function 'efi_mokvar_table_init':
drivers/firmware/efi/mokvar-table.c:139:5: error: implicit declaration of function 'early_memunmap' [-Werror=implicit-function-declaration]
  139 |     early_memunmap(va, map_size);
      |     ^~~~~~~~~~~~~~
drivers/firmware/efi/mokvar-table.c:148:9: error: implicit declaration of function 'early_memremap' [-Werror=implicit-function-declaration]
  148 |    va = early_memremap(efi.mokvar_table, map_size);
      |         ^~~~~~~~~~~~~~
drivers/firmware/efi/mokvar-table.c:148:7: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
  148 |    va = early_memremap(efi.mokvar_table, map_size);
      |       ^
cc1: some warnings being treated as errors
make[4]: *** [scripts/Makefile.build:283: drivers/firmware/efi/mokvar-table.o] Error 1

Cheers,
Nathan

Re: [PATCH V2 1/3] efi: Support for MOK variable config table

[Ard Biesheuvel]

On Thu, 1 Oct 2020 at 19:44, Nathan Chancellor <natechancellor@gmail.com> wrote:
>
> On Fri, Sep 04, 2020 at 09:31:05PM -0400, Lenny Szubowicz wrote:
> > Because of system-specific EFI firmware limitations, EFI volatile
> > variables may not be capable of holding the required contents of
> > the Machine Owner Key (MOK) certificate store when the certificate
> > list grows above some size. Therefore, an EFI boot loader may pass
> > the MOK certs via a EFI configuration table created specifically for
> > this purpose to avoid this firmware limitation.
> >
> > An EFI configuration table is a much more primitive mechanism
> > compared to EFI variables and is well suited for one-way passage
> > of static information from a pre-OS environment to the kernel.
> >
> > This patch adds initial kernel support to recognize, parse,
> > and validate the EFI MOK configuration table, where named
> > entries contain the same data that would otherwise be provided
> > in similarly named EFI variables.
> >
> > Additionally, this patch creates a sysfs binary file for each
> > EFI MOK configuration table entry found. These files are read-only
> > to root and are provided for use by user space utilities such as
> > mokutil.
> >
> > A subsequent patch will load MOK certs into the trusted platform
> > key ring using this infrastructure.
> >
> > Signed-off-by: Lenny Szubowicz <lszubowi@redhat.com>
>
> I have not seen this reported yet but this breaks arm allyesconfig and
> allmodconfig when CPU_LITTLE_ENDIAN is force selected (because CONFIG_EFI
> will actually be enabled):
>
> $ cat le.config
> CONFIG_CPU_BIG_ENDIAN=n
>
> $ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- KCONFIG_ALLCONFIG=le.config allyesconfig drivers/firmware/efi/mokvar-table.o
> drivers/firmware/efi/mokvar-table.c: In function 'efi_mokvar_table_init':
> drivers/firmware/efi/mokvar-table.c:139:5: error: implicit declaration of function 'early_memunmap' [-Werror=implicit-function-declaration]
>   139 |     early_memunmap(va, map_size);
>       |     ^~~~~~~~~~~~~~
> drivers/firmware/efi/mokvar-table.c:148:9: error: implicit declaration of function 'early_memremap' [-Werror=implicit-function-declaration]
>   148 |    va = early_memremap(efi.mokvar_table, map_size);
>       |         ^~~~~~~~~~~~~~
> drivers/firmware/efi/mokvar-table.c:148:7: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
>   148 |    va = early_memremap(efi.mokvar_table, map_size);
>       |       ^
> cc1: some warnings being treated as errors
> make[4]: *** [scripts/Makefile.build:283: drivers/firmware/efi/mokvar-table.o] Error 1
>
> Cheers,
> Nathan

Hi Nathan,

Does adding

#include <asm/early_ioremap.h>

to drivers/firmware/efi/mokvar-table.c fix the issue?

Re: [PATCH V2 1/3] efi: Support for MOK variable config table

[Nathan Chancellor]

On Thu, Oct 01, 2020 at 10:57:07PM +0200, Ard Biesheuvel wrote:
> On Thu, 1 Oct 2020 at 19:44, Nathan Chancellor <natechancellor@gmail.com> wrote:
> >
> > On Fri, Sep 04, 2020 at 09:31:05PM -0400, Lenny Szubowicz wrote:
> > > Because of system-specific EFI firmware limitations, EFI volatile
> > > variables may not be capable of holding the required contents of
> > > the Machine Owner Key (MOK) certificate store when the certificate
> > > list grows above some size. Therefore, an EFI boot loader may pass
> > > the MOK certs via a EFI configuration table created specifically for
> > > this purpose to avoid this firmware limitation.
> > >
> > > An EFI configuration table is a much more primitive mechanism
> > > compared to EFI variables and is well suited for one-way passage
> > > of static information from a pre-OS environment to the kernel.
> > >
> > > This patch adds initial kernel support to recognize, parse,
> > > and validate the EFI MOK configuration table, where named
> > > entries contain the same data that would otherwise be provided
> > > in similarly named EFI variables.
> > >
> > > Additionally, this patch creates a sysfs binary file for each
> > > EFI MOK configuration table entry found. These files are read-only
> > > to root and are provided for use by user space utilities such as
> > > mokutil.
> > >
> > > A subsequent patch will load MOK certs into the trusted platform
> > > key ring using this infrastructure.
> > >
> > > Signed-off-by: Lenny Szubowicz <lszubowi@redhat.com>
> >
> > I have not seen this reported yet but this breaks arm allyesconfig and
> > allmodconfig when CPU_LITTLE_ENDIAN is force selected (because CONFIG_EFI
> > will actually be enabled):
> >
> > $ cat le.config
> > CONFIG_CPU_BIG_ENDIAN=n
> >
> > $ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- KCONFIG_ALLCONFIG=le.config allyesconfig drivers/firmware/efi/mokvar-table.o
> > drivers/firmware/efi/mokvar-table.c: In function 'efi_mokvar_table_init':
> > drivers/firmware/efi/mokvar-table.c:139:5: error: implicit declaration of function 'early_memunmap' [-Werror=implicit-function-declaration]
> >   139 |     early_memunmap(va, map_size);
> >       |     ^~~~~~~~~~~~~~
> > drivers/firmware/efi/mokvar-table.c:148:9: error: implicit declaration of function 'early_memremap' [-Werror=implicit-function-declaration]
> >   148 |    va = early_memremap(efi.mokvar_table, map_size);
> >       |         ^~~~~~~~~~~~~~
> > drivers/firmware/efi/mokvar-table.c:148:7: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
> >   148 |    va = early_memremap(efi.mokvar_table, map_size);
> >       |       ^
> > cc1: some warnings being treated as errors
> > make[4]: *** [scripts/Makefile.build:283: drivers/firmware/efi/mokvar-table.o] Error 1
> >
> > Cheers,
> > Nathan
> 
> Hi Nathan,
> 
> Does adding
> 
> #include <asm/early_ioremap.h>
> 
> to drivers/firmware/efi/mokvar-table.c fix the issue?

Indeed, that was much simpler than I thought it would be... If you send
or apply a patch, feel free to add:

Tested-by: Nathan Chancellor <natechancellor@gmail.com>

Cheers,
Nathan