Masami-san
On Fri, 12 Nov 2021 at 15:05, Masami Hiramatsu
<masami.hirama...@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.hirama...@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.apalodi...@linaro.org>
