All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] efi/runtime-wrappers: bound the wait for EFI runtime service calls
@ 2026-06-12 11:01 Breno Leitao
  2026-06-12 11:01 ` [PATCH v2 1/6] efi: fix stale reference to efi_recover_from_page_fault() Breno Leitao
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Breno Leitao @ 2026-06-12 11:01 UTC (permalink / raw)
  To: Ard Biesheuvel, Ilias Apalodimas, Borislav Petkov,
	Andy Lutomirski, Kees Cook, Tony Luck, Guilherme G. Piccoli,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin
  Cc: linux-efi, linux-kernel, Breno Leitao, kernel-team

When an EFI runtime service call hangs in firmware, the kworker on
efi_rts_wq is stuck inside the firmware call and cannot be cancelled.
The kernel currently waits indefinitely on the completion, and the
caller holds efi_runtime_lock for the duration, so every subsequent
EFI runtime caller (efivarfs, NVRAM writes, set_wakeup_time, ACPI PRM
handlers, ...) is wedged until reboot. The only externally visible
symptom is a "workqueue lockup" message and userspace processes
piling up uninterruptibly on the semaphore.

A real example from one of our NVIDIA Grace hosts:

  BUG: workqueue lockup - pool cpus=28 node=0 flags=0x0 nice=0 stuck for 127s!
  ...
  CPU: 28 PID: 590 Comm: kworker/u288:6
  Workqueue: efi_rts_wq efi_call_rts
  Call trace:
   0x4052f11ecc (P)
   0x4052f10ed4
   ...
   __efi_rt_asm_wrapper+0x50/0x78
   efi_call_rts+0x178/0x240
   process_scheduled_works+0x17c/0x420
   worker_thread+0x184/0x4d8
   kthread+0xcc/0x1f8
   ret_from_fork+0x10/0x20

PC and LR are inside EFI runtime services firmware memory; firmware
never returned; the worker stayed stuck across the 127s / 157s / 188s
"workqueue lockup" reports until external monitoring eventually rebooted
the host.

This series doesn't fix the firmware bug - that's vendor territory -
but it stops one stuck EFI call from taking the rest of userspace
down with it, and turns a generic stalled-task mystery into an
unambiguous "EFI firmware is at fault" signal in dmesg, which is
especially valuable at fleet scale where the same symptom could
otherwise be attributed to dozens of unrelated stalls.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
Changes in v2:
- Drop v1's efi_rts_dead flag; reuse the existing EFI_RUNTIME_SERVICES bit
(cleared on timeout) and return EFI_ABORTED instead of EFI_TIMEOUT (per Ard). 
- Also guard the non-blocking paths (set_variable/query_variable_info/reset_system)
 and park the leaked worker via a shared efi_rts_park_worker() reused by x86's
page-fault handler;
- Split into smaller prep patches.
- Link to v1: https://lore.kernel.org/r/20260609-efi_timeout-v1-0-69a896faa805@debian.org

---
Breno Leitao (6):
      efi: fix stale reference to efi_recover_from_page_fault()
      efi/runtime-wrappers: handle queue_work() failure with goto exit
      efi/runtime-wrappers: check EFI_RUNTIME_SERVICES before using efi_rts_work
      efi/runtime-wrappers: bound the wait for EFI runtime service calls
      efi/runtime-wrappers: honour EFI_RUNTIME_SERVICES in the non-blocking paths
      efi/runtime-wrappers: retire the worker if a wedged call ever returns

 arch/x86/platform/efi/quirks.c          |  9 +----
 drivers/firmware/efi/runtime-wrappers.c | 65 ++++++++++++++++++++++++++++-----
 include/linux/efi.h                     |  6 ++-
 3 files changed, 61 insertions(+), 19 deletions(-)
---
base-commit: a87737435cfa134f9cdcc696ba3080759d04cf72
change-id: 20260609-efi_timeout-6f51d5bbcfb7

Best regards,
-- 
Breno Leitao <leitao@debian.org>


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

* [PATCH v2 1/6] efi: fix stale reference to efi_recover_from_page_fault()
  2026-06-12 11:01 [PATCH v2 0/6] efi/runtime-wrappers: bound the wait for EFI runtime service calls Breno Leitao
@ 2026-06-12 11:01 ` Breno Leitao
  2026-06-12 11:01 ` [PATCH v2 2/6] efi/runtime-wrappers: handle queue_work() failure with goto exit Breno Leitao
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Breno Leitao @ 2026-06-12 11:01 UTC (permalink / raw)
  To: Ard Biesheuvel, Ilias Apalodimas, Borislav Petkov,
	Andy Lutomirski, Kees Cook, Tony Luck, Guilherme G. Piccoli,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin
  Cc: linux-efi, linux-kernel, Breno Leitao, kernel-team

efi_recover_from_page_fault() was renamed to
efi_crash_gracefully_on_page_fault(), but the comment above enum
efi_rts_ids was not updated. Use the current name.

Fixes: c46f52231e79 ("x86/{fault,efi}: Fix and rename efi_recover_from_page_fault()")
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/linux/efi.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index ccbc35479684a..24221a8424121 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1212,8 +1212,8 @@ efi_call_acpi_prm_handler(efi_status_t (__efiapi *handler_addr)(u64, void *),
 
 /*
  * efi_runtime_service() function identifiers.
- * "NONE" is used by efi_recover_from_page_fault() to check if the page
- * fault happened while executing an efi runtime service.
+ * "NONE" is used by efi_crash_gracefully_on_page_fault() to check if the
+ * page fault happened while executing an efi runtime service.
  */
 enum efi_rts_ids {
 	EFI_NONE,

-- 
2.53.0-Meta


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

* [PATCH v2 2/6] efi/runtime-wrappers: handle queue_work() failure with goto exit
  2026-06-12 11:01 [PATCH v2 0/6] efi/runtime-wrappers: bound the wait for EFI runtime service calls Breno Leitao
  2026-06-12 11:01 ` [PATCH v2 1/6] efi: fix stale reference to efi_recover_from_page_fault() Breno Leitao
@ 2026-06-12 11:01 ` Breno Leitao
  2026-06-12 11:01 ` [PATCH v2 3/6] efi/runtime-wrappers: check EFI_RUNTIME_SERVICES before using efi_rts_work Breno Leitao
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Breno Leitao @ 2026-06-12 11:01 UTC (permalink / raw)
  To: Ard Biesheuvel, Ilias Apalodimas, Borislav Petkov,
	Andy Lutomirski, Kees Cook, Tony Luck, Guilherme G. Piccoli,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin
  Cc: linux-efi, linux-kernel, Breno Leitao, kernel-team

Convert the queue_work() failure path in __efi_queue_work() to a
goto exit instead of falling through to the wait and the
WARN_ON_ONCE(status == EFI_ABORTED) below it. A failed queue_work()
leaves the status at its initial EFI_ABORTED, so that warning would
fire even though no call ran; it is meant for a completed call that
returned EFI_ABORTED.

No change for the common (successful enqueue) path. This also prepares
__efi_queue_work() for the timeout handling added later.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/firmware/efi/runtime-wrappers.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index da8d296216441..ebeec87ed0b0a 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -338,10 +338,12 @@ static efi_status_t __efi_queue_work(enum efi_rts_ids id,
 	 * queue_work() returns 0 if work was already on queue,
 	 * _ideally_ this should never happen.
 	 */
-	if (queue_work(efi_rts_wq, &efi_rts_work.work))
-		wait_for_completion(&efi_rts_work.efi_rts_comp);
-	else
+	if (!queue_work(efi_rts_wq, &efi_rts_work.work)) {
 		pr_err("Failed to queue work to efi_rts_wq.\n");
+		goto exit;
+	}
+
+	wait_for_completion(&efi_rts_work.efi_rts_comp);
 
 	WARN_ON_ONCE(efi_rts_work.status == EFI_ABORTED);
 exit:

-- 
2.53.0-Meta


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

* [PATCH v2 3/6] efi/runtime-wrappers: check EFI_RUNTIME_SERVICES before using efi_rts_work
  2026-06-12 11:01 [PATCH v2 0/6] efi/runtime-wrappers: bound the wait for EFI runtime service calls Breno Leitao
  2026-06-12 11:01 ` [PATCH v2 1/6] efi: fix stale reference to efi_recover_from_page_fault() Breno Leitao
  2026-06-12 11:01 ` [PATCH v2 2/6] efi/runtime-wrappers: handle queue_work() failure with goto exit Breno Leitao
@ 2026-06-12 11:01 ` Breno Leitao
  2026-06-12 11:01 ` [PATCH v2 4/6] efi/runtime-wrappers: bound the wait for EFI runtime service calls Breno Leitao
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Breno Leitao @ 2026-06-12 11:01 UTC (permalink / raw)
  To: Ard Biesheuvel, Ilias Apalodimas, Borislav Petkov,
	Andy Lutomirski, Kees Cook, Tony Luck, Guilherme G. Piccoli,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin
  Cc: linux-efi, linux-kernel, Breno Leitao, kernel-team

Move the EFI_RUNTIME_SERVICES check to the top of __efi_queue_work() and
return directly, so a caller that finds runtime services disabled returns
without touching the shared efi_rts_work. No functional change.

This prepares for bounding the wait, where a timeout will clear
EFI_RUNTIME_SERVICES while the leaked worker still owns efi_rts_work; a
later caller must then bail out before reinitialising it.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/firmware/efi/runtime-wrappers.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index ebeec87ed0b0a..0cd350760446c 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -320,17 +320,16 @@ static void __nocfi efi_call_rts(struct work_struct *work)
 static efi_status_t __efi_queue_work(enum efi_rts_ids id,
 				     union efi_rts_args *args)
 {
+	if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
+		pr_warn_once("EFI Runtime Services are disabled!\n");
+		return EFI_DEVICE_ERROR;
+	}
+
 	efi_rts_work.efi_rts_id = id;
 	efi_rts_work.args = args;
 	efi_rts_work.caller = __builtin_return_address(0);
 	efi_rts_work.status = EFI_ABORTED;
 
-	if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
-		pr_warn_once("EFI Runtime Services are disabled!\n");
-		efi_rts_work.status = EFI_DEVICE_ERROR;
-		goto exit;
-	}
-
 	init_completion(&efi_rts_work.efi_rts_comp);
 	INIT_WORK(&efi_rts_work.work, efi_call_rts);
 

-- 
2.53.0-Meta


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

* [PATCH v2 4/6] efi/runtime-wrappers: bound the wait for EFI runtime service calls
  2026-06-12 11:01 [PATCH v2 0/6] efi/runtime-wrappers: bound the wait for EFI runtime service calls Breno Leitao
                   ` (2 preceding siblings ...)
  2026-06-12 11:01 ` [PATCH v2 3/6] efi/runtime-wrappers: check EFI_RUNTIME_SERVICES before using efi_rts_work Breno Leitao
@ 2026-06-12 11:01 ` Breno Leitao
  2026-06-12 11:01 ` [PATCH v2 5/6] efi/runtime-wrappers: honour EFI_RUNTIME_SERVICES in the non-blocking paths Breno Leitao
  2026-06-12 11:01 ` [PATCH v2 6/6] efi/runtime-wrappers: retire the worker if a wedged call ever returns Breno Leitao
  5 siblings, 0 replies; 10+ messages in thread
From: Breno Leitao @ 2026-06-12 11:01 UTC (permalink / raw)
  To: Ard Biesheuvel, Ilias Apalodimas, Borislav Petkov,
	Andy Lutomirski, Kees Cook, Tony Luck, Guilherme G. Piccoli,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin
  Cc: linux-efi, linux-kernel, Breno Leitao, kernel-team

When an EFI runtime service hangs in firmware, the efi_rts_wq worker is
stuck inside the call and cannot be cancelled. __efi_queue_work() then
waits on the completion forever while holding efi_runtime_lock, so every
later EFI caller is wedged until reboot; the only symptom is a "workqueue
lockup" and tasks piling up on the semaphore.

Replace wait_for_completion() with wait_for_completion_timeout() bounded
by EFI_RTS_TIMEOUT (120 seconds). On timeout, clear EFI_RUNTIME_SERVICES
and return EFI_ABORTED so later callers fail fast at the entry check
instead of each paying another 120 seconds. The wedged worker is
intentionally leaked and keeps ownership of efi_rts_work.

Known limitation: the efi_rts_args the worker holds points into the
caller's stack frame; if firmware unblocks after the timeout and writes
the output buffers, they land in reused memory. Firmware hung this long
rarely recovers; a follow-up could bounce the buffers through kmalloc.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/firmware/efi/runtime-wrappers.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index 0cd350760446c..8badf0419a148 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -118,6 +118,14 @@ union efi_rts_args {
 
 struct efi_runtime_work efi_rts_work;
 
+/*
+ * Upper bound on how long we wait for a single EFI runtime service
+ * call to finish before declaring firmware wedged. Chosen to be longer
+ * than any plausible legitimate call (including UpdateCapsule on slow
+ * SPI-NOR) while still bounding userspace wait time.
+ */
+#define EFI_RTS_TIMEOUT		(120 * HZ)
+
 /*
  * efi_queue_work:	Queue EFI runtime service call and wait for completion
  * @_rts:		EFI runtime service function identifier
@@ -342,7 +350,13 @@ static efi_status_t __efi_queue_work(enum efi_rts_ids id,
 		goto exit;
 	}
 
-	wait_for_completion(&efi_rts_work.efi_rts_comp);
+	if (!wait_for_completion_timeout(&efi_rts_work.efi_rts_comp,
+					 EFI_RTS_TIMEOUT)) {
+		pr_err("EFI runtime service %d wedged in firmware; disabling EFI runtime services\n",
+		       id);
+		clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
+		return EFI_ABORTED;
+	}
 
 	WARN_ON_ONCE(efi_rts_work.status == EFI_ABORTED);
 exit:

-- 
2.53.0-Meta


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

* [PATCH v2 5/6] efi/runtime-wrappers: honour EFI_RUNTIME_SERVICES in the non-blocking paths
  2026-06-12 11:01 [PATCH v2 0/6] efi/runtime-wrappers: bound the wait for EFI runtime service calls Breno Leitao
                   ` (3 preceding siblings ...)
  2026-06-12 11:01 ` [PATCH v2 4/6] efi/runtime-wrappers: bound the wait for EFI runtime service calls Breno Leitao
@ 2026-06-12 11:01 ` Breno Leitao
  2026-06-16 12:10   ` Breno Leitao
  2026-06-12 11:01 ` [PATCH v2 6/6] efi/runtime-wrappers: retire the worker if a wedged call ever returns Breno Leitao
  5 siblings, 1 reply; 10+ messages in thread
From: Breno Leitao @ 2026-06-12 11:01 UTC (permalink / raw)
  To: Ard Biesheuvel, Ilias Apalodimas, Borislav Petkov,
	Andy Lutomirski, Kees Cook, Tony Luck, Guilherme G. Piccoli,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin
  Cc: linux-efi, linux-kernel, Breno Leitao, kernel-team

Three wrappers call firmware directly instead of going through
__efi_queue_work(), and none of them check whether runtime services are
still enabled: virt_efi_set_variable_nb(),
virt_efi_query_variable_info_nb() and virt_efi_reset_system(). Once a
hang has cleared EFI_RUNTIME_SERVICES - or efi_recover_from_page_fault()
has cleared it on a firmware page fault - these paths still enter the
(possibly wedged) firmware, e.g. an EFI pstore write through the
non-blocking SetVariable() variant, in violation of UEFI's
non-reentrancy rules. reset_system() is reachable too: efi_reboot()
only gates it on the static efi_rt_services_supported() mask, which does
not track the runtime disable.

Check efi_enabled(EFI_RUNTIME_SERVICES) at the top of each before taking
efi_runtime_lock and calling into firmware.

Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/firmware/efi/runtime-wrappers.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index 8badf0419a148..842f72d44211f 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -461,6 +461,9 @@ virt_efi_set_variable_nb(efi_char16_t *name, efi_guid_t *vendor, u32 attr,
 {
 	efi_status_t status;
 
+	if (!efi_enabled(EFI_RUNTIME_SERVICES))
+		return EFI_DEVICE_ERROR;
+
 	if (down_trylock(&efi_runtime_lock))
 		return EFI_NOT_READY;
 
@@ -500,6 +503,9 @@ virt_efi_query_variable_info_nb(u32 attr, u64 *storage_space,
 	if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION)
 		return EFI_UNSUPPORTED;
 
+	if (!efi_enabled(EFI_RUNTIME_SERVICES))
+		return EFI_DEVICE_ERROR;
+
 	if (down_trylock(&efi_runtime_lock))
 		return EFI_NOT_READY;
 
@@ -527,6 +533,11 @@ static void __nocfi
 virt_efi_reset_system(int reset_type, efi_status_t status,
 		      unsigned long data_size, efi_char16_t *data)
 {
+	if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
+		pr_warn("EFI Runtime Services are disabled, not invoking reset_system()\n");
+		return;
+	}
+
 	if (down_trylock(&efi_runtime_lock)) {
 		pr_warn("failed to invoke the reset_system() runtime service:\n"
 			"could not get exclusive access to the firmware\n");

-- 
2.53.0-Meta


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

* [PATCH v2 6/6] efi/runtime-wrappers: retire the worker if a wedged call ever returns
  2026-06-12 11:01 [PATCH v2 0/6] efi/runtime-wrappers: bound the wait for EFI runtime service calls Breno Leitao
                   ` (4 preceding siblings ...)
  2026-06-12 11:01 ` [PATCH v2 5/6] efi/runtime-wrappers: honour EFI_RUNTIME_SERVICES in the non-blocking paths Breno Leitao
@ 2026-06-12 11:01 ` Breno Leitao
  2026-06-12 11:11   ` Ard Biesheuvel
  5 siblings, 1 reply; 10+ messages in thread
From: Breno Leitao @ 2026-06-12 11:01 UTC (permalink / raw)
  To: Ard Biesheuvel, Ilias Apalodimas, Borislav Petkov,
	Andy Lutomirski, Kees Cook, Tony Luck, Guilherme G. Piccoli,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin
  Cc: linux-efi, linux-kernel, Breno Leitao, kernel-team

When __efi_queue_work() times out it disables runtime services and
returns, but the kworker is still blocked inside firmware. If the
firmware eventually unblocks, efi_call_rts() would run its tail on an
efi_rts_work that the timed-out caller has long abandoned: signalling a
stale completion and clearing efi_runtime_lock_owner that may by then
belong to another caller.

If runtime services have been disabled by the time the call returns,
park the worker in a schedule() loop instead, so it never touches
efi_rts_work again or returns to the workqueue.

x86's efi_crash_gracefully_on_page_fault() already ends in the same loop
for the page-fault case. Factor it into efi_rts_park_worker() and call it
from both paths.

Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/x86/platform/efi/quirks.c          |  9 +--------
 drivers/firmware/efi/runtime-wrappers.c | 21 +++++++++++++++++++++
 include/linux/efi.h                     |  2 ++
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 90a065fcb1fab..02c56a02eb9bc 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -832,12 +832,5 @@ void efi_crash_gracefully_on_page_fault(unsigned long phys_addr,
 	clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
 	pr_info("Froze efi_rts_wq and disabled EFI Runtime Services\n");
 
-	/*
-	 * Call schedule() in an infinite loop, so that any spurious wake ups
-	 * will never run efi_rts_wq again.
-	 */
-	for (;;) {
-		set_current_state(TASK_IDLE);
-		schedule();
-	}
+	efi_rts_park_worker();
 }
diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index 842f72d44211f..998e5f8f1c62c 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -219,6 +219,19 @@ static struct task_struct *efi_runtime_lock_owner;
 extern struct semaphore __efi_uv_runtime_lock __alias(efi_runtime_lock);
 #endif
 
+/*
+ * Park a worker that must never run efi_rts_wq again: EFI runtime services
+ * have been disabled and its efi_rts_work is abandoned. Loop in schedule()
+ * so a spurious wakeup cannot resume it.
+ */
+void efi_rts_park_worker(void)
+{
+	for (;;) {
+		set_current_state(TASK_IDLE);
+		schedule();
+	}
+}
+
 /*
  * Calls the appropriate efi_runtime_service() with the appropriate
  * arguments.
@@ -320,6 +333,14 @@ static void __nocfi efi_call_rts(struct work_struct *work)
 	efi_call_virt_check_flags(flags, efi_rts_work.caller);
 	arch_efi_call_virt_teardown();
 
+	/*
+	 * If __efi_queue_work() timed out and disabled runtime services, the
+	 * caller is gone and efi_rts_work is no longer ours: park the worker
+	 * so it never signals the stale completion or runs again.
+	 */
+	if (!efi_enabled(EFI_RUNTIME_SERVICES))
+		efi_rts_park_worker();
+
 	efi_rts_work.status = status;
 	complete(&efi_rts_work.efi_rts_comp);
 	efi_runtime_lock_owner = NULL;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 24221a8424121..015505423277e 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1256,6 +1256,8 @@ extern struct efi_runtime_work efi_rts_work;
 /* Workqueue to queue EFI Runtime Services */
 extern struct workqueue_struct *efi_rts_wq;
 
+void efi_rts_park_worker(void);
+
 struct linux_efi_memreserve {
 	int		size;			// allocated size of the array
 	atomic_t	count;			// number of entries used

-- 
2.53.0-Meta


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

* Re: [PATCH v2 6/6] efi/runtime-wrappers: retire the worker if a wedged call ever returns
  2026-06-12 11:01 ` [PATCH v2 6/6] efi/runtime-wrappers: retire the worker if a wedged call ever returns Breno Leitao
@ 2026-06-12 11:11   ` Ard Biesheuvel
  2026-06-16 10:13     ` Breno Leitao
  0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2026-06-12 11:11 UTC (permalink / raw)
  To: Breno Leitao, Ilias Apalodimas, Borislav Petkov, Andy Lutomirski,
	Kees Cook, Tony Luck, Guilherme G. Piccoli, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H . Peter Anvin
  Cc: linux-efi, linux-kernel, kernel-team

Hi,

On Fri, 12 Jun 2026, at 13:01, Breno Leitao wrote:
> When __efi_queue_work() times out it disables runtime services and
> returns, but the kworker is still blocked inside firmware. If the
> firmware eventually unblocks, efi_call_rts() would run its tail on an
> efi_rts_work that the timed-out caller has long abandoned: signalling a
> stale completion and clearing efi_runtime_lock_owner that may by then
> belong to another caller.
>
> If runtime services have been disabled by the time the call returns,
> park the worker in a schedule() loop instead, so it never touches
> efi_rts_work again or returns to the workqueue.
>
> x86's efi_crash_gracefully_on_page_fault() already ends in the same loop
> for the page-fault case. Factor it into efi_rts_park_worker() and call it
> from both paths.
>
> Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  arch/x86/platform/efi/quirks.c          |  9 +--------
>  drivers/firmware/efi/runtime-wrappers.c | 21 +++++++++++++++++++++
>  include/linux/efi.h                     |  2 ++
>  3 files changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/platform/efi/quirks.c 
> b/arch/x86/platform/efi/quirks.c
> index 90a065fcb1fab..02c56a02eb9bc 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -832,12 +832,5 @@ void efi_crash_gracefully_on_page_fault(unsigned 
> long phys_addr,
>  	clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
>  	pr_info("Froze efi_rts_wq and disabled EFI Runtime Services\n");
> 
> -	/*
> -	 * Call schedule() in an infinite loop, so that any spurious wake ups
> -	 * will never run efi_rts_wq again.
> -	 */
> -	for (;;) {
> -		set_current_state(TASK_IDLE);
> -		schedule();
> -	}
> +	efi_rts_park_worker();
>  }
> diff --git a/drivers/firmware/efi/runtime-wrappers.c 
> b/drivers/firmware/efi/runtime-wrappers.c
> index 842f72d44211f..998e5f8f1c62c 100644
> --- a/drivers/firmware/efi/runtime-wrappers.c
> +++ b/drivers/firmware/efi/runtime-wrappers.c
> @@ -219,6 +219,19 @@ static struct task_struct *efi_runtime_lock_owner;
>  extern struct semaphore __efi_uv_runtime_lock 
> __alias(efi_runtime_lock);
>  #endif
> 
> +/*
> + * Park a worker that must never run efi_rts_wq again: EFI runtime services
> + * have been disabled and its efi_rts_work is abandoned. Loop in schedule()
> + * so a spurious wakeup cannot resume it.
> + */
> +void efi_rts_park_worker(void)
> +{
> +	for (;;) {
> +		set_current_state(TASK_IDLE);
> +		schedule();
> +	}
> +}
> +
>  /*
>   * Calls the appropriate efi_runtime_service() with the appropriate
>   * arguments.
> @@ -320,6 +333,14 @@ static void __nocfi efi_call_rts(struct work_struct *work)
>  	efi_call_virt_check_flags(flags, efi_rts_work.caller);
>  	arch_efi_call_virt_teardown();
> 
> +	/*
> +	 * If __efi_queue_work() timed out and disabled runtime services, the
> +	 * caller is gone and efi_rts_work is no longer ours: park the worker
> +	 * so it never signals the stale completion or runs again.
> +	 */
> +	if (!efi_enabled(EFI_RUNTIME_SERVICES))
> +		efi_rts_park_worker();
> +

So one thing to note here is that the arm64 version of the exception recovery
(in efi_runtime_fixup_exception()) will also clear EFI_RUNTIME_SERVICES, and
that will occur before this check. This means we will park the worker not only
on a time out, but also on an sync exception in the firmware.

I suspect this is actually what we want, but it deserves to be called out.


>  	efi_rts_work.status = status;
>  	complete(&efi_rts_work.efi_rts_comp);
>  	efi_runtime_lock_owner = NULL;
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 24221a8424121..015505423277e 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1256,6 +1256,8 @@ extern struct efi_runtime_work efi_rts_work;
>  /* Workqueue to queue EFI Runtime Services */
>  extern struct workqueue_struct *efi_rts_wq;
> 
> +void efi_rts_park_worker(void);
> +
>  struct linux_efi_memreserve {
>  	int		size;			// allocated size of the array
>  	atomic_t	count;			// number of entries used
>
> -- 
> 2.53.0-Meta

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

* Re: [PATCH v2 6/6] efi/runtime-wrappers: retire the worker if a wedged call ever returns
  2026-06-12 11:11   ` Ard Biesheuvel
@ 2026-06-16 10:13     ` Breno Leitao
  0 siblings, 0 replies; 10+ messages in thread
From: Breno Leitao @ 2026-06-16 10:13 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Ilias Apalodimas, Borislav Petkov, Andy Lutomirski, Kees Cook,
	Tony Luck, Guilherme G. Piccoli, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, H . Peter Anvin, linux-efi,
	linux-kernel, kernel-team, rmikey

Hello Ard,

On Fri, Jun 12, 2026 at 01:11:11PM +0200, Ard Biesheuvel wrote:
> On Fri, 12 Jun 2026, at 13:01, Breno Leitao wrote:
> > +	/*
> > +	 * If __efi_queue_work() timed out and disabled runtime services, the
> > +	 * caller is gone and efi_rts_work is no longer ours: park the worker
> > +	 * so it never signals the stale completion or runs again.
> > +	 */
> > +	if (!efi_enabled(EFI_RUNTIME_SERVICES))
> > +		efi_rts_park_worker();
> > +
> 
> So one thing to note here is that the arm64 version of the exception recovery
> (in efi_runtime_fixup_exception()) will also clear EFI_RUNTIME_SERVICES, and
> that will occur before this check. This means we will park the worker not only
> on a time out, but also on an sync exception in the firmware.

Correct. Just to clarify the code flow for a faulty EFI:

	efi_call_rts  (EFI faults)
	-> __do_kernel_fault()
	->  efi_runtime_fixup_exception() == true   (clears EFI_RUNTIME_SERVICES, rewrites regs)
	-> __efi_rt_asm_recover
	-> caller of __efi_rt_asm_wrapper ->
	-> back into efi_call_rts(), with status = EFI_ABORTED (and EFI_RUNTIME_SERVICES unset).

Previous it was completing (complete(&efi_rts_work.efi_rts_comp);) and
disabling the EFI_RUNTIME_SERVICES

With this change it is parking the workthread with falut as well (with
EFI_RUNTIME_SERVICES disabled).

> I suspect this is actually what we want, but it deserves to be called out.

I think so.

It would not be hard to park on timeout, probably adding a new variable to
track "tiemout operations":


  if (!wait_for_completion_timeout(&efi_rts_work.efi_rts_comp, EFI_RTS_TIMEOUT)) {
      pr_err("EFI runtime service %d wedged in firmware; disabling EFI runtime services\n", id);
+     WRITE_ONCE(efi_rts_work.timeout, true); 
      clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
      return EFI_ABORT
  }

Then Worker tail tests the timeout field, not the bit:

  if (READ_ONCE(efi_rts_work.field))
      efi_rts_park_worker();

But, from my newbie view, you want to park in both cases.

Thanks for the heads-up,
--breno

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

* Re: [PATCH v2 5/6] efi/runtime-wrappers: honour EFI_RUNTIME_SERVICES in the non-blocking paths
  2026-06-12 11:01 ` [PATCH v2 5/6] efi/runtime-wrappers: honour EFI_RUNTIME_SERVICES in the non-blocking paths Breno Leitao
@ 2026-06-16 12:10   ` Breno Leitao
  0 siblings, 0 replies; 10+ messages in thread
From: Breno Leitao @ 2026-06-16 12:10 UTC (permalink / raw)
  To: Ard Biesheuvel, Ilias Apalodimas, Borislav Petkov,
	Andy Lutomirski, Kees Cook, Tony Luck, Guilherme G. Piccoli,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin
  Cc: linux-efi, linux-kernel, kernel-team

On Fri, Jun 12, 2026 at 04:01:32AM -0700, Breno Leitao wrote:
> Three wrappers call firmware directly instead of going through
> __efi_queue_work(), and none of them check whether runtime services are
> still enabled: virt_efi_set_variable_nb(),
> virt_efi_query_variable_info_nb() and virt_efi_reset_system(). Once a
> hang has cleared EFI_RUNTIME_SERVICES - or efi_recover_from_page_fault()
> has cleared it on a firmware page fault - these paths still enter the
> (possibly wedged) firmware, e.g. an EFI pstore write through the
> non-blocking SetVariable() variant, in violation of UEFI's
> non-reentrancy rules. reset_system() is reachable too: efi_reboot()
> only gates it on the static efi_rt_services_supported() mask, which does
> not track the runtime disable.
> 
> Check efi_enabled(EFI_RUNTIME_SERVICES) at the top of each before taking
> efi_runtime_lock and calling into firmware.
> 
> Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  drivers/firmware/efi/runtime-wrappers.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
> index 8badf0419a148..842f72d44211f 100644
> --- a/drivers/firmware/efi/runtime-wrappers.c
> +++ b/drivers/firmware/efi/runtime-wrappers.c
> @@ -461,6 +461,9 @@ virt_efi_set_variable_nb(efi_char16_t *name, efi_guid_t *vendor, u32 attr,
>  {
>  	efi_status_t status;
>  
> +	if (!efi_enabled(EFI_RUNTIME_SERVICES))
> +		return EFI_DEVICE_ERROR;
> +
>  	if (down_trylock(&efi_runtime_lock))
>  		return EFI_NOT_READY;

Shashiko reported a good feedback, that the check for
EFI_RUNTIME_SERVICES needs to be inside the efi_runtime_lock to avoid
TOCTOU issues. Fixing it and respining.

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

end of thread, other threads:[~2026-06-16 12:10 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-12 11:01 [PATCH v2 0/6] efi/runtime-wrappers: bound the wait for EFI runtime service calls Breno Leitao
2026-06-12 11:01 ` [PATCH v2 1/6] efi: fix stale reference to efi_recover_from_page_fault() Breno Leitao
2026-06-12 11:01 ` [PATCH v2 2/6] efi/runtime-wrappers: handle queue_work() failure with goto exit Breno Leitao
2026-06-12 11:01 ` [PATCH v2 3/6] efi/runtime-wrappers: check EFI_RUNTIME_SERVICES before using efi_rts_work Breno Leitao
2026-06-12 11:01 ` [PATCH v2 4/6] efi/runtime-wrappers: bound the wait for EFI runtime service calls Breno Leitao
2026-06-12 11:01 ` [PATCH v2 5/6] efi/runtime-wrappers: honour EFI_RUNTIME_SERVICES in the non-blocking paths Breno Leitao
2026-06-16 12:10   ` Breno Leitao
2026-06-12 11:01 ` [PATCH v2 6/6] efi/runtime-wrappers: retire the worker if a wedged call ever returns Breno Leitao
2026-06-12 11:11   ` Ard Biesheuvel
2026-06-16 10:13     ` Breno Leitao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.