AVB assumed its partitions were on mmc: avb_ops_alloc() took a bare device number and get_partition() called find_mmc_device(). bootmeth_android accepts any bootdev but passes only desc->devnum, so an Android bootflow found on nvme 0 or usb 0 is verified against mmc 0.
Carry a uclass id alongside the device number and resolve the block device with blk_get_devnum_by_uclass_id(), keeping mmc_init() and the hardware partition switch on the mmc path only. "avb init <dev>" still means mmc, "avb init <interface> <dev>" selects another bus, and both now look the device up before allocating. get_boot_interface() returns UCLASS_INVALID without ops, so a caller that skipped "avb init" fails rather than reading mmc. Signed-off-by: Jan Kehren <[email protected]> --- Changes in v2: - Split: the renames moved to the preceding patch - Put the "avb init <dev>" comment on the branch it describes, by testing argc == 2 rather than argc == 3 - Drop the single-use part_num and move the user-area comment next to mmc_switch_part() boot/bootmeth_android.c | 2 +- cmd/avb.c | 24 ++++++--- common/Kconfig | 7 ++- common/avb_verify.c | 71 +++++++++++++++++--------- doc/android/avb2.rst | 7 ++- include/avb_verify.h | 26 +++++++--- test/py/tests/test_android/test_avb.py | 28 ++++++++++ 7 files changed, 127 insertions(+), 38 deletions(-) diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c index 0db08d4f86..ae05c08488 100644 --- a/boot/bootmeth_android.c +++ b/boot/bootmeth_android.c @@ -443,7 +443,7 @@ static int run_avb_verification(struct bootflow *bflow) bool unlocked = false; int ret; - avb_ops = avb_ops_alloc(desc->devnum); + avb_ops = avb_ops_alloc(desc->uclass_id, desc->devnum); if (!avb_ops) return log_msg_ret("avb ops", -ENOMEM); diff --git a/cmd/avb.c b/cmd/avb.c index 8fbd48ee5a..4c1f56a33a 100644 --- a/cmd/avb.c +++ b/cmd/avb.c @@ -16,17 +16,28 @@ static struct AvbOps *avb_ops; int do_avb_init(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { - unsigned long mmc_dev; + struct blk_desc *desc; - if (argc != 2) + if (argc < 2 || argc > 3) return CMD_RET_USAGE; - mmc_dev = hextoul(argv[1], NULL); + /* "avb init <dev>" keeps its old meaning of mmc <dev> */ + if (argc == 2) + desc = blk_get_devnum_by_uclass_id(UCLASS_MMC, + hextoul(argv[1], NULL)); + else + desc = blk_get_devnum_by_uclass_idname(argv[1], + hextoul(argv[2], NULL)); + + if (!desc) { + printf("Can't find the requested device\n"); + return CMD_RET_FAILURE; + } if (avb_ops) avb_ops_free(avb_ops); - avb_ops = avb_ops_alloc(mmc_dev); + avb_ops = avb_ops_alloc(desc->uclass_id, desc->devnum); if (avb_ops) return CMD_RET_SUCCESS; else @@ -428,7 +439,7 @@ int do_avb_write_pvalue(struct cmd_tbl *cmdtp, int flag, int argc, } static struct cmd_tbl cmd_avb[] = { - U_BOOT_CMD_MKENT(init, 2, 0, do_avb_init, "", ""), + U_BOOT_CMD_MKENT(init, 3, 0, do_avb_init, "", ""), U_BOOT_CMD_MKENT(read_rb, 2, 0, do_avb_read_rb, "", ""), U_BOOT_CMD_MKENT(write_rb, 3, 0, do_avb_write_rb, "", ""), U_BOOT_CMD_MKENT(is_unlocked, 1, 0, do_avb_is_unlocked, "", ""), @@ -464,7 +475,8 @@ static int do_avb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) U_BOOT_CMD( avb, 29, 0, do_avb, "Provides commands for testing Android Verified Boot 2.0 functionality", - "init <dev> - initialize avb2 for <dev>\n" + "init [<interface>] <dev> - initialize avb2 for hex <dev> on <interface>\n" + " (mmc, nvme, usb, ...; defaults to mmc)\n" "avb read_rb <num> - read rollback index at location <num>\n" "avb write_rb <num> <rb> - write rollback index <rb> to <num>\n" "avb is_unlocked - returns unlock status of the device\n" diff --git a/common/Kconfig b/common/Kconfig index c739225621..5590fbfdb2 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -885,9 +885,14 @@ config AVB_VERIFY This option enables compilation of bootloader-dependent operations, used by Android Verified Boot 2.0 library (libavb). Includes: * Helpers to process strings in order to build OS bootargs. - * Helpers to access MMC, similar to drivers/fastboot/fb_mmc.c. + * Helpers to access block devices, similar to + drivers/fastboot/fb_mmc.c. * Helpers to alloc/init/free avb ops. + Only mmc is initialised by AVB itself. Any other interface must + already be enumerated when AVB runs, either by the board code or by + a command such as "usb start". + if AVB_VERIFY config AVB_BUF_ADDR diff --git a/common/avb_verify.c b/common/avb_verify.c index fc29433c3c..b898ebb78f 100644 --- a/common/avb_verify.c +++ b/common/avb_verify.c @@ -8,6 +8,7 @@ #include <cpu_func.h> #include <image.h> #include <malloc.h> +#include <mmc.h> #include <part.h> #include <tee.h> #include <tee/optee_ta_avb.h> @@ -386,44 +387,68 @@ static unsigned long avb_blk_write(struct avb_blk_part *part, lbaint_t start, start, sectors, tmp_buf); } -static struct avb_blk_part *get_partition(AvbOps *ops, const char *partition) +static int avb_init_mmc(int dev_num) { + struct mmc *mmc; int ret; - u8 dev_num; - int part_num = 0; - struct avb_blk_part *part; - struct blk_desc *blk; - - part = malloc(sizeof(struct avb_blk_part)); - if (!part) - return NULL; - dev_num = get_boot_device(ops); - part->mmc = find_mmc_device(dev_num); - if (!part->mmc) { + mmc = find_mmc_device(dev_num); + if (!mmc) { printf("%s: no MMC device at slot %x\n", __func__, dev_num); - goto err; + return -ENODEV; } - ret = mmc_init(part->mmc); + ret = mmc_init(mmc); if (ret) { printf("%s: MMC initialization failed, err = %d\n", __func__, ret); - goto err; + return ret; } - if (IS_MMC(part->mmc)) { - ret = mmc_switch_part(part->mmc, part_num); + if (IS_MMC(mmc)) { + /* Select the user area, where the GPT and our images live */ + ret = mmc_switch_part(mmc, 0); if (ret) { printf("%s: MMC part switch failed, err = %d\n", __func__, ret); - goto err; + return ret; } } - blk = mmc_get_blk_desc(part->mmc); + return 0; +} + +static struct avb_blk_part *get_partition(AvbOps *ops, const char *partition) +{ + enum uclass_id uclass_id; + struct avb_blk_part *part; + struct blk_desc *blk; + int dev_num; + int ret; + + part = malloc(sizeof(struct avb_blk_part)); + if (!part) + return NULL; + + dev_num = get_boot_device(ops); + uclass_id = get_boot_interface(ops); + if (uclass_id == UCLASS_INVALID || dev_num < 0) { + printf("%s: no storage interface, run 'avb init' first\n", + __func__); + goto err; + } + + if (uclass_id == UCLASS_MMC) { + ret = avb_init_mmc(dev_num); + if (ret) + goto err; + } + + blk = blk_get_devnum_by_uclass_id(uclass_id, dev_num); if (!blk) { - printf("%s: failed to obtain block descriptor\n", __func__); + /* Every bus but mmc must already be started when AVB runs */ + printf("%s: no %s device %d, is the bus started?\n", __func__, + blk_get_uclass_name(uclass_id), dev_num); goto err; } @@ -433,7 +458,6 @@ static struct avb_blk_part *get_partition(AvbOps *ops, const char *partition) goto err; } - part->dev_num = dev_num; part->blk = blk; return part; @@ -1111,7 +1135,7 @@ __weak AvbIOResult avb_read_root_key_digest(AvbOps *ops, uint8_t *digest) * AVB2.0 AvbOps alloc/initialisation/free * ============================================================================ */ -AvbOps *avb_ops_alloc(int boot_device) +AvbOps *avb_ops_alloc(enum uclass_id uclass_id, int dev_num) { struct AvbOpsData *ops_data; @@ -1134,7 +1158,8 @@ AvbOps *avb_ops_alloc(int boot_device) ops_data->ops.read_persistent_value = read_persistent_value; #endif ops_data->ops.get_size_of_partition = get_size_of_partition; - ops_data->mmc_dev = boot_device; + ops_data->uclass_id = uclass_id; + ops_data->dev_num = dev_num; return &ops_data->ops; } diff --git a/doc/android/avb2.rst b/doc/android/avb2.rst index a514701241..cef49d3e0d 100644 --- a/doc/android/avb2.rst +++ b/doc/android/avb2.rst @@ -29,6 +29,10 @@ integration in ``common/avb_verify.c`` and the platform port under For additional details check [1]_. +Only mmc is started by AVB itself. Any other interface must already be +enumerated when AVB runs, either by the board code or by a command such as +``usb start``, or the partition lookup fails. + AVB using OP-TEE (optional) ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -80,7 +84,8 @@ AVB 2.0 U-Boot shell commands Provides CLI interface to invoke AVB 2.0 verification + misc. commands for different testing purposes:: - avb init <dev> - initialize avb 2 for <dev> + avb init [<interface>] <dev> - initialize avb 2 for hex <dev> on + <interface> (mmc, nvme, usb, ...; defaults to mmc) avb read_rb <num> - read rollback index at location <num> avb write_rb <num> <rb> - write rollback index <rb> to <num> avb is_unlocked - returns unlock status of the device diff --git a/include/avb_verify.h b/include/avb_verify.h index 8c97adbfbf..6dbbb39a8a 100644 --- a/include/avb_verify.h +++ b/include/avb_verify.h @@ -7,8 +7,9 @@ #define _AVB_VERIFY_H #include <../lib/libavb/libavb.h> +#include <blk.h> #include <mapmem.h> -#include <mmc.h> +#include <part.h> #define AVB_MAX_ARGS 1024 #define VERITY_TABLE_OPT_RESTART "restart_on_corruption" @@ -24,7 +25,8 @@ enum avb_boot_state { struct AvbOpsData { struct AvbOps ops; - int mmc_dev; + enum uclass_id uclass_id; + int dev_num; enum avb_boot_state boot_state; #ifdef CONFIG_OPTEE_TA_AVB struct udevice *tee; @@ -33,8 +35,6 @@ struct AvbOpsData { }; struct avb_blk_part { - int dev_num; - struct mmc *mmc; struct blk_desc *blk; struct disk_partition info; }; @@ -44,7 +44,7 @@ enum avb_io_type { AVB_IO_WRITE }; -AvbOps *avb_ops_alloc(int boot_device); +AvbOps *avb_ops_alloc(enum uclass_id uclass_id, int dev_num); void avb_ops_free(AvbOps *ops); /** @@ -110,10 +110,24 @@ static inline int get_boot_device(AvbOps *ops) if (ops) { data = ops->user_data; if (data) - return data->mmc_dev; + return data->dev_num; } return -1; } +/* Mirrors get_boot_device(): no ops means no interface, not a default one */ +static inline enum uclass_id get_boot_interface(AvbOps *ops) +{ + struct AvbOpsData *data; + + if (ops) { + data = ops->user_data; + if (data) + return data->uclass_id; + } + + return UCLASS_INVALID; +} + #endif /* _AVB_VERIFY_H */ diff --git a/test/py/tests/test_android/test_avb.py b/test/py/tests/test_android/test_avb.py index 137d83e1de..4c5b3c6f59 100644 --- a/test/py/tests/test_android/test_avb.py +++ b/test/py/tests/test_android/test_avb.py @@ -135,3 +135,31 @@ def test_avb_persistent_values(ubman): response = ubman.run_command('avb read_pvalue test 12') assert response == 'Read 12 bytes, value = value_value' + + [email protected]('cmd_avb') [email protected]('cmd_mmc') +def test_avb_init_interface(ubman): + """Check that 'avb init' accepts an explicit interface + """ + + response = ubman.run_command('avb init mmc %s' % str(mmc_dev)) + assert response == '' + + response = ubman.run_command('avb init %s' % str(mmc_dev)) + assert response == '' + + [email protected]('cmd_avb') +def test_avb_init_bad_device(ubman): + """Check that 'avb init' rejects a device that does not exist + """ + + response = ubman.run_command('avb init mmc ff') + assert response.find("Can't find the requested device") != -1 + + response = ubman.run_command('avb init ff') + assert response.find("Can't find the requested device") != -1 + + response = ubman.run_command('avb init nonsense 0') + assert response.find("Can't find the requested device") != -1 -- 2.43.0
