Re: [PATCH 1/2] efi/runtime-wrappers: bound the wait for EFI runtime service calls

[Ard Biesheuvel]

Hi Breno,

On Tue, 9 Jun 2026, at 13:55, Breno Leitao wrote:
> 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.
>
> Replace the indefinite wait_for_completion() in __efi_queue_work()
> with wait_for_completion_timeout() bounded by EFI_RTS_TIMEOUT
> (120 seconds). On timeout, log the wedged runtime service id and
> return EFI_TIMEOUT to the caller.
>
> The wedged worker is intentionally leaked - it is still inside
> firmware and cannot be cancelled - and the shared efi_rts_work is
> abandoned to it. EFI runtime services become unavailable until reboot;
> the alternative is permanent userspace hang.
>
> This patch should land together with the follow-up that introduces
> the efi_rts_dead fast-fail flag: between the two, a subsequent
> __efi_queue_work() caller will re-INIT_WORK() and re-init_completion()
> on the work_struct and completion that the leaked worker still owns,
> which can corrupt workqueue state and let the next caller observe
> the leaked call's status as if it were its own. The split exists for
> review clarity; reviewers may prefer to squash.
>
> Known limitation: the union efi_rts_args that __efi_queue_work() hands
> to the worker contains pointers into the caller's stack frame (the
> compound literal in efi_queue_work() and the in/out buffers it points
> to, e.g. *tm in GetTime). Once the caller returns -EIO and unwinds,
> those slots are reusable. If firmware eventually unblocks and writes
> to the output buffers after the timeout has fired, the writes land in
> whatever now occupies that memory. In practice firmware that hangs for
> more than 120 seconds tends to stay hung. A follow-up bouncing args
> and output buffers through kmalloc would close this gap.
>
> At fleet scale this turns a generic "workqueue lockup" or stalled-task
> mystery into an unambiguous "EFI firmware is at fault" signal in
> dmesg.
>
> Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx>
> ---
>  drivers/firmware/efi/runtime-wrappers.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/firmware/efi/runtime-wrappers.c 
> b/drivers/firmware/efi/runtime-wrappers.c
> index da8d29621644..6ce6d094066e 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
> @@ -338,10 +346,16 @@ 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;
> +	}
> +
> +	if (!wait_for_completion_timeout(&efi_rts_work.efi_rts_comp,
> +					 EFI_RTS_TIMEOUT)) {
> +		pr_err("EFI runtime service %d wedged in firmware\n", id);
> +		return EFI_TIMEOUT;

Could we just clear the EFI_RUNTIME_SERVICES bit here right away? That
way, we probably won't need the second patch (unless I'm mistaken). It
/should/ also block the calls that are not routed via the workqueue,
e.g., any EFI pstore calls to the non-blocking SetVariable() variant,
but I just noticed that we never check EFI_RUNTIME_SERVICES on those
code paths, which is probably a bug. 

And please return EFI_ABORTED rather than EFI_TIMEOUT - probably doesn't
matter in practice but I'd like to avoid introducing more EFI return codes
in the runtime context that the spec mentions only for boot services stuff.