[PATCH 0/9] efi: implement support for EFI RT properties table

[PATCH 0/9] efi: implement support for EFI RT properties table

[Ard Biesheuvel]

The UEFI spec version 2.8 errata A defines a configuration table called
EFI_RT_PROPERTIES_TABLE that carries a mask describing which EFI runtime
services are still functional at OS runtime.

Even though any runtime services that cease to be functional when exiting
boot services are still required to return EFI_UNSUPPORTED when called by
the OS, having this mask is helpful, since we can use it to prevent modules
like efi-rtc or efivars from loading, instead of allowing them to probe and
fail with an error.

So let's wire this up: make some room in struct efi for the mask, read it
from the EFI_RT_PROPERTIES_TABLE if available, and replace various instances
of 'if (efi_enabled(EFI_RUNTIME_SERVICES))' with checks for the runtime
service in question that the code relies upon.

Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Peter Jones <pjones@redhat.com>
Cc: Alexander Graf <agraf@csgraf.de>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Jeff Brasen <jbrasen@nvidia.com>
Cc: Atish Patra <Atish.Patra@wdc.com>
Cc: x86@kernel.org

Ard Biesheuvel (9):
  efi: store mask of supported runtime services in struct efi
  efi: add support for EFI_RT_PROPERTIES table
  efi: use more granular check for availability for variable services
  efi: register EFI rtc platform device only when available
  infiniband: hfi1: use EFI GetVariable only when available
  scsi: iscsi: use EFI GetVariable only when available
  efi: use EFI ResetSystem only when available
  x86/ima: use EFI GetVariable only when available
  integrity: check properly whether EFI GetVariable() is available

 arch/x86/kernel/ima_arch.c                    |  2 +-
 drivers/firmware/efi/efi-pstore.c             |  2 +-
 drivers/firmware/efi/efi.c                    | 70 +++++++++++--------
 drivers/firmware/efi/efivars.c                |  2 +-
 drivers/firmware/efi/reboot.c                 |  4 +-
 drivers/infiniband/hw/hfi1/efivar.c           |  2 +-
 drivers/rtc/Makefile                          |  4 --
 drivers/rtc/rtc-efi-platform.c                | 35 ----------
 drivers/scsi/isci/init.c                      |  2 +-
 fs/efivarfs/super.c                           |  2 +-
 include/linux/efi.h                           | 40 +++++++++++
 security/integrity/platform_certs/load_uefi.c |  2 +-
 12 files changed, 89 insertions(+), 78 deletions(-)
 delete mode 100644 drivers/rtc/rtc-efi-platform.c

-- 
2.17.1

[PATCH 1/9] efi: store mask of supported runtime services in struct efi

[Ard Biesheuvel]

Revision 2.8 of the UEFI spec introduces provisions for firmware to
advertise lack of support for certain runtime services at OS runtime.
Let's store this mask in struct efi for easy access.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/efi.c | 27 ++++++++++-------
 include/linux/efi.h        | 31 ++++++++++++++++++++
 2 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 7dbe1487b111..703a019d81b4 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -34,6 +34,7 @@
 #include <asm/early_ioremap.h>
 
 struct efi __read_mostly efi = {
+	.runtime_supported_mask = EFI_RT_SUPPORTED_ALL,
 	.acpi			= EFI_INVALID_TABLE_ADDR,
 	.acpi20			= EFI_INVALID_TABLE_ADDR,
 	.smbios			= EFI_INVALID_TABLE_ADDR,
@@ -301,16 +302,22 @@ static int __init efisubsys_init(void)
 	if (!efi_enabled(EFI_BOOT))
 		return 0;
 
-	/*
-	 * Since we process only one efi_runtime_service() at a time, an
-	 * ordered workqueue (which creates only one execution context)
-	 * should suffice all our needs.
-	 */
-	efi_rts_wq = alloc_ordered_workqueue("efi_rts_wq", 0);
-	if (!efi_rts_wq) {
-		pr_err("Creating efi_rts_wq failed, EFI runtime services disabled.\n");
-		clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
-		return 0;
+	if (!efi_enabled(EFI_RUNTIME_SERVICES))
+		efi.runtime_supported_mask = 0;
+
+	if (efi.runtime_supported_mask) {
+		/*
+		 * Since we process only one efi_runtime_service() at a time, an
+		 * ordered workqueue (which creates only one execution context)
+		 * should suffice for all our needs.
+		 */
+		efi_rts_wq = alloc_ordered_workqueue("efi_rts_wq", 0);
+		if (!efi_rts_wq) {
+			pr_err("Creating efi_rts_wq failed, EFI runtime services disabled.\n");
+			clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
+			efi.runtime_supported_mask = 0;
+			return 0;
+		}
 	}
 
 	/* We register the efi directory at /sys/firmware/efi */
diff --git a/include/linux/efi.h b/include/linux/efi.h
index a0008e3d4e9d..57695f400044 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -523,6 +523,7 @@ typedef struct {
 extern struct efi {
 	const efi_runtime_services_t	*runtime;		/* EFI runtime services table */
 	unsigned int			runtime_version;	/* Runtime services version */
+	unsigned int			runtime_supported_mask;
 
 	unsigned long			acpi;			/* ACPI table  (IA64 ext 0.71) */
 	unsigned long			acpi20;			/* ACPI table  (ACPI 2.0) */
@@ -551,6 +552,26 @@ extern struct efi {
 	unsigned long			flags;
 } efi;
 
+#define EFI_RT_SUPPORTED_GET_TIME 				0x0001
+#define EFI_RT_SUPPORTED_SET_TIME 				0x0002
+#define EFI_RT_SUPPORTED_GET_WAKEUP_TIME			0x0004
+#define EFI_RT_SUPPORTED_SET_WAKEUP_TIME			0x0008
+#define EFI_RT_SUPPORTED_GET_VARIABLE				0x0010
+#define EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME			0x0020
+#define EFI_RT_SUPPORTED_SET_VARIABLE				0x0040
+#define EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP		0x0080
+#define EFI_RT_SUPPORTED_CONVERT_POINTER			0x0100
+#define EFI_RT_SUPPORTED_GET_NEXT_HIGH_MONOTONIC_COUNT		0x0200
+#define EFI_RT_SUPPORTED_RESET_SYSTEM				0x0400
+#define EFI_RT_SUPPORTED_UPDATE_CAPSULE				0x0800
+#define EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES		0x1000
+#define EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO			0x2000
+
+#define EFI_RT_SUPPORTED_ALL					0x3fff
+
+#define EFI_RT_SUPPORTED_TIME_SERVICES				0x000f
+#define EFI_RT_SUPPORTED_VARIABLE_SERVICES			0x0070
+
 extern struct mm_struct efi_mm;
 
 static inline int
@@ -761,6 +782,11 @@ static inline bool __pure efi_soft_reserve_enabled(void)
 	return IS_ENABLED(CONFIG_EFI_SOFT_RESERVE)
 		&& __efi_soft_reserve_enabled();
 }
+
+static inline bool efi_rt_services_supported(unsigned int mask)
+{
+	return (efi.runtime_supported_mask & mask) == mask;
+}
 #else
 static inline bool efi_enabled(int feature)
 {
@@ -779,6 +805,11 @@ static inline bool efi_soft_reserve_enabled(void)
 {
 	return false;
 }
+
+static inline bool efi_rt_services_supported(unsigned int mask)
+{
+	return false;
+}
 #endif
 
 extern int efi_status_to_err(efi_status_t status);
-- 
2.17.1

[PATCH 2/9] efi: add support for EFI_RT_PROPERTIES table

[Ard Biesheuvel]

Take the newly introduced EFI_RT_PROPERTIES_TABLE configuration table
into account, which carries a mask of which EFI runtime services are
still functional after ExitBootServices() has been called by the OS.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/efi.c | 12 ++++++++++++
 include/linux/efi.h        |  9 +++++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 703a019d81b4..a35230517f9c 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -47,6 +47,7 @@ EXPORT_SYMBOL(efi);
 
 static unsigned long __ro_after_init rng_seed = EFI_INVALID_TABLE_ADDR;
 static unsigned long __initdata mem_reserve = EFI_INVALID_TABLE_ADDR;
+static unsigned long __initdata rt_prop = EFI_INVALID_TABLE_ADDR;
 
 struct mm_struct efi_mm = {
 	.mm_rb			= RB_ROOT,
@@ -449,6 +450,7 @@ static const efi_config_table_type_t common_tables[] __initconst = {
 	{LINUX_EFI_TPM_EVENT_LOG_GUID, "TPMEventLog", &efi.tpm_log},
 	{LINUX_EFI_TPM_FINAL_LOG_GUID, "TPMFinalLog", &efi.tpm_final_log},
 	{LINUX_EFI_MEMRESERVE_TABLE_GUID, "MEMRESERVE", &mem_reserve},
+	{EFI_RT_PROPERTIES_TABLE_GUID, "RTPROP", &rt_prop},
 #ifdef CONFIG_EFI_RCI2_TABLE
 	{DELLEMC_EFI_RCI2_TABLE_GUID, NULL, &rci2_table_phys},
 #endif
@@ -575,6 +577,16 @@ int __init efi_config_parse_tables(const efi_config_table_t *config_tables,
 		}
 	}
 
+	if (rt_prop != EFI_INVALID_TABLE_ADDR) {
+		efi_rt_properties_table_t *tbl;
+
+		tbl = early_memremap(rt_prop, sizeof(*tbl));
+		if (tbl) {
+			efi.runtime_supported_mask &= tbl->runtime_services_supported;
+			early_memunmap(tbl, sizeof(*tbl));
+		}
+	}
+
 	return 0;
 }
 
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 57695f400044..2ab33d5d6ca5 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -334,6 +334,7 @@ void efi_native_runtime_setup(void);
 #define EFI_TCG2_PROTOCOL_GUID			EFI_GUID(0x607f766c, 0x7455, 0x42be,  0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f)
 #define EFI_LOAD_FILE_PROTOCOL_GUID		EFI_GUID(0x56ec3091, 0x954c, 0x11d2,  0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
 #define EFI_LOAD_FILE2_PROTOCOL_GUID		EFI_GUID(0x4006c0c1, 0xfcb3, 0x403e,  0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d)
+#define EFI_RT_PROPERTIES_TABLE_GUID		EFI_GUID(0xeb66918a, 0x7eef, 0x402a,  0x84, 0x2e, 0x93, 0x1d, 0x21, 0xc3, 0x8a, 0xe9)
 
 #define EFI_IMAGE_SECURITY_DATABASE_GUID	EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596,  0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
 #define EFI_SHIM_LOCK_GUID			EFI_GUID(0x605dab50, 0xe046, 0x4300,  0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
@@ -486,6 +487,14 @@ typedef struct {
 #define EFI_PROPERTIES_TABLE_VERSION	0x00010000
 #define EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA	0x1
 
+typedef struct {
+	u16 version;
+	u16 length;
+	u32 runtime_services_supported;
+} efi_rt_properties_table_t;
+
+#define EFI_RT_PROPERTIES_TABLE_VERSION	0x1
+
 #define EFI_INVALID_TABLE_ADDR		(~0UL)
 
 typedef struct {
-- 
2.17.1

[PATCH 3/9] efi: use more granular check for availability for variable services

[Ard Biesheuvel]

The UEFI spec rev 2.8 permits firmware implementations to support only
a subset of EFI runtime services at OS runtime (i.e., after the call to
ExitBootServices()), so let's take this into account in the drivers that
rely specifically on the availability of the EFI variable services.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/efi-pstore.c |  2 +-
 drivers/firmware/efi/efi.c        | 28 ++++++--------------
 drivers/firmware/efi/efivars.c    |  2 +-
 fs/efivarfs/super.c               |  2 +-
 4 files changed, 11 insertions(+), 23 deletions(-)

diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
index 9ea13e8d12ec..d2f6855d205b 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -356,7 +356,7 @@ static struct pstore_info efi_pstore_info = {
 
 static __init int efivars_pstore_init(void)
 {
-	if (!efi_enabled(EFI_RUNTIME_SERVICES))
+	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_VARIABLE_SERVICES))
 		return 0;
 
 	if (!efivars_kobject())
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index a35230517f9c..abf4c02e0201 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -328,12 +328,13 @@ static int __init efisubsys_init(void)
 		return -ENOMEM;
 	}
 
-	error = generic_ops_register();
-	if (error)
-		goto err_put;
-
-	if (efi_enabled(EFI_RUNTIME_SERVICES))
+	if (efi_rt_services_supported(EFI_RT_SUPPORTED_VARIABLE_SERVICES)) {
 		efivar_ssdt_load();
+		error = generic_ops_register();
+		if (error)
+			goto err_put;
+		platform_device_register_simple("efivars", 0, NULL, 0);
+	}
 
 	error = sysfs_create_group(efi_kobj, &efi_subsys_attr_group);
 	if (error) {
@@ -358,7 +359,8 @@ static int __init efisubsys_init(void)
 err_remove_group:
 	sysfs_remove_group(efi_kobj, &efi_subsys_attr_group);
 err_unregister:
-	generic_ops_unregister();
+	if (efi_rt_services_supported(EFI_RT_SUPPORTED_VARIABLE_SERVICES))
+		generic_ops_unregister();
 err_put:
 	kobject_put(efi_kobj);
 	return error;
@@ -650,20 +652,6 @@ void __init efi_systab_report_header(const efi_table_hdr_t *systab_hdr,
 		vendor);
 }
 
-#ifdef CONFIG_EFI_VARS_MODULE
-static int __init efi_load_efivars(void)
-{
-	struct platform_device *pdev;
-
-	if (!efi_enabled(EFI_RUNTIME_SERVICES))
-		return 0;
-
-	pdev = platform_device_register_simple("efivars", 0, NULL, 0);
-	return PTR_ERR_OR_ZERO(pdev);
-}
-device_initcall(efi_load_efivars);
-#endif
-
 static __initdata char memory_type_name[][20] = {
 	"Reserved",
 	"Loader Code",
diff --git a/drivers/firmware/efi/efivars.c b/drivers/firmware/efi/efivars.c
index 7576450c8254..d309abca5091 100644
--- a/drivers/firmware/efi/efivars.c
+++ b/drivers/firmware/efi/efivars.c
@@ -664,7 +664,7 @@ int efivars_sysfs_init(void)
 	struct kobject *parent_kobj = efivars_kobject();
 	int error = 0;
 
-	if (!efi_enabled(EFI_RUNTIME_SERVICES))
+	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_VARIABLE_SERVICES))
 		return -ENODEV;
 
 	/* No efivars has been registered yet */
diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index fa4f6447ddad..12c66f5d92dd 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -252,7 +252,7 @@ static struct file_system_type efivarfs_type = {
 
 static __init int efivarfs_init(void)
 {
-	if (!efi_enabled(EFI_RUNTIME_SERVICES))
+	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_VARIABLE_SERVICES))
 		return -ENODEV;
 
 	if (!efivars_kobject())
-- 
2.17.1

[PATCH 4/9] efi: register EFI rtc platform device only when available

[Ard Biesheuvel]

Drop the separate driver that registers the EFI rtc on all EFI
systems that have runtime services available, and instead, move
the registration into the core EFI code, and make it conditional
on whether the actual time related services are available.

Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: linux-rtc@vger.kernel.org
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/efi.c     |  3 ++
 drivers/rtc/Makefile           |  4 ---
 drivers/rtc/rtc-efi-platform.c | 35 --------------------
 3 files changed, 3 insertions(+), 39 deletions(-)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index abf4c02e0201..69a585106d30 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -321,6 +321,9 @@ static int __init efisubsys_init(void)
 		}
 	}
 
+	if (efi_rt_services_supported(EFI_RT_SUPPORTED_TIME_SERVICES))
+		platform_device_register_simple("rtc-efi", 0, NULL, 0);
+
 	/* We register the efi directory at /sys/firmware/efi */
 	efi_kobj = kobject_create_and_add("efi", firmware_kobj);
 	if (!efi_kobj) {
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 4ac8f19fb631..24c7dfa1bd7d 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -12,10 +12,6 @@ obj-$(CONFIG_RTC_CLASS)		+= rtc-core.o
 obj-$(CONFIG_RTC_MC146818_LIB)	+= rtc-mc146818-lib.o
 rtc-core-y			:= class.o interface.o
 
-ifdef CONFIG_RTC_DRV_EFI
-rtc-core-y			+= rtc-efi-platform.o
-endif
-
 rtc-core-$(CONFIG_RTC_NVMEM)		+= nvmem.o
 rtc-core-$(CONFIG_RTC_INTF_DEV)		+= dev.o
 rtc-core-$(CONFIG_RTC_INTF_PROC)	+= proc.o
diff --git a/drivers/rtc/rtc-efi-platform.c b/drivers/rtc/rtc-efi-platform.c
deleted file mode 100644
index 6c037dc4e3dc..000000000000
--- a/drivers/rtc/rtc-efi-platform.c
+++ /dev/null
@@ -1,35 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Moved from arch/ia64/kernel/time.c
- *
- * Copyright (C) 1998-2003 Hewlett-Packard Co
- *	Stephane Eranian <eranian@hpl.hp.com>
- *	David Mosberger <davidm@hpl.hp.com>
- * Copyright (C) 1999 Don Dugger <don.dugger@intel.com>
- * Copyright (C) 1999-2000 VA Linux Systems
- * Copyright (C) 1999-2000 Walt Drummond <drummond@valinux.com>
- */
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/init.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/efi.h>
-#include <linux/platform_device.h>
-
-static struct platform_device rtc_efi_dev = {
-	.name = "rtc-efi",
-	.id = -1,
-};
-
-static int __init rtc_init(void)
-{
-	if (efi_enabled(EFI_RUNTIME_SERVICES))
-		if (platform_device_register(&rtc_efi_dev) < 0)
-			pr_err("unable to register rtc device...\n");
-
-	/* not necessarily an error */
-	return 0;
-}
-module_init(rtc_init);
-- 
2.17.1

[PATCH 5/9] infiniband: hfi1: use EFI GetVariable only when available

[Ard Biesheuvel]

Replace the EFI runtime services check with one that tells us whether
EFI GetVariable() is implemented by the firmware.

Cc: Mike Marciniszyn <mike.marciniszyn@intel.com>
Cc: Dennis Dalessandro <dennis.dalessandro@intel.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: linux-rdma@vger.kernel.org
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/infiniband/hw/hfi1/efivar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/hfi1/efivar.c b/drivers/infiniband/hw/hfi1/efivar.c
index d106d23016ba..c22ab7b5163b 100644
--- a/drivers/infiniband/hw/hfi1/efivar.c
+++ b/drivers/infiniband/hw/hfi1/efivar.c
@@ -78,7 +78,7 @@ static int read_efi_var(const char *name, unsigned long *size,
 	*size = 0;
 	*return_data = NULL;
 
-	if (!efi_enabled(EFI_RUNTIME_SERVICES))
+	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
 		return -EOPNOTSUPP;
 
 	uni_name = kcalloc(strlen(name) + 1, sizeof(efi_char16_t), GFP_KERNEL);
-- 
2.17.1

[PATCH 6/9] scsi: iscsi: use EFI GetVariable only when available

[Ard Biesheuvel]

Replace the EFI runtime services check with one that tells us whether
EFI GetVariable() is implemented by the firmware.

Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/scsi/isci/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index b48aac8dfcb8..974c3b9116d5 100644
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -621,7 +621,7 @@ static int isci_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		return -ENOMEM;
 	pci_set_drvdata(pdev, pci_info);
 
-	if (efi_enabled(EFI_RUNTIME_SERVICES))
+	if (efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
 		orom = isci_get_efi_var(pdev);
 
 	if (!orom)
-- 
2.17.1

[PATCH 7/9] efi: use EFI ResetSystem only when available

[Ard Biesheuvel]

Do not attempt to call EFI ResetSystem if the runtime supported mask tells
us it is no longer functional at OS runtime.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/reboot.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/reboot.c b/drivers/firmware/efi/reboot.c
index 7effff969eb9..73089a24f04b 100644
--- a/drivers/firmware/efi/reboot.c
+++ b/drivers/firmware/efi/reboot.c
@@ -15,7 +15,7 @@ void efi_reboot(enum reboot_mode reboot_mode, const char *__unused)
 	const char *str[] = { "cold", "warm", "shutdown", "platform" };
 	int efi_mode, cap_reset_mode;
 
-	if (!efi_enabled(EFI_RUNTIME_SERVICES))
+	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_RESET_SYSTEM))
 		return;
 
 	switch (reboot_mode) {
@@ -64,7 +64,7 @@ static void efi_power_off(void)
 
 static int __init efi_shutdown_init(void)
 {
-	if (!efi_enabled(EFI_RUNTIME_SERVICES))
+	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_RESET_SYSTEM))
 		return -ENODEV;
 
 	if (efi_poweroff_required()) {
-- 
2.17.1

[PATCH 8/9] x86/ima: use EFI GetVariable only when available

[Ard Biesheuvel]

Replace the EFI runtime services check with one that tells us whether
EFI GetVariable() is implemented by the firmware.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/kernel/ima_arch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c
index 4d4f5d9faac3..cb6ed616a543 100644
--- a/arch/x86/kernel/ima_arch.c
+++ b/arch/x86/kernel/ima_arch.c
@@ -19,7 +19,7 @@ static enum efi_secureboot_mode get_sb_mode(void)
 
 	size = sizeof(secboot);
 
-	if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
+	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE)) {
 		pr_info("ima: secureboot mode unknown, no efi\n");
 		return efi_secureboot_mode_unknown;
 	}
-- 
2.17.1

[PATCH 9/9] integrity: check properly whether EFI GetVariable() is available

[Ard Biesheuvel]

Testing the value of the efi.get_variable function pointer is not
the right way to establish whether the platform supports EFI
variables at runtime. Instead, use the newly added granular check
that can test for the presence of each EFI runtime service
individually.

Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 security/integrity/platform_certs/load_uefi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index 111898aad56e..e2fe1bd3abb9 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -76,7 +76,7 @@ static int __init load_uefi_certs(void)
 	unsigned long dbsize = 0, dbxsize = 0, moksize = 0;
 	int rc = 0;
 
-	if (!efi.get_variable)
+	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
-- 
2.17.1

Re: [PATCH 0/9] efi: implement support for EFI RT properties table

[Heinrich Schuchardt]

On 2/19/20 6:18 PM, Ard Biesheuvel wrote:
> The UEFI spec version 2.8 errata A defines a configuration table called
> EFI_RT_PROPERTIES_TABLE that carries a mask describing which EFI runtime
> services are still functional at OS runtime.
This configuration table defined in UEFI spec 2.8A is replacing the
RuntimeServicesSupported variable introduced in UEFI spec 2.8 (which is
already implemented in U-Boot).

Replacing APIs as an "erratum" is unfortunate.

I am wondering whether in your implementation you will have to check for
both the variable if the firmware implements UEFI 2.8 and for the table
if the firmware implements UEFI 2.8A.

Best regards

Heinrich

>
> Even though any runtime services that cease to be functional when exiting
> boot services are still required to return EFI_UNSUPPORTED when called by
> the OS, having this mask is helpful, since we can use it to prevent modules
> like efi-rtc or efivars from loading, instead of allowing them to probe and
> fail with an error.
>
> So let's wire this up: make some room in struct efi for the mask, read it
> from the EFI_RT_PROPERTIES_TABLE if available, and replace various instances
> of 'if (efi_enabled(EFI_RUNTIME_SERVICES))' with checks for the runtime
> service in question that the code relies upon.
>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Peter Jones <pjones@redhat.com>
> Cc: Alexander Graf <agraf@csgraf.de>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Jeff Brasen <jbrasen@nvidia.com>
> Cc: Atish Patra <Atish.Patra@wdc.com>
> Cc: x86@kernel.org
>
> Ard Biesheuvel (9):
>    efi: store mask of supported runtime services in struct efi
>    efi: add support for EFI_RT_PROPERTIES table
>    efi: use more granular check for availability for variable services
>    efi: register EFI rtc platform device only when available
>    infiniband: hfi1: use EFI GetVariable only when available
>    scsi: iscsi: use EFI GetVariable only when available
>    efi: use EFI ResetSystem only when available
>    x86/ima: use EFI GetVariable only when available
>    integrity: check properly whether EFI GetVariable() is available
>
>   arch/x86/kernel/ima_arch.c                    |  2 +-
>   drivers/firmware/efi/efi-pstore.c             |  2 +-
>   drivers/firmware/efi/efi.c                    | 70 +++++++++++--------
>   drivers/firmware/efi/efivars.c                |  2 +-
>   drivers/firmware/efi/reboot.c                 |  4 +-
>   drivers/infiniband/hw/hfi1/efivar.c           |  2 +-
>   drivers/rtc/Makefile                          |  4 --
>   drivers/rtc/rtc-efi-platform.c                | 35 ----------
>   drivers/scsi/isci/init.c                      |  2 +-
>   fs/efivarfs/super.c                           |  2 +-
>   include/linux/efi.h                           | 40 +++++++++++
>   security/integrity/platform_certs/load_uefi.c |  2 +-
>   12 files changed, 89 insertions(+), 78 deletions(-)
>   delete mode 100644 drivers/rtc/rtc-efi-platform.c
>

Re: [PATCH 0/9] efi: implement support for EFI RT properties table

[Ard Biesheuvel]

On Wed, 19 Feb 2020 at 19:59, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 2/19/20 6:18 PM, Ard Biesheuvel wrote:
> > The UEFI spec version 2.8 errata A defines a configuration table called
> > EFI_RT_PROPERTIES_TABLE that carries a mask describing which EFI runtime
> > services are still functional at OS runtime.
> This configuration table defined in UEFI spec 2.8A is replacing the
> RuntimeServicesSupported variable introduced in UEFI spec 2.8 (which is
> already implemented in U-Boot).
>

Indeed. But using a variable was a mistake, and no OS implemented
support for it yet, so we decided it was best to issue an errata and
pretend that the variable never existed.

> Replacing APIs as an "erratum" is unfortunate.
>

It was a dilemma. Having the two solutions co-exist was not a great
prospect either.

> I am wondering whether in your implementation you will have to check for
> both the variable if the firmware implements UEFI 2.8 and for the table
> if the firmware implements UEFI 2.8A.
>

No, Linux will only look for the config table - that was the whole point.

Re: [PATCH 9/9] integrity: check properly whether EFI GetVariable() is available

[Serge E. Hallyn]

On Wed, Feb 19, 2020 at 06:19:07PM +0100, Ard Biesheuvel wrote:
> Testing the value of the efi.get_variable function pointer is not
> the right way to establish whether the platform supports EFI
> variables at runtime. Instead, use the newly added granular check
> that can test for the presence of each EFI runtime service
> individually.
> 
> Cc: James Morris <jmorris@namei.org>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: linux-security-module@vger.kernel.org
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  security/integrity/platform_certs/load_uefi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
> index 111898aad56e..e2fe1bd3abb9 100644
> --- a/security/integrity/platform_certs/load_uefi.c
> +++ b/security/integrity/platform_certs/load_uefi.c
> @@ -76,7 +76,7 @@ static int __init load_uefi_certs(void)
>  	unsigned long dbsize = 0, dbxsize = 0, moksize = 0;
>  	int rc = 0;
>  
> -	if (!efi.get_variable)
> +	if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))

Sorry, where is this defined?

>  		return false;
>  
>  	/* Get db, MokListRT, and dbx.  They might not exist, so it isn't
> -- 
> 2.17.1

Re: [PATCH 9/9] integrity: check properly whether EFI GetVariable() is available

[Ard Biesheuvel]

On Wed, 19 Feb 2020 at 21:46, Serge E. Hallyn <serge@hallyn.com> wrote:
>
> On Wed, Feb 19, 2020 at 06:19:07PM +0100, Ard Biesheuvel wrote:
> > Testing the value of the efi.get_variable function pointer is not
> > the right way to establish whether the platform supports EFI
> > variables at runtime. Instead, use the newly added granular check
> > that can test for the presence of each EFI runtime service
> > individually.
> >
> > Cc: James Morris <jmorris@namei.org>
> > Cc: "Serge E. Hallyn" <serge@hallyn.com>
> > Cc: linux-security-module@vger.kernel.org
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  security/integrity/platform_certs/load_uefi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
> > index 111898aad56e..e2fe1bd3abb9 100644
> > --- a/security/integrity/platform_certs/load_uefi.c
> > +++ b/security/integrity/platform_certs/load_uefi.c
> > @@ -76,7 +76,7 @@ static int __init load_uefi_certs(void)
> >       unsigned long dbsize = 0, dbxsize = 0, moksize = 0;
> >       int rc = 0;
> >
> > -     if (!efi.get_variable)
> > +     if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
>
> Sorry, where is this defined?
>

Apologies, I failed to cc everyone on the whole series.

It is defined in the first patch.

https://lore.kernel.org/linux-efi/20200219171907.11894-1-ardb@kernel.org/

Re: [PATCH 4/9] efi: register EFI rtc platform device only when available

[Alexandre Belloni]

On 19/02/2020 18:19:02+0100, Ard Biesheuvel wrote:
> Drop the separate driver that registers the EFI rtc on all EFI
> systems that have runtime services available, and instead, move
> the registration into the core EFI code, and make it conditional
> on whether the actual time related services are available.
> 
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: linux-rtc@vger.kernel.org
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  drivers/firmware/efi/efi.c     |  3 ++
>  drivers/rtc/Makefile           |  4 ---
>  drivers/rtc/rtc-efi-platform.c | 35 --------------------
>  3 files changed, 3 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index abf4c02e0201..69a585106d30 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -321,6 +321,9 @@ static int __init efisubsys_init(void)
>  		}
>  	}
>  
> +	if (efi_rt_services_supported(EFI_RT_SUPPORTED_TIME_SERVICES))
> +		platform_device_register_simple("rtc-efi", 0, NULL, 0);
> +
>  	/* We register the efi directory at /sys/firmware/efi */
>  	efi_kobj = kobject_create_and_add("efi", firmware_kobj);
>  	if (!efi_kobj) {
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 4ac8f19fb631..24c7dfa1bd7d 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -12,10 +12,6 @@ obj-$(CONFIG_RTC_CLASS)		+= rtc-core.o
>  obj-$(CONFIG_RTC_MC146818_LIB)	+= rtc-mc146818-lib.o
>  rtc-core-y			:= class.o interface.o
>  
> -ifdef CONFIG_RTC_DRV_EFI
> -rtc-core-y			+= rtc-efi-platform.o
> -endif
> -
>  rtc-core-$(CONFIG_RTC_NVMEM)		+= nvmem.o
>  rtc-core-$(CONFIG_RTC_INTF_DEV)		+= dev.o
>  rtc-core-$(CONFIG_RTC_INTF_PROC)	+= proc.o
> diff --git a/drivers/rtc/rtc-efi-platform.c b/drivers/rtc/rtc-efi-platform.c
> deleted file mode 100644
> index 6c037dc4e3dc..000000000000
> --- a/drivers/rtc/rtc-efi-platform.c
> +++ /dev/null
> @@ -1,35 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * Moved from arch/ia64/kernel/time.c
> - *
> - * Copyright (C) 1998-2003 Hewlett-Packard Co
> - *	Stephane Eranian <eranian@hpl.hp.com>
> - *	David Mosberger <davidm@hpl.hp.com>
> - * Copyright (C) 1999 Don Dugger <don.dugger@intel.com>
> - * Copyright (C) 1999-2000 VA Linux Systems
> - * Copyright (C) 1999-2000 Walt Drummond <drummond@valinux.com>
> - */
> -
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> -#include <linux/init.h>
> -#include <linux/kernel.h>
> -#include <linux/module.h>
> -#include <linux/efi.h>
> -#include <linux/platform_device.h>
> -
> -static struct platform_device rtc_efi_dev = {
> -	.name = "rtc-efi",
> -	.id = -1,
> -};
> -
> -static int __init rtc_init(void)
> -{
> -	if (efi_enabled(EFI_RUNTIME_SERVICES))
> -		if (platform_device_register(&rtc_efi_dev) < 0)
> -			pr_err("unable to register rtc device...\n");
> -
> -	/* not necessarily an error */
> -	return 0;
> -}
> -module_init(rtc_init);
> -- 
> 2.17.1
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 9/9] integrity: check properly whether EFI GetVariable() is available

[Serge E. Hallyn]

On Wed, Feb 19, 2020 at 10:00:11PM +0100, Ard Biesheuvel wrote:
> On Wed, 19 Feb 2020 at 21:46, Serge E. Hallyn <serge@hallyn.com> wrote:
> >
> > On Wed, Feb 19, 2020 at 06:19:07PM +0100, Ard Biesheuvel wrote:
> > > Testing the value of the efi.get_variable function pointer is not
> > > the right way to establish whether the platform supports EFI
> > > variables at runtime. Instead, use the newly added granular check
> > > that can test for the presence of each EFI runtime service
> > > individually.
> > >
> > > Cc: James Morris <jmorris@namei.org>
> > > Cc: "Serge E. Hallyn" <serge@hallyn.com>
> > > Cc: linux-security-module@vger.kernel.org
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > ---
> > >  security/integrity/platform_certs/load_uefi.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
> > > index 111898aad56e..e2fe1bd3abb9 100644
> > > --- a/security/integrity/platform_certs/load_uefi.c
> > > +++ b/security/integrity/platform_certs/load_uefi.c
> > > @@ -76,7 +76,7 @@ static int __init load_uefi_certs(void)
> > >       unsigned long dbsize = 0, dbxsize = 0, moksize = 0;
> > >       int rc = 0;
> > >
> > > -     if (!efi.get_variable)
> > > +     if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE))
> >
> > Sorry, where is this defined?
> >
> 
> Apologies, I failed to cc everyone on the whole series.
> 
> It is defined in the first patch.
> 
> https://lore.kernel.org/linux-efi/20200219171907.11894-1-ardb@kernel.org/

Gotcha, thanks, I shoulda get-lore-mbox'ed it :)

Acked-by: Serge Hallyn <serge@hallyn.com>

thanks,
-serge