Hi Igor,
On Sun, May 10, 2020 at 01:30:53PM +0300, Igor Opaniuk wrote:
> Hi Gary,
>
> Thanks for the patch and sorry for a late reply.
No problem.
> On Wed, May 6, 2020 at 5:06 PM Gary Bisson
> <gary.bis...@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.bis...@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;
> }
Indeed.
> > + 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].
I'll check it out and offer a v2. Thanks for your feedback.
Regards,
Gary
