[PATCH] cmd: avb: free partition buffer upon verify completion

[Igor Opaniuk]

Hi Gary,

Thanks for the patch and sorry for a late reply.

On Wed, May 6, 2020 at 5:06 PM Gary Bisson
<gary.bisson at boundarydevices.com> wrote:
>
> Allows to run 'avb verify' multiple times which can be useful after a
> failure to be able to re-flash the partition and try again.
>
> Signed-off-by: Gary Bisson <gary.bisson at boundarydevices.com>
> ---
> Hi,
>
> This was added because of the following scenario:
> 1- fastboot flash boot boot.img
> 2- avb verify
>   -> fails because vbmeta wasn't updated
> 3- fastboot flash vbmeta vbmeta.img
> 4- avb verify
>   -> fails because it can't allocate memory as previous buffer wasn't
> freed
>
> Let me know if you have any questions.
>
> Regards,
> Gary
> ---
>  cmd/avb.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/cmd/avb.c b/cmd/avb.c
> index a4de5c40a2..67154de4e1 100644
> --- a/cmd/avb.c
> +++ b/cmd/avb.c
> @@ -312,6 +312,14 @@ int do_avb_verify_part(cmd_tbl_t *cmdtp, int flag,
>                 printf("Unknown error occurred\n");
>         }
>
> +       /* Free image buffers now that verification is complete */
> +       if (slot_result != AVB_SLOT_VERIFY_RESULT_ERROR_OOM) {
Freeing out_data can be done not only when res is
AVB_SLOT_VERIFY_RESULT_ERROR_OOM.
For example, in the line 1196 in [1]:

AvbSlotVerifyResult sub_ret = avb_append_options(ops,
                                                 slot_data,
                                                 &toplevel_vbmeta,
                                                 algorithm_type,
                                                 hashtree_error_mode);
if (sub_ret != AVB_SLOT_VERIFY_RESULT_OK) {
        ret = sub_ret;
        goto fail;
}

> +               int i;
> +               for (i = 0; i < (int)out_data->num_loaded_partitions; i++)
> +                       if (out_data->loaded_partitions[i].data)
> +                               free(out_data->loaded_partitions[i].data);

It's better to use avb_slot_verify_data_free()  for the whole out_data instead
for proper cleanup. Check how it's done in unittests for libavb [2].

> +       }
> +
>         return res;
>  }
>
> --
> 2.26.2
>

[1] lib/libavb/avb_slot_verify.c
[2] https://android.googlesource.com/platform/external/avb/+/refs/tags/android-9.0.0_r37/test/avb_slot_verify_unittest.cc#156

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk at gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk