Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64/efi: only disable the EFI runtime service that faulted
@ 2026-10-21 23:05 Hamza Mahfooz
  2026-10-22  6:05 ` Ard Biesheuvel
  0 siblings, 1 reply; 3+ messages in thread
From: Hamza Mahfooz @ 2026-10-21 23:05 UTC (permalink / raw)
  To: linux-efi
  Cc: Ard Biesheuvel, Ilias Apalodimas, Catalin Marinas, Will Deacon,
	Mark Rutland, Kees Cook, Arnd Bergmann, Greg Kroah-Hartman,
	linux-arm-kernel, Hamza Mahfooz

When the firmware faults while executing an EFI runtime service, arm64
recovers from the exception, returns EFI_ABORTED to the caller and
disables all EFI runtime services for the remainder of the boot.

That is a rather big hammer. For instance, on Microsoft C2141 and C4143
machines, GetTime() faults when it is called by the rtc-efi driver:

  [Firmware Bug]: Unable to handle paging request in EFI runtime service
  WARNING: CPU: 9 PID: 1 at drivers/firmware/efi/runtime-wrappers.c:498 __efi_queue_work+0xe8/0x128
  Hardware name: Microsoft C2141/C2141  , BIOS C2141.BS.0301.AG.01.03 08/21/2024
  Call trace:
   __efi_queue_work+0xe8/0x128
   virt_efi_get_time+0x64/0xc0
   efi_rtc_probe+0x4c/0x188
   platform_probe+0x70/0xd8
   really_probe+0x190/0x3d8
   ...

and as a result, the variable services become unavailable too, which
breaks efivarfs (and with it efibootmgr, fwupd etc.), even though there
is nothing wrong with them.

So, if the faulting call was made via efi_rts_wq, which is the case for
all runtime services apart from ResetSystem() and the non-blocking
variable services, only disable the service that faulted: clear its bit
in efi.runtime_supported_mask, so that callers that check for it via
efi_rt_services_supported() no longer try to use it, and fail any
further calls to it with EFI_DEVICE_ERROR, which is what all runtime
service calls return today once they have been disabled. Otherwise,
and for the ACPI PRM handlers and the LKDTM tests, which have no bit
of their own, keep disabling all runtime services.

Update the LKDTM EFI_RUNTIME_CRASH test accordingly, as the fault it
provokes in GetNextVariableName() now only disables that service.

Signed-off-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
---
 arch/arm64/kernel/efi.c                 |  2 +-
 drivers/firmware/efi/runtime-wrappers.c | 85 ++++++++++++++++++++++++-
 drivers/misc/lkdtm/bugs.c               |  4 +-
 include/linux/efi.h                     |  1 +
 4 files changed, 88 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 0ec90fd17..330909d56 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -227,7 +227,7 @@ bool efi_runtime_fixup_exception(struct pt_regs *regs, const char *msg)
 
 	pr_err(FW_BUG "Unable to handle %s in EFI runtime service\n", msg);
 	add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
-	clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
+	efi_runtime_disable_faulting_service();
 
 	regs->regs[0]	= EFI_ABORTED;
 	regs->regs[30]	= efi_rt_stack_top[-1];
diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index eea4f32e6..221196c95 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -132,6 +132,82 @@ struct efi_runtime_work efi_rts_work;
  */
 static bool efi_rts_abandoned;
 
+/*
+ * The EFI runtime services that are called via efi_rts_wq, and their bits in
+ * efi.runtime_supported_mask.
+ */
+static const struct {
+	unsigned int	mask;
+	const char	*name;
+} efi_rts_services[] = {
+	[EFI_GET_TIME]			= { EFI_RT_SUPPORTED_GET_TIME,
+					    "GetTime" },
+	[EFI_SET_TIME]			= { EFI_RT_SUPPORTED_SET_TIME,
+					    "SetTime" },
+	[EFI_GET_WAKEUP_TIME]		= { EFI_RT_SUPPORTED_GET_WAKEUP_TIME,
+					    "GetWakeupTime" },
+	[EFI_SET_WAKEUP_TIME]		= { EFI_RT_SUPPORTED_SET_WAKEUP_TIME,
+					    "SetWakeupTime" },
+	[EFI_GET_VARIABLE]		= { EFI_RT_SUPPORTED_GET_VARIABLE,
+					    "GetVariable" },
+	[EFI_GET_NEXT_VARIABLE]		= { EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME,
+					    "GetNextVariableName" },
+	[EFI_SET_VARIABLE]		= { EFI_RT_SUPPORTED_SET_VARIABLE,
+					    "SetVariable" },
+	[EFI_QUERY_VARIABLE_INFO]	= { EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO,
+					    "QueryVariableInfo" },
+	[EFI_GET_NEXT_HIGH_MONO_COUNT]	= { EFI_RT_SUPPORTED_GET_NEXT_HIGH_MONOTONIC_COUNT,
+					    "GetNextHighMonotonicCount" },
+	[EFI_UPDATE_CAPSULE]		= { EFI_RT_SUPPORTED_UPDATE_CAPSULE,
+					    "UpdateCapsule" },
+	[EFI_QUERY_CAPSULE_CAPS]	= { EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES,
+					    "QueryCapsuleCapabilities" },
+};
+
+/*
+ * The EFI runtime services that have been disabled because they faulted. This
+ * is tracked separately from efi.runtime_supported_mask, so that services that
+ * the firmware reports as unsupported can still be called.
+ */
+static unsigned int efi_rts_faulted;
+
+static unsigned int efi_rts_mask(enum efi_rts_ids id)
+{
+	return id < ARRAY_SIZE(efi_rts_services) ? efi_rts_services[id].mask : 0;
+}
+
+static bool efi_rts_service_faulted(enum efi_rts_ids id)
+{
+	return efi_rts_faulted & efi_rts_mask(id);
+}
+
+/**
+ * efi_runtime_disable_faulting_service() - disable a faulting runtime service
+ *
+ * Called by architecture code after it has recovered from an exception taken
+ * by the firmware while executing an EFI runtime service. If the call was made
+ * via efi_rts_wq, only the service that faulted is disabled, so that the other
+ * ones remain usable. Otherwise, all EFI runtime services are disabled.
+ */
+void efi_runtime_disable_faulting_service(void)
+{
+	enum efi_rts_ids id = efi_rts_work.efi_rts_id;
+	unsigned int mask = 0;
+
+	if (current_work() == &efi_rts_work.work)
+		mask = efi_rts_mask(id);
+
+	if (!mask) {
+		pr_err("Disabling EFI runtime services\n");
+		clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
+		return;
+	}
+
+	pr_err("Disabling EFI runtime service %s()\n", efi_rts_services[id].name);
+	efi_rts_faulted |= mask;
+	efi.runtime_supported_mask &= ~mask;
+}
+
 /*
  * efi_queue_work:	Queue EFI runtime service call and wait for completion
  * @_rts:		EFI runtime service function identifier
@@ -388,6 +464,9 @@ static efi_status_t __efi_queue_work(enum efi_rts_ids id,
 		return EFI_DEVICE_ERROR;
 	}
 
+	if (efi_rts_service_faulted(id))
+		return EFI_DEVICE_ERROR;
+
 	efi_rts_work.efi_rts_id = id;
 	efi_rts_work.args = args;
 	efi_rts_work.caller = __builtin_return_address(0);
@@ -520,7 +599,8 @@ virt_efi_set_variable_nb(efi_char16_t *name, efi_guid_t *vendor, u32 attr,
 	if (down_trylock(&efi_runtime_lock))
 		return EFI_NOT_READY;
 
-	if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
+	if (!efi_enabled(EFI_RUNTIME_SERVICES) ||
+	    efi_rts_service_faulted(EFI_SET_VARIABLE)) {
 		up(&efi_runtime_lock);
 		return EFI_DEVICE_ERROR;
 	}
@@ -564,7 +644,8 @@ virt_efi_query_variable_info_nb(u32 attr, u64 *storage_space,
 	if (down_trylock(&efi_runtime_lock))
 		return EFI_NOT_READY;
 
-	if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
+	if (!efi_enabled(EFI_RUNTIME_SERVICES) ||
+	    efi_rts_service_faulted(EFI_QUERY_VARIABLE_INFO)) {
 		up(&efi_runtime_lock);
 		return EFI_DEVICE_ERROR;
 	}
diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
index 8ed368ea3..fec6eaca2 100644
--- a/drivers/misc/lkdtm/bugs.c
+++ b/drivers/misc/lkdtm/bugs.c
@@ -836,7 +836,9 @@ static void __maybe_unused lkdtm_EFI_RUNTIME_CRASH(void)
 	 */
 	status = efi.get_next_variable(&size, L"", &(efi_guid_t){});
 
-	if (status != EFI_ABORTED || efi_enabled(EFI_RUNTIME_SERVICES))
+	if (status != EFI_ABORTED ||
+	    (efi_enabled(EFI_RUNTIME_SERVICES) &&
+	     efi_rt_services_supported(EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME)))
 		pr_err("FAIL: EFI GetNextVariable() did not abort (%#lx)\n",
 		       status);
 }
diff --git a/include/linux/efi.h b/include/linux/efi.h
index cc1df1577..12d3e4a07 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1139,6 +1139,7 @@ extern unsigned long efi_call_virt_save_flags(void);
 
 void efi_runtime_assert_lock_held(void);
 efi_status_t efi_runtime_set_enable_flag(bool enable);
+void efi_runtime_disable_faulting_service(void);
 
 enum efi_secureboot_mode {
 	efi_secureboot_mode_unset,
-- 
2.56.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] arm64/efi: only disable the EFI runtime service that faulted
  2026-10-21 23:05 [PATCH] arm64/efi: only disable the EFI runtime service that faulted Hamza Mahfooz
@ 2026-10-22  6:05 ` Ard Biesheuvel
  2026-10-22 20:30   ` Hamza Mahfooz
  0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2026-10-22  6:05 UTC (permalink / raw)
  To: Hamza Mahfooz, linux-efi
  Cc: Ilias Apalodimas, Catalin Marinas, Will Deacon, Mark Rutland,
	Kees Cook, Arnd Bergmann, Greg Kroah-Hartman, linux-arm-kernel

Hello Hamza,

On Thu, 22 Oct 2026, at 01:05, Hamza Mahfooz wrote:
> When the firmware faults while executing an EFI runtime service, arm64
> recovers from the exception, returns EFI_ABORTED to the caller and
> disables all EFI runtime services for the remainder of the boot.
>
> That is a rather big hammer.

It does not just return EFI_ABORTED to the caller. It basically does a
longjmp() out of the faulting context straight back to the caller in
the kernel, skipping all execution that would have occurred on the
way back out if the fault had not occurred.

Assuming that the runtime services are still in a sane state after this
is not justified, so I don't think making this change is a good idea.

Do the systems in question expose the RTC via the standardized ACPI
methods as well? I'd be fine with some logic that avoids rtc-efi
entirely if the RTC can be driven by a native ACPI driver: that
seems a worthwhile improvement in its own right, given that EFI
runtime services are evil and should be avoided where possible.


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] arm64/efi: only disable the EFI runtime service that faulted
  2026-10-22  6:05 ` Ard Biesheuvel
@ 2026-10-22 20:30   ` Hamza Mahfooz
  0 siblings, 0 replies; 3+ messages in thread
From: Hamza Mahfooz @ 2026-10-22 20:30 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, Ilias Apalodimas, Catalin Marinas, Will Deacon,
	Mark Rutland, Kees Cook, Arnd Bergmann, Greg Kroah-Hartman,
	linux-arm-kernel

On Thu, Oct 22, 2026 at 08:05:17AM +0200, Ard Biesheuvel wrote:
> Hello Hamza,
> 
> On Thu, 22 Oct 2026, at 01:05, Hamza Mahfooz wrote:
> > When the firmware faults while executing an EFI runtime service, arm64
> > recovers from the exception, returns EFI_ABORTED to the caller and
> > disables all EFI runtime services for the remainder of the boot.
> >
> > That is a rather big hammer.
> 
> It does not just return EFI_ABORTED to the caller. It basically does a
> longjmp() out of the faulting context straight back to the caller in
> the kernel, skipping all execution that would have occurred on the
> way back out if the fault had not occurred.
> 
> Assuming that the runtime services are still in a sane state after this
> is not justified, so I don't think making this change is a good idea.
> 
> Do the systems in question expose the RTC via the standardized ACPI
> methods as well? I'd be fine with some logic that avoids rtc-efi

Ya the machines export ACPI000E devices and acpi-tad is able to
register them (as /dev/rtc0). I'll send out a series that I've tested
that implements that idea soon.

> entirely if the RTC can be driven by a native ACPI driver: that
> seems a worthwhile improvement in its own right, given that EFI
> runtime services are evil and should be avoided where possible.


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-22 20:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-21 23:05 [PATCH] arm64/efi: only disable the EFI runtime service that faulted Hamza Mahfooz
2026-10-22  6:05 ` Ard Biesheuvel
2026-10-22 20:30   ` Hamza Mahfooz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox