| Message ID | 163672231514.111863.5335945815077457121.stgit@localhost |
|---|---|
| State | New |
| Headers | show |
| Series | efi_loader: capsule: Record capsule result only if capsule is read | expand |
Masami-san On Fri, 12 Nov 2021 at 15:05, Masami Hiramatsu <masami.hiramatsu@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@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@linaro.org>
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); + 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)
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@linaro.org> --- lib/efi_loader/efi_capsule.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)