* [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* 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
* [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