| Message ID | 20240828143924.3987331-1-JPEWhacker@gmail.com |
|---|---|
| State | Accepted |
| Commit | cc2f60c13f6aaf1bd277cf10c8b375c2e4a695b7 |
| Delegated to: | Mattijs Korpershoek |
| Headers | show |
| Series | [v2] android_ab: Fixes: Fix backup offset calculation | expand |
Hi Joshua, Thank you for the patch. On mer., août 28, 2024 at 08:37, Joshua Watt <jpewhacker@gmail.com> wrote: > The backup offset is in bytes, but was incorrectly be interpreted as > blocks, leading to it being written to the wrong location. Fix the > calculation, clarify that ANDROID_AB_BACKUP_OFFSET is in bytes and must > be a multiple of the block size, and add a runtime check to validate the > offset. > > Signed-off-by: Joshua Watt <JPEWhacker@gmail.com> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> As Igor suggested, I will add: Fixes: 3430f24bc69d ("android_ab: Try backup booloader_message") When applying. > --- > boot/android_ab.c | 9 +++++++-- > common/Kconfig | 3 ++- > 2 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/boot/android_ab.c b/boot/android_ab.c > index 143f373aae..1196a189ed 100644 > --- a/boot/android_ab.c > +++ b/boot/android_ab.c > @@ -139,8 +139,13 @@ static int ab_control_store(struct blk_desc *dev_desc, > { > ulong abc_offset, abc_blocks, ret; > > - abc_offset = offset + > - offsetof(struct bootloader_message_ab, slot_suffix) / > + if (offset % part_info->blksz) { > + log_err("ANDROID: offset not block aligned\n"); > + return -EINVAL; > + } > + > + abc_offset = (offset + > + offsetof(struct bootloader_message_ab, slot_suffix)) / > part_info->blksz; > abc_blocks = DIV_ROUND_UP(sizeof(struct bootloader_control), > part_info->blksz); > diff --git a/common/Kconfig b/common/Kconfig > index 83c81edac2..e1b8557e0c 100644 > --- a/common/Kconfig > +++ b/common/Kconfig > @@ -986,7 +986,8 @@ config ANDROID_AB_BACKUP_OFFSET > help > If non-zero, a backup bootloader message starting at this offset in > the partition will tried in the event that the primary one (starting > - at offset 0) fails its checksum. > + at offset 0) fails its checksum. The offset is in bytes and must be > + multiple of the block size. > > endmenu > > -- > 2.45.2
Hi, On Wed, 28 Aug 2024 08:37:57 -0600, Joshua Watt wrote: > The backup offset is in bytes, but was incorrectly be interpreted as > blocks, leading to it being written to the wrong location. Fix the > calculation, clarify that ANDROID_AB_BACKUP_OFFSET is in bytes and must > be a multiple of the block size, and add a runtime check to validate the > offset. > > > [...] Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu) [1/1] android_ab: Fixes: Fix backup offset calculation https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/cc2f60c13f6aaf1bd277cf10c8b375c2e4a695b7 -- Mattijs
diff --git a/boot/android_ab.c b/boot/android_ab.c index 143f373aae..1196a189ed 100644 --- a/boot/android_ab.c +++ b/boot/android_ab.c @@ -139,8 +139,13 @@ static int ab_control_store(struct blk_desc *dev_desc, { ulong abc_offset, abc_blocks, ret; - abc_offset = offset + - offsetof(struct bootloader_message_ab, slot_suffix) / + if (offset % part_info->blksz) { + log_err("ANDROID: offset not block aligned\n"); + return -EINVAL; + } + + abc_offset = (offset + + offsetof(struct bootloader_message_ab, slot_suffix)) / part_info->blksz; abc_blocks = DIV_ROUND_UP(sizeof(struct bootloader_control), part_info->blksz); diff --git a/common/Kconfig b/common/Kconfig index 83c81edac2..e1b8557e0c 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -986,7 +986,8 @@ config ANDROID_AB_BACKUP_OFFSET help If non-zero, a backup bootloader message starting at this offset in the partition will tried in the event that the primary one (starting - at offset 0) fails its checksum. + at offset 0) fails its checksum. The offset is in bytes and must be + multiple of the block size. endmenu
The backup offset is in bytes, but was incorrectly be interpreted as blocks, leading to it being written to the wrong location. Fix the calculation, clarify that ANDROID_AB_BACKUP_OFFSET is in bytes and must be a multiple of the block size, and add a runtime check to validate the offset. Signed-off-by: Joshua Watt <JPEWhacker@gmail.com> --- boot/android_ab.c | 9 +++++++-- common/Kconfig | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-)