[PATCH] cmd: avb: free partition buffer upon verify completion
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@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) { + 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); + } + return res; }
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@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@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...
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.bisson@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@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
participants (2)
-
Gary Bisson -
Igor Opaniuk