bootmeth_android let libavb read boot and vendor_boot into its own buffers, hash them and free them, and then read both partitions again to boot them. The bytes that were verified and the bytes that were booted were therefore two separate reads of the storage device, which an attacker able to interpose the device can tell apart (reported on the list as a TOCTOU issue). It also read every image twice.
Load boot and vendor_boot first and hand the buffers to libavb through AvbOps.get_preloaded_partition(), so the hashes are computed over the memory that bootm consumes afterwards. If a hash descriptor covers more bytes than the boot image header announced, the tail is read into the same buffer, bounded by the partition size; a descriptor larger than the buffer fails verification. Reported-by: Ahmad Fatoum <[email protected]> Signed-off-by: Igor Opaniuk <[email protected]> --- boot/bootmeth_android.c | 73 ++++++++++++++++++++++++++++++++++++-------- common/avb_verify.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++ include/avb_verify.h | 36 ++++++++++++++++++++++ 3 files changed, 177 insertions(+), 12 deletions(-) diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c index 0db08d4f861..862d0d67a2f 100644 --- a/boot/bootmeth_android.c +++ b/boot/bootmeth_android.c @@ -48,6 +48,11 @@ struct android_priv { u32 header_version; u32 boot_img_size; u32 vendor_boot_img_size; + /* Where boot and vendor_boot were loaded, and the partition sizes */ + ulong boot_addr; + ulong boot_part_size; + ulong vendor_boot_addr; + ulong vendor_boot_part_size; }; static int android_check(struct udevice *dev, struct bootflow_iter *iter) @@ -353,7 +358,8 @@ static int android_read_file(struct udevice *dev, struct bootflow *bflow, * Return: 0 if OK, negative errno on failure. */ static int read_slotted_partition(struct blk_desc *desc, const char *const name, - const char slot[2], ulong image_size, ulong addr) + const char slot[2], ulong image_size, ulong addr, + ulong *part_size) { struct disk_partition partition; char partname[PART_NAME_LEN]; @@ -396,6 +402,9 @@ static int read_slotted_partition(struct blk_desc *desc, const char *const name, if (n < num_blks) return log_msg_ret("part read", -EIO); + if (part_size) + *part_size = (ulong)partition.size * desc->blksz; + return 0; } @@ -440,6 +449,7 @@ static int run_avb_verification(struct bootflow *bflow) enum avb_boot_state boot_state; char *extra_args; char slot_suffix[3] = ""; + char partname[PART_NAME_LEN]; bool unlocked = false; int ret; @@ -450,6 +460,35 @@ static int run_avb_verification(struct bootflow *bflow) if (priv->slot) sprintf(slot_suffix, "_%s", priv->slot); + /* + * Hand libavb the buffers boot and vendor_boot were loaded into, so the + * bytes it hashes are the bytes that get booted and the partitions are + * read only once. + */ + if (priv->boot_addr) { + snprintf(partname, sizeof(partname), "boot%s", slot_suffix); + ret = avb_ops_add_preloaded(avb_ops, partname, + map_sysmem(priv->boot_addr, 0), + priv->boot_img_size, + priv->boot_part_size); + if (ret) { + ret = log_msg_ret("avb preload boot", ret); + goto out; + } + } + if (priv->vendor_boot_addr) { + snprintf(partname, sizeof(partname), "vendor_boot%s", + slot_suffix); + ret = avb_ops_add_preloaded(avb_ops, partname, + map_sysmem(priv->vendor_boot_addr, 0), + priv->vendor_boot_img_size, + priv->vendor_boot_part_size); + if (ret) { + ret = log_msg_ret("avb preload vendor_boot", ret); + goto out; + } + } + ret = avb_ops->read_is_device_unlocked(avb_ops, &unlocked); if (ret != AVB_IO_RESULT_OK) { ret = log_msg_ret("avb lock", -EIO); @@ -566,6 +605,26 @@ static int boot_android_normal(struct bootflow *bflow) ulong loadaddr = env_get_hex("loadaddr", 0); ulong vloadaddr = env_get_hex("vendor_boot_comp_addr_r", 0); + /* + * Load the images first and verify them in place: libavb hashes these + * buffers (see run_avb_verification()), so a storage device that + * returns different data on a second read cannot slip past the check. + */ + ret = read_slotted_partition(desc, "boot", priv->slot, priv->boot_img_size, + loadaddr, &priv->boot_part_size); + if (ret < 0) + return log_msg_ret("read boot", ret); + priv->boot_addr = loadaddr; + + if (priv->header_version >= 3) { + ret = read_slotted_partition(desc, "vendor_boot", priv->slot, + priv->vendor_boot_img_size, vloadaddr, + &priv->vendor_boot_part_size); + if (ret < 0) + return log_msg_ret("read vendor_boot", ret); + priv->vendor_boot_addr = vloadaddr; + } + ret = run_avb_verification(bflow); if (ret < 0) return log_msg_ret("avb", ret); @@ -575,18 +634,8 @@ static int boot_android_normal(struct bootflow *bflow) if (ret < 0) return log_msg_ret("read slot", ret); - ret = read_slotted_partition(desc, "boot", priv->slot, priv->boot_img_size, - loadaddr); - if (ret < 0) - return log_msg_ret("read boot", ret); - - if (priv->header_version >= 3) { - ret = read_slotted_partition(desc, "vendor_boot", priv->slot, - priv->vendor_boot_img_size, vloadaddr); - if (ret < 0) - return log_msg_ret("read vendor_boot", ret); + if (priv->header_version >= 3) set_avendor_bootimg_addr(vloadaddr); - } set_abootimg_addr(loadaddr); if (priv->slot) diff --git a/common/avb_verify.c b/common/avb_verify.c index 9231366d5fa..e7685c29d65 100644 --- a/common/avb_verify.c +++ b/common/avb_verify.c @@ -1079,6 +1079,85 @@ AvbIOResult avb_write_lock_state(AvbOps *ops, bool locked) } #endif +/** + * get_preloaded_partition() - hand libavb a partition the caller loaded + * + * Without this callback libavb reads every requested partition into its own + * buffer, hashes that copy and frees it, and the bootloader then reads the + * partition a second time to boot it: the verified bytes and the booted bytes + * are different reads of the storage device. Serving the caller's buffer makes + * libavb hash the memory that is booted afterwards. + * + * The boot image header may describe fewer bytes than the hash descriptor + * covers; the missing tail is then read into the same buffer, up to its + * capacity, before it is handed out. + */ +static AvbIOResult get_preloaded_partition(AvbOps *ops, const char *partition, + size_t num_bytes, + uint8_t **out_pointer, + size_t *out_num_bytes_preloaded) +{ + struct AvbOpsData *ops_data = ops->user_data; + struct avb_preloaded *p = NULL; + size_t num_read = 0; + AvbIOResult rc; + int i; + + *out_pointer = NULL; + *out_num_bytes_preloaded = 0; + + for (i = 0; i < ops_data->num_preloaded; i++) { + if (!strcmp(ops_data->preloaded[i].name, partition)) { + p = &ops_data->preloaded[i]; + break; + } + } + if (!p) + return AVB_IO_RESULT_OK; /* not preloaded, libavb reads it */ + + if (num_bytes > p->max_size) { + printf("%s: %s: %zu bytes to verify, buffer holds %zu\n", + __func__, partition, num_bytes, p->max_size); + return AVB_IO_RESULT_ERROR_INSUFFICIENT_SPACE; + } + + if (num_bytes > p->size) { + rc = read_from_partition(ops, partition, p->size, + num_bytes - p->size, + (u8 *)p->addr + p->size, &num_read); + if (rc != AVB_IO_RESULT_OK) + return rc; + if (num_read != num_bytes - p->size) + return AVB_IO_RESULT_ERROR_IO; + p->size = num_bytes; + } + + *out_pointer = p->addr; + *out_num_bytes_preloaded = num_bytes; + + return AVB_IO_RESULT_OK; +} + +int avb_ops_add_preloaded(AvbOps *ops, const char *partition, void *addr, + size_t size, size_t max_size) +{ + struct AvbOpsData *ops_data = ops->user_data; + struct avb_preloaded *p; + + if (ops_data->num_preloaded >= AVB_MAX_PRELOADED) + return -ENOSPC; + if (strlen(partition) >= sizeof(p->name) || size > max_size) + return -EINVAL; + + p = &ops_data->preloaded[ops_data->num_preloaded++]; + strcpy(p->name, partition); + p->addr = addr; + p->size = size; + p->max_size = max_size; + + return 0; +} + /** * ============================================================================ * AVB root key digest providers (selected via CONFIG_AVB_ROOT_KEY_*) @@ -1146,6 +1225,7 @@ AvbOps *avb_ops_alloc(int boot_device) ops_data->ops.user_data = ops_data; ops_data->ops.read_from_partition = read_from_partition; + ops_data->ops.get_preloaded_partition = get_preloaded_partition; ops_data->ops.write_to_partition = write_to_partition; ops_data->ops.validate_vbmeta_public_key = validate_vbmeta_public_key; ops_data->ops.read_rollback_index = read_rollback_index; diff --git a/include/avb_verify.h b/include/avb_verify.h index d2136b2ce02..cac8b816c03 100644 --- a/include/avb_verify.h +++ b/include/avb_verify.h @@ -22,6 +22,26 @@ enum avb_boot_state { AVB_RED, }; +#define AVB_MAX_PRELOADED 4 + +/** + * struct avb_preloaded - a partition already loaded into memory by the caller + * + * libavb hashes the preloaded bytes instead of reading the partition again, + * so what gets verified is exactly what the bootloader goes on to boot. + * + * @name: partition name including the slot suffix, e.g. "boot_a" + * @addr: start of the buffer holding the partition data + * @size: number of bytes loaded so far + * @max_size: capacity of the buffer (at most the partition size) + */ +struct avb_preloaded { + char name[32]; + void *addr; + size_t size; + size_t max_size; +}; + struct AvbOpsData { struct AvbOps ops; int mmc_dev; @@ -30,6 +50,8 @@ struct AvbOpsData { struct udevice *tee; u32 session; #endif + struct avb_preloaded preloaded[AVB_MAX_PRELOADED]; + int num_preloaded; }; struct mmc_part { @@ -45,6 +67,20 @@ enum mmc_io_type { }; AvbOps *avb_ops_alloc(int boot_device); + +/** + * avb_ops_add_preloaded() - register a partition that is already in memory + * + * @ops: AvbOps from avb_ops_alloc() + * @partition: partition name including the slot suffix, e.g. "boot_a" + * @addr: buffer holding the first @size bytes of the partition + * @size: bytes loaded in @addr + * @max_size: capacity of @addr; a hash descriptor covering more than this + * fails verification instead of overflowing the buffer + * Return: 0 on success, -ENOSPC when no slot is left, -EINVAL on a bad name + */ +int avb_ops_add_preloaded(AvbOps *ops, const char *partition, void *addr, + size_t size, size_t max_size); #ifdef CONFIG_OPTEE_TA_AVB AvbIOResult avb_write_lock_state(AvbOps *ops, bool locked); #endif -- 2.53.0
