[PATCH] efi_loader: check update capsule parameters

[Heinrich Schuchardt]

Cc: Takahiro, Sughosh, Ilias

On 6/11/21 6:15 PM, Vincent Stehlé wrote:
> UpdateCapsule() must return EFI_INVALID_PARAMETER in a number of cases,
> listed by the UEFI specification and tested by the SCT. Add a common
> function to do that.
>
> This fixes SCT UpdateCapsule_Conf failures.
>
> Reviewed-by: Grant Likely <grant.likely at arm.com>
> Signed-off-by: Vincent Stehlé <vincent.stehle at arm.com>
> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Cc: Alexander Graf <agraf at csgraf.de>
> ---
>   include/efi_loader.h         | 24 ++++++++++++++++++++++++
>   lib/efi_loader/efi_capsule.c |  8 ++++----
>   lib/efi_loader/efi_runtime.c |  8 ++++++++
>   3 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 0a9c82a257e..426d1c72d7d 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -910,6 +910,30 @@ extern const struct efi_firmware_management_protocol efi_fmp_fit;
>   extern const struct efi_firmware_management_protocol efi_fmp_raw;
>
>   /* Capsule update */
> +static inline efi_status_t
> +efi_valid_update_capsule_params(struct efi_capsule_header
> +						**capsule_header_array,
> +				efi_uintn_t capsule_count,
> +				u64 scatter_gather_list)
> +{
> +	u32 flags;
> +
> +	if (!capsule_count)
> +		return EFI_INVALID_PARAMETER;

If capsule count > 1, don't you have to check all capsules headers?

> +
> +	flags = capsule_header_array[0]->flags;
> +
> +	if (((flags & CAPSULE_FLAGS_PERSIST_ACROSS_RESET) &&
> +	     !scatter_gather_list) ||
> +	    ((flags & CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE) &&
> +	     !(flags & CAPSULE_FLAGS_PERSIST_ACROSS_RESET)) ||
> +	    ((flags & CAPSULE_FLAGS_INITIATE_RESET) &&
> +	     !(flags & CAPSULE_FLAGS_PERSIST_ACROSS_RESET)))
> +		return EFI_INVALID_PARAMETER;

What happens if capsule(0) has CAPSULE_FLAGS_INITIATE_RESET and
capsule(4) has !CAPSULE_FLAGS_PERSIST_ACROSS_RESET?

Best regards

Heinrich

> +
> +	return EFI_SUCCESS;
> +}
> +
>   efi_status_t EFIAPI efi_update_capsule(
>   		struct efi_capsule_header **capsule_header_array,
>   		efi_uintn_t capsule_count,
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 60309d4a07d..380cfd70290 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -442,12 +442,12 @@ efi_status_t EFIAPI efi_update_capsule(
>   	EFI_ENTRY("%p, %zu, %llu\n", capsule_header_array, capsule_count,
>   		  scatter_gather_list);
>
> -	if (!capsule_count) {
> -		ret = EFI_INVALID_PARAMETER;
> +	ret = efi_valid_update_capsule_params(capsule_header_array,
> +					      capsule_count,
> +					      scatter_gather_list);
> +	if (ret != EFI_SUCCESS)
>   		goto out;
> -	}
>
> -	ret = EFI_SUCCESS;
>   	for (i = 0, capsule = *capsule_header_array; i < capsule_count;
>   	     i++, capsule = *(++capsule_header_array)) {
>   		/* sanity check */
> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> index 93a695fc27e..449ad8b9f36 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -467,6 +467,14 @@ efi_status_t __efi_runtime EFIAPI efi_update_capsule_unsupported(
>   			efi_uintn_t capsule_count,
>   			u64 scatter_gather_list)
>   {
> +	efi_status_t ret;
> +
> +	ret = efi_valid_update_capsule_params(capsule_header_array,
> +					      capsule_count,
> +					      scatter_gather_list);
> +	if (ret != EFI_SUCCESS)
> +		return ret;
> +
>   	return EFI_UNSUPPORTED;
>   }
>
>