[PATCH] efi_loader: capsule: Record capsule result only if capsule is read

[Ilias Apalodimas]

Masami-san

On Fri, 12 Nov 2021 at 15:05, Masami Hiramatsu
<masami.hiramatsu at linaro.org> wrote:
>
> Record capsule update result only if the capsule file is
> successfully read, because the capsule GUID is not sure when
> the file can not be read or the file is not a capsule.
> Without this fix, if user puts a dummy (non-capsule) file
> under (ESP)EFI/UpdateCapsule, U-Boot causes a synchronous
> abort.
>
> This also fixes use-after-free bug of the 'capsule' variable.
>
> Fixes: c74cd8bd08d1 ("efi_loader: capsule: add capsule_on_disk support")
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu at linaro.org>
> ---
>  lib/efi_loader/efi_capsule.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 850937fd12..502bcfca6e 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -1108,13 +1108,13 @@ efi_status_t efi_launch_capsules(void)
>                                 log_err("Applying capsule %ls failed\n",
>                                         files[i]);
>
> +                       /* create CapsuleXXXX */
> +                       set_capsule_result(index, capsule, ret);
> +

The fix seems correct, this was probably broken all along since the
code frees the capsule before accessing it, even if it's successfully
read.

>                         free(capsule);
>                 } else {
>                         log_err("Reading capsule %ls failed\n", files[i]);
>                 }
> -               /* create CapsuleXXXX */
> -               set_capsule_result(index, capsule, ret);
> -
>                 /* delete a capsule either in case of success or failure */
>                 ret = efi_capsule_delete_file(files[i]);
>                 if (ret != EFI_SUCCESS)
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>