[PATCH v3 2/2] android_ab: Fix ANDROID_AB_BACKUP_OFFSET

Colin McAllister colin.mcallister at garmin.com
Fri Mar 8 17:59:37 CET 2024


Currently, setting CONFIG_AB_BACKUP_OFFSET in a target's defconfig will
not actually enable the #if protected code in android_ab.c. This is
because "CONFIG_" should have been prepended to the config macro, or the
macros defined in kconfig.h could have been used.

The code included by ANDROID_AB_BACKUP_OFFSET has been refactored to no
longer be conditionally compiled by preprocessor conditionals and
instead use C conditionals. This better aligns with the Linux kernel
style guide.

Fixes: 3430f24bc6 ("android_ab: Try backup booloader_message")
Signed-off-by: Colin McAllister <colin.mcallister at garmin.com>
Cc: Joshua Watt <JPEWhacker at gmail.com>
Cc: Simon Glass <sjg at chromium.org>
---
v2:
  - Replaced #if conditionals with C if conditionals
  - Opted to use CONFIG_ANDROID_AB_BACKUP_OFFSET directly instead of
    macros in kconfig.h as CONFIG_ANDROID_AB_BACKUP_OFFSET is not a
        boolean or tristate value and doesn't have different values when
        building SPL or TPL.
v3:
  - Added "Fixes:" tag

 boot/android_ab.c | 97 ++++++++++++++++++++++-------------------------
 1 file changed, 45 insertions(+), 52 deletions(-)

diff --git a/boot/android_ab.c b/boot/android_ab.c
index 9a3d15ec60..f547aa64e4 100644
--- a/boot/android_ab.c
+++ b/boot/android_ab.c
@@ -187,13 +187,12 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info,
                   bool dec_tries)
 {
        struct bootloader_control *abc = NULL;
+       struct bootloader_control *backup_abc = NULL;
        u32 crc32_le;
        int slot, i, ret;
        bool store_needed = false;
+       bool valid_backup = false;
        char slot_suffix[4];
-#if ANDROID_AB_BACKUP_OFFSET
-       struct bootloader_control *backup_abc = NULL;
-#endif

        ret = ab_control_create_from_disk(dev_desc, part_info, &abc, 0);
        if (ret < 0) {
@@ -205,53 +204,49 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info,
                return ret;
        }

-#if ANDROID_AB_BACKUP_OFFSET
-       ret = ab_control_create_from_disk(dev_desc, part_info, &backup_abc,
-                                         ANDROID_AB_BACKUP_OFFSET);
-       if (ret < 0) {
-               free(abc);
-               return ret;
+       if (CONFIG_ANDROID_AB_BACKUP_OFFSET) {
+               ret = ab_control_create_from_disk(dev_desc, part_info, &backup_abc,
+                                                 CONFIG_ANDROID_AB_BACKUP_OFFSET);
+               if (ret < 0) {
+                       free(abc);
+                       return ret;
+               }
        }
-#endif

        crc32_le = ab_control_compute_crc(abc);
        if (abc->crc32_le != crc32_le) {
                log_err("ANDROID: Invalid CRC-32 (expected %.8x, found %.8x),",
                        crc32_le, abc->crc32_le);
-#if ANDROID_AB_BACKUP_OFFSET
-               crc32_le = ab_control_compute_crc(backup_abc);
-               if (backup_abc->crc32_le != crc32_le) {
-                       log_err("ANDROID: Invalid backup CRC-32 ");
-                       log_err("expected %.8x, found %.8x),",
-                               crc32_le, backup_abc->crc32_le);
-#endif
-
-                       log_err("re-initializing A/B metadata.\n");
+               if (CONFIG_ANDROID_AB_BACKUP_OFFSET) {
+                       crc32_le = ab_control_compute_crc(backup_abc);
+                       if (backup_abc->crc32_le != crc32_le) {
+                               log_err(" ANDROID: Invalid backup CRC-32 ");
+                               log_err("(expected %.8x, found %.8x),",
+                                       crc32_le, backup_abc->crc32_le);
+                       } else {
+                               valid_backup = true;
+                               log_info(" copying A/B metadata from backup.\n");
+                               memcpy(abc, backup_abc, sizeof(*abc));
+                       }
+               }

+               if (!valid_backup) {
+                       log_err(" re-initializing A/B metadata.\n");
                        ret = ab_control_default(abc);
                        if (ret < 0) {
-#if ANDROID_AB_BACKUP_OFFSET
-                               free(backup_abc);
-#endif
+                               if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
+                                       free(backup_abc);
                                free(abc);
                                return -ENODATA;
                        }
-#if ANDROID_AB_BACKUP_OFFSET
-               } else {
-                       /*
-                        * Backup is valid. Copy it to the primary
-                        */
-                       memcpy(abc, backup_abc, sizeof(*abc));
                }
-#endif
                store_needed = true;
        }

        if (abc->magic != BOOT_CTRL_MAGIC) {
                log_err("ANDROID: Unknown A/B metadata: %.8x\n", abc->magic);
-#if ANDROID_AB_BACKUP_OFFSET
-               free(backup_abc);
-#endif
+               if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
+                       free(backup_abc);
                free(abc);
                return -ENODATA;
        }
@@ -259,9 +254,8 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info,
        if (abc->version > BOOT_CTRL_VERSION) {
                log_err("ANDROID: Unsupported A/B metadata version: %.8x\n",
                        abc->version);
-#if ANDROID_AB_BACKUP_OFFSET
-               free(backup_abc);
-#endif
+               if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
+                       free(backup_abc);
                free(abc);
                return -ENODATA;
        }
@@ -338,30 +332,29 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info,
                abc->crc32_le = ab_control_compute_crc(abc);
                ret = ab_control_store(dev_desc, part_info, abc, 0);
                if (ret < 0) {
-#if ANDROID_AB_BACKUP_OFFSET
-                       free(backup_abc);
-#endif
+                       if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
+                               free(backup_abc);
                        free(abc);
                        return ret;
                }
        }

-#if ANDROID_AB_BACKUP_OFFSET
-       /*
-        * If the backup doesn't match the primary, write the primary
-        * to the backup offset
-        */
-       if (memcmp(backup_abc, abc, sizeof(*abc)) != 0) {
-               ret = ab_control_store(dev_desc, part_info, abc,
-                                      ANDROID_AB_BACKUP_OFFSET);
-               if (ret < 0) {
-                       free(backup_abc);
-                       free(abc);
-                       return ret;
+       if (CONFIG_ANDROID_AB_BACKUP_OFFSET) {
+               /*
+               * If the backup doesn't match the primary, write the primary
+               * to the backup offset
+               */
+               if (memcmp(backup_abc, abc, sizeof(*abc)) != 0) {
+                       ret = ab_control_store(dev_desc, part_info, abc,
+                                              CONFIG_ANDROID_AB_BACKUP_OFFSET);
+                       if (ret < 0) {
+                               free(backup_abc);
+                               free(abc);
+                               return ret;
+                       }
                }
+               free(backup_abc);
        }
-       free(backup_abc);
-#endif

        free(abc);

--
2.43.2


________________________________

CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.


More information about the U-Boot mailing list