From 4e599aa73a386dd1ba5091937ef2fc388d01ddd2 Mon Sep 17 00:00:00 2001 From: Aaron Kling Date: Mon, 13 Jan 2025 10:11:45 +0100 Subject: [PATCH 1/4] boot: android: Check kcmdline's for NULL in android_image_get_kernel() kcmdline and kcmdline_extra strings can be NULL. In that case, we still read the content from 0x00000 and pass that to the kernel, which is completely wrong. Fix android_image_get_kernel() to check for NULL before checking if they are empty strings. Fixes: 53a0ddb6d3be ("boot: android: fix extra command line support") Signed-off-by: Aaron Kling Reviewed-by: Nicolas Belin Reviewed-by: Julien Masson Tested-by: Sam Day Link: https://lore.kernel.org/r/20250113-kcmdline-extra-fix-v1-1-03cc9c039159@baylibre.com Signed-off-by: Mattijs Korpershoek --- boot/image-android.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/boot/image-android.c b/boot/image-android.c index 60a422dfb74..fa4e14ca469 100644 --- a/boot/image-android.c +++ b/boot/image-android.c @@ -337,12 +337,12 @@ int android_image_get_kernel(const void *hdr, if (bootargs) len += strlen(bootargs); - if (*img_data.kcmdline) { + if (img_data.kcmdline && *img_data.kcmdline) { printf("Kernel command line: %s\n", img_data.kcmdline); len += strlen(img_data.kcmdline) + (len ? 1 : 0); /* +1 for extra space */ } - if (*img_data.kcmdline_extra) { + if (img_data.kcmdline_extra && *img_data.kcmdline_extra) { printf("Kernel extra command line: %s\n", img_data.kcmdline_extra); len += strlen(img_data.kcmdline_extra) + (len ? 1 : 0); /* +1 for extra space */ } @@ -357,13 +357,13 @@ int android_image_get_kernel(const void *hdr, if (bootargs) strcpy(newbootargs, bootargs); - if (*img_data.kcmdline) { + if (img_data.kcmdline && *img_data.kcmdline) { if (*newbootargs) /* If there is something in newbootargs, a space is needed */ strcat(newbootargs, " "); strcat(newbootargs, img_data.kcmdline); } - if (*img_data.kcmdline_extra) { + if (img_data.kcmdline_extra && *img_data.kcmdline_extra) { if (*newbootargs) /* If there is something in newbootargs, a space is needed */ strcat(newbootargs, " "); strcat(newbootargs, img_data.kcmdline_extra); From af3afb70fc5e6cd39ceec70e1224a9a8ff4ab24b Mon Sep 17 00:00:00 2001 From: Igor Opaniuk Date: Thu, 9 Jan 2025 12:28:22 +0100 Subject: [PATCH 2/4] MAINTAINERS: move myself to reviewers for avb/ab As Mattijs Korpershoek is in fact doing overall maintenance of AVB/AB code, move myself to reviewers. CC: Mattijs Korpershoek Signed-off-by: Igor Opaniuk Reviewed-by: Mattijs Korpershoek Link: https://lore.kernel.org/r/20250109112854.825204-1-igor.opaniuk@gmail.com Signed-off-by: Mattijs Korpershoek --- MAINTAINERS | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 5a575f43277..e45b5a55663 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -66,8 +66,8 @@ F: lib/alist.c F: test/lib/alist.c ANDROID AB -M: Igor Opaniuk M: Mattijs Korpershoek +R: Igor Opaniuk R: Sam Protsenko S: Maintained T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git @@ -77,8 +77,8 @@ F: include/android_ab.h F: test/py/tests/test_android/test_ab.py ANDROID AVB -M: Igor Opaniuk M: Mattijs Korpershoek +R: Igor Opaniuk S: Maintained T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git F: cmd/avb.c From ae58cd7b39207175c8696d7bf38321b1a4c9957a Mon Sep 17 00:00:00 2001 From: Mattijs Korpershoek Date: Wed, 8 Jan 2025 15:38:41 +0100 Subject: [PATCH 3/4] bootstd: android: Add missing NULL in the avb partition list When booting an Android build with AVB enabled, it's still possible to deactivate the check for development purposes if the bootloader state is UNLOCKED. This is very useful for development and can be done at flashing time via: $ fastboot flash --disable-verity --disable-verification vbmeta vbmeta.img However, with bootmeth_android, we cannot boot this way: Scanning bootdev 'mmc@fa10000.bootdev': 0 android ready mmc 0 mmc@fa10000.bootdev.whole ** Booting bootflow 'mmc@fa10000.bootdev.whole' with android avb_vbmeta_image.c:188: ERROR: Hash does not match! avb_slot_verify.c:732: ERROR: vbmeta_a: Error verifying vbmeta image: HASH_MISMATCH get_partition: can't find partition '_a' avb_slot_verify.c:496: ERROR: _a: Error determining partition size. Verification failed, reason: I/O error occurred while trying to load data Boot failed (err=-5) No more bootdevs From the logs we can see that avb tries to read a partition named '_a'. It's doing so because the last element of requested_partitions implicitly is '\0', but the doc explicitly request it to be NULL instead. Add NULL as last element to requested_partitions to avoid this problem. Fixes: 125d9f3306ea ("bootstd: Add a bootmeth for Android") Reviewed-by: Julien Masson Link: https://lore.kernel.org/r/20250108-avb-disable-verif-v2-1-ba7d3b0d5b6a@baylibre.com Signed-off-by: Mattijs Korpershoek --- boot/bootmeth_android.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c index d8c92b44a99..4ce89d5bcef 100644 --- a/boot/bootmeth_android.c +++ b/boot/bootmeth_android.c @@ -422,7 +422,7 @@ static int run_avb_verification(struct bootflow *bflow) { struct blk_desc *desc = dev_get_uclass_plat(bflow->blk); struct android_priv *priv = bflow->bootmeth_priv; - const char * const requested_partitions[] = {"boot", "vendor_boot"}; + const char * const requested_partitions[] = {"boot", "vendor_boot", NULL}; struct AvbOps *avb_ops; AvbSlotVerifyResult result; AvbSlotVerifyData *out_data; From 6745cbed6edc06fae7fbc4b360e39c3963d57b13 Mon Sep 17 00:00:00 2001 From: Mattijs Korpershoek Date: Wed, 8 Jan 2025 15:38:42 +0100 Subject: [PATCH 4/4] bootstd: android: Allow boot with AVB failures when unlocked When the bootloader is UNLOCKED, it should be possible to boot Android even if AVB reports verification errors [1]. This allows developers to flash modified partitions on userdebug/engineering builds. Developers can do so on unlocked devices with: $ fastboot flash --disable-verity --disable-verification vbmeta vbmeta.img In such case, bootmeth_android refuses to boot. Allow the boot to continue when the device is UNLOCKED and AVB reports verification errors. [1] https://source.android.com/docs/security/features/verifiedboot/boot-flow#unlocked-devices Fixes: 125d9f3306ea ("bootstd: Add a bootmeth for Android") Reviewed-by: Julien Masson Link: https://lore.kernel.org/r/20250108-avb-disable-verif-v2-2-ba7d3b0d5b6a@baylibre.com Signed-off-by: Mattijs Korpershoek --- boot/bootmeth_android.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c index 4ce89d5bcef..a5a86b29d7f 100644 --- a/boot/bootmeth_android.c +++ b/boot/bootmeth_android.c @@ -450,17 +450,26 @@ static int run_avb_verification(struct bootflow *bflow) AVB_HASHTREE_ERROR_MODE_RESTART_AND_INVALIDATE, &out_data); - if (result != AVB_SLOT_VERIFY_RESULT_OK) { - printf("Verification failed, reason: %s\n", - str_avb_slot_error(result)); - avb_slot_verify_data_free(out_data); - return log_msg_ret("avb verify", -EIO); - } - - if (unlocked) - boot_state = AVB_ORANGE; - else + if (!unlocked) { + /* When device is locked, we only accept AVB_SLOT_VERIFY_RESULT_OK */ + if (result != AVB_SLOT_VERIFY_RESULT_OK) { + printf("Verification failed, reason: %s\n", + str_avb_slot_error(result)); + avb_slot_verify_data_free(out_data); + return log_msg_ret("avb verify", -EIO); + } boot_state = AVB_GREEN; + } else { + /* When device is unlocked, we also accept verification errors */ + if (result != AVB_SLOT_VERIFY_RESULT_OK && + result != AVB_SLOT_VERIFY_RESULT_ERROR_VERIFICATION) { + printf("Unlocked verification failed, reason: %s\n", + str_avb_slot_error(result)); + avb_slot_verify_data_free(out_data); + return log_msg_ret("avb verify unlocked", -EIO); + } + boot_state = AVB_ORANGE; + } extra_args = avb_set_state(avb_ops, boot_state); if (extra_args) { @@ -470,9 +479,11 @@ static int run_avb_verification(struct bootflow *bflow) goto free_out_data; } - ret = avb_append_commandline(bflow, out_data->cmdline); - if (ret < 0) - goto free_out_data; + if (result == AVB_SLOT_VERIFY_RESULT_OK) { + ret = avb_append_commandline(bflow, out_data->cmdline); + if (ret < 0) + goto free_out_data; + } return 0;