Re: [PATCH] efi: capsule: publish capsule_pending after efi_reset_type

["Ard Biesheuvel" <ardb@kernel.org>]

Hello Jaidev,

On Tue, 22 Sep 2026, at 03:09, Jaidev Shastri via B4 Relay wrote:
> From: Jaidev Shastri <jaidevshastri@vt.edu>
>
> efi_capsule_update_locked() sets capsule_pending and then
> efi_reset_type, both with plain stores. efi_capsule_pending() reads them
> from the reboot path without capsule_mutex.
>
> The comment above efi_capsule_pending() covers a caller that misses the
> update entirely. It does not cover the other outcome: with neither the
> stores nor the loads ordered, a caller can observe capsule_pending set
> and efi_reset_type still -1, and the reboot path then acts on a capsule
> with an invalid reset type.
>
> Write efi_reset_type first and publish the flag with
> smp_store_release(), paired with smp_load_acquire() in
> efi_capsule_pending(). A caller that misses the update entirely still
> behaves as documented.
>
> Found with MBCheck, a static herd7-based memory consistency checker.
>
> Signed-off-by: Jaidev Shastri <jaidevshastri@vt.edu>
> ---
>  drivers/firmware/efi/capsule.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>

I think the tool's conclusion is correct that concurrent execution
of efi_capsule_pending() and efi_capsule_update() may result in the
re-ordering and subsequent misreporting of the EFI reset type.

However, in practice, efi_capsule_pending() is only called on the
reboot path after all other CPUs have been stopped, so there is
really nothing to fix here.



> diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
> index dd6252638..2129007d8 100644
> --- a/drivers/firmware/efi/capsule.c
> +++ b/drivers/firmware/efi/capsule.c
> @@ -50,7 +50,8 @@ static DEFINE_MUTEX(capsule_mutex);
>   */
>  bool efi_capsule_pending(int *reset_type)
>  {
> -	if (!capsule_pending)
> +	/* Pairs with the smp_store_release() in efi_capsule_update_locked(). */
> +	if (!smp_load_acquire(&capsule_pending))
>  		return false;
> 
>  	if (reset_type)
> @@ -173,8 +174,14 @@ efi_capsule_update_locked(efi_capsule_header_t *capsule,
> 
>  	status = efi.update_capsule(&capsule, 1, sglist_phys);
>  	if (status == EFI_SUCCESS) {
> -		capsule_pending = true;
>  		efi_reset_type = reset;
> +		/*
> +		 * efi_capsule_pending() reads the flag without capsule_mutex
> +		 * and then the reset type, which is stored above. Publish the
> +		 * flag with release semantics so that a reader that sees it
> +		 * also sees the matching reset type.
> +		 */
> +		smp_store_release(&capsule_pending, true);
>  	}
> 
>  	return efi_status_to_err(status);
>
> ---
> base-commit: 93f51579e7df248780214094418f205253383cc5
> change-id: 20260921-mb-efi-capsule-d111fa00ae80
>
> Best regards,
> --  
> Jaidev Shastri <jaidevshastri@vt.edu>