| Message ID | 20231128112636.26655-1-avromanov@salutedevices.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Mattijs Korpershoek |
| Headers | show |
| Series | [v1] android_ab: don't ignore ab_control_store return code | expand |
Hello! Really sorry for the noise, but please, take a look at my patch :) On Tue, Nov 28, 2023 at 02:26:34PM +0300, Alexey Romanov wrote: > ab_control_store() can return an error if writing to disk fails. > In this case, we have to pass the error code to the caller. > > Signed-off-by: Alexey Romanov <avromanov@salutedevices.com> > --- > boot/android_ab.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/boot/android_ab.c b/boot/android_ab.c > index 73b55c196c..5a3152dd53 100644 > --- a/boot/android_ab.c > +++ b/boot/android_ab.c > @@ -337,7 +337,15 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, > > if (store_needed) { > abc->crc32_le = ab_control_compute_crc(abc); > - ab_control_store(dev_desc, part_info, abc, 0); > + ret = ab_control_store(dev_desc, part_info, abc, 0); > + if (ret < 0) { > +#if ANDROID_AB_BACKUP_OFFSET > + free(backup_abc); > +#endif > + free(abc); > + log_err("ANDROID: failed to store boot control block: %d\n", ret); > + return ret; > + } > } > > #if ANDROID_AB_BACKUP_OFFSET > @@ -346,8 +354,14 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, > * to the backup offset > */ > if (memcmp(backup_abc, abc, sizeof(*abc)) != 0) { > - ab_control_store(dev_desc, part_info, abc, > + ret = ab_control_store(dev_desc, part_info, abc, > ANDROID_AB_BACKUP_OFFSET); > + if (ret < 0) { > + free(backup_abc); > + free(abc); > + log_err("ANDROID: failed to store boot control block: %d\n", ret); > + return ret; > + } > } > free(backup_abc); > #endif > -- > 2.39.2 >
Hi Alexey, Thank you for your patch. I'm looping in the Android AB reviewers and maintainers (Sam and Igor). They were not automatically add due to a stale MAINTAINERS entry[1] [1] https://lore.kernel.org/all/20231220-maintainers-fix-ab-v1-1-dd16fc5285ed@baylibre.com/ On Tue, Nov 28, 2023 at 14:26, Alexey Romanov <avromanov@salutedevices.com> wrote: > ab_control_store() can return an error if writing to disk fails. > In this case, we have to pass the error code to the caller. > > Signed-off-by: Alexey Romanov <avromanov@salutedevices.com> > --- > boot/android_ab.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/boot/android_ab.c b/boot/android_ab.c > index 73b55c196c..5a3152dd53 100644 > --- a/boot/android_ab.c > +++ b/boot/android_ab.c > @@ -337,7 +337,15 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, > > if (store_needed) { > abc->crc32_le = ab_control_compute_crc(abc); > - ab_control_store(dev_desc, part_info, abc, 0); > + ret = ab_control_store(dev_desc, part_info, abc, 0); > + if (ret < 0) { > +#if ANDROID_AB_BACKUP_OFFSET > + free(backup_abc); > +#endif > + free(abc); > + log_err("ANDROID: failed to store boot control block: %d\n", ret); There is no point in logging here, because ab_control_store() already logs: ret = blk_dwrite(dev_desc, part_info->start + abc_offset, abc_blocks, abc); if (IS_ERR_VALUE(ret)) { log_err("ANDROID: Could not write back the misc partition\n"); return -EIO; } > + return ret; > + } > } > > #if ANDROID_AB_BACKUP_OFFSET > @@ -346,8 +354,14 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, > * to the backup offset > */ > if (memcmp(backup_abc, abc, sizeof(*abc)) != 0) { > - ab_control_store(dev_desc, part_info, abc, > + ret = ab_control_store(dev_desc, part_info, abc, > ANDROID_AB_BACKUP_OFFSET); > + if (ret < 0) { > + free(backup_abc); > + free(abc); > + log_err("ANDROID: failed to store boot control block: %d\n", ret); Same here With that addressed: Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> > + return ret; > + } > } > free(backup_abc); > #endif > -- > 2.39.2
diff --git a/boot/android_ab.c b/boot/android_ab.c index 73b55c196c..5a3152dd53 100644 --- a/boot/android_ab.c +++ b/boot/android_ab.c @@ -337,7 +337,15 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, if (store_needed) { abc->crc32_le = ab_control_compute_crc(abc); - ab_control_store(dev_desc, part_info, abc, 0); + ret = ab_control_store(dev_desc, part_info, abc, 0); + if (ret < 0) { +#if ANDROID_AB_BACKUP_OFFSET + free(backup_abc); +#endif + free(abc); + log_err("ANDROID: failed to store boot control block: %d\n", ret); + return ret; + } } #if ANDROID_AB_BACKUP_OFFSET @@ -346,8 +354,14 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, * to the backup offset */ if (memcmp(backup_abc, abc, sizeof(*abc)) != 0) { - ab_control_store(dev_desc, part_info, abc, + ret = ab_control_store(dev_desc, part_info, abc, ANDROID_AB_BACKUP_OFFSET); + if (ret < 0) { + free(backup_abc); + free(abc); + log_err("ANDROID: failed to store boot control block: %d\n", ret); + return ret; + } } free(backup_abc); #endif
ab_control_store() can return an error if writing to disk fails. In this case, we have to pass the error code to the caller. Signed-off-by: Alexey Romanov <avromanov@salutedevices.com> --- boot/android_ab.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)