libavb: Load entire partition if |allow_verification_error| is true.
This is needed to make the common workflow of
$ fastboot flash boot /path/to/boot.img
work. To do this we need to introduce a new AvbOps operation to get
the partition size. Note that libavb integrators had already had to do
this to implement read_from_partition() when a negative offset is
passed... so this shouldn't be a lot of extra work.
Also note that since libavb has no stable API this is not a breaking
change so there is no need to bump any version numbers (version
numbers are mostly for on-disk formats) ... in other words, libavb
integrators are expected to re-integrate and re-test their bootloader
code every time they uprev to a newer libavb. In this case they need
to implement a new AvbOps operation.
Add some extra docs to AvbOps to spell out that the struct should be
zeroed before being populated with function pointers. This is to
ensure unimplemented operations are always set to NULL.
For now handle the case where the newly operation is NULL (e.g. not
implemented) and just warn using avb_error() that it should be
implemented.
Add pre-condition checks to avb_slot_verify() to check that all
required operations at least are set.
Add a new unit test for this and also implement it in the
examples/uefi boot loader and libavb_user.
Bug: 37709309
Test: New unit test + all unit tests pass.
Test: Manually tested on UEFI-based bootloader.
Change-Id: Id225af91add2e52167994e80b5b3a788c6909c15
diff --git a/examples/uefi/Makefile b/examples/uefi/Makefile
index 6349057..e19a273 100644
--- a/examples/uefi/Makefile
+++ b/examples/uefi/Makefile
@@ -62,7 +62,7 @@
EFI_SHARED_OBJ = $(patsubst %.efi,%.so,$(EFI_TARGET))
EFI_CFLAGS = \
- -DAVB_COMPILATION -std=gnu99 \
+ -DAVB_COMPILATION -DAVB_ENABLE_DEBUG -std=gnu99 \
-I$(ANDROID_BUILD_TOP)/system/core/mkbootimg/ \
-I$(AVB) \
-I$(GNUEFI) \
diff --git a/examples/uefi/uefi_avb_ops.c b/examples/uefi/uefi_avb_ops.c
index e849b71..bdbcc2c 100644
--- a/examples/uefi/uefi_avb_ops.c
+++ b/examples/uefi/uefi_avb_ops.c
@@ -295,6 +295,34 @@
return AVB_IO_RESULT_OK;
}
+static AvbIOResult get_size_of_partition(AvbOps* ops,
+ const char* partition_name,
+ uint64_t* out_size) {
+ EFI_STATUS err;
+ GPTEntry* partition_entry;
+ uint64_t partition_size;
+ UEFIAvbOpsData* data = (UEFIAvbOpsData*)ops->user_data;
+
+ avb_assert(partition_name != NULL);
+
+ err = find_partition_entry_by_name(
+ data->block_io, partition_name, &partition_entry);
+ if (EFI_ERROR(err)) {
+ return AVB_IO_RESULT_ERROR_NO_SUCH_PARTITION;
+ }
+
+ partition_size =
+ (partition_entry->last_lba - partition_entry->first_lba + 1) *
+ data->block_io->Media->BlockSize;
+
+ if (out_size != NULL) {
+ *out_size = partition_size;
+ }
+
+ avb_free(partition_entry);
+ return AVB_IO_RESULT_OK;
+}
+
/* Helper method to get the parent path to the current |walker| path
* given the initial path, |init|. Resulting path is stored in |next|.
* Caller is responsible for freeing |next|. Stores allocated bytes
@@ -618,6 +646,7 @@
data->ops.write_rollback_index = write_rollback_index;
data->ops.read_is_device_unlocked = read_is_device_unlocked;
data->ops.get_unique_guid_for_partition = get_unique_guid_for_partition;
+ data->ops.get_size_of_partition = get_size_of_partition;
data->ab_ops.ops = &data->ops;
data->ab_ops.read_ab_metadata = avb_ab_data_read;
diff --git a/libavb/avb_ops.h b/libavb/avb_ops.h
index 908c66c..de36b59 100644
--- a/libavb/avb_ops.h
+++ b/libavb/avb_ops.h
@@ -71,6 +71,10 @@
/* High-level operations/functions/methods that are platform
* dependent.
+ *
+ * Operations may be added in the future so when implementing it
+ * always make sure to zero out sizeof(AvbOps) bytes of the struct to
+ * ensure that unimplemented operations are set to NULL.
*/
struct AvbOps {
/* This pointer can be used by the application/bootloader using
@@ -205,6 +209,16 @@
const char* partition,
char* guid_buf,
size_t guid_buf_size);
+
+ /* Gets the size of a partition with the name in |partition|
+ * (NUL-terminated UTF-8 string). Returns the value in
+ * |out_size_num_bytes|.
+ *
+ * Returns AVB_IO_RESULT_OK on success, otherwise an error code.
+ */
+ AvbIOResult (*get_size_of_partition)(AvbOps* ops,
+ const char* partition,
+ uint64_t* out_size_num_bytes);
};
#ifdef __cplusplus
diff --git a/libavb/avb_slot_verify.c b/libavb/avb_slot_verify.c
index 469a788..3218605 100644
--- a/libavb/avb_slot_verify.c
+++ b/libavb/avb_slot_verify.c
@@ -87,6 +87,7 @@
uint8_t* digest;
size_t digest_len;
const char* found;
+ uint64_t image_size;
if (!avb_hash_descriptor_validate_and_byteswap(
(const AvbHashDescriptor*)descriptor, &hash_desc)) {
@@ -116,18 +117,44 @@
goto out;
}
- image_buf = avb_malloc(hash_desc.image_size);
+ /* If we're allowing verification errors then hash_desc.image_size
+ * may no longer match what's in the partition... so in this case
+ * just load the entire partition.
+ *
+ * For example, this can happen if a developer does 'fastboot flash
+ * boot /path/to/new/and/bigger/boot.img'. We want this to work
+ * since it's such a common workflow.
+ */
+ image_size = hash_desc.image_size;
+ if (allow_verification_error) {
+ if (ops->get_size_of_partition == NULL) {
+ avb_errorv(part_name,
+ ": The get_size_of_partition() operation is "
+ "not implemented so we may not load the entire partition. "
+ "Please implement.",
+ NULL);
+ } else {
+ io_ret = ops->get_size_of_partition(ops, part_name, &image_size);
+ if (io_ret == AVB_IO_RESULT_ERROR_OOM) {
+ ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM;
+ goto out;
+ } else if (io_ret != AVB_IO_RESULT_OK) {
+ avb_errorv(part_name, ": Error determining partition size.\n", NULL);
+ ret = AVB_SLOT_VERIFY_RESULT_ERROR_IO;
+ goto out;
+ }
+ avb_debugv(part_name, ": Loading entire partition.\n", NULL);
+ }
+ }
+
+ image_buf = avb_malloc(image_size);
if (image_buf == NULL) {
ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM;
goto out;
}
- io_ret = ops->read_from_partition(ops,
- part_name,
- 0 /* offset */,
- hash_desc.image_size,
- image_buf,
- &part_num_read);
+ io_ret = ops->read_from_partition(
+ ops, part_name, 0 /* offset */, image_size, image_buf, &part_num_read);
if (io_ret == AVB_IO_RESULT_ERROR_OOM) {
ret = AVB_SLOT_VERIFY_RESULT_ERROR_OOM;
goto out;
@@ -136,7 +163,7 @@
ret = AVB_SLOT_VERIFY_RESULT_ERROR_IO;
goto out;
}
- if (part_num_read != hash_desc.image_size) {
+ if (part_num_read != image_size) {
avb_errorv(part_name, ": Read fewer than requested bytes.\n", NULL);
ret = AVB_SLOT_VERIFY_RESULT_ERROR_IO;
goto out;
@@ -196,7 +223,7 @@
loaded_partition =
&slot_data->loaded_partitions[slot_data->num_loaded_partitions++];
loaded_partition->partition_name = avb_strdup(found);
- loaded_partition->data_size = hash_desc.image_size;
+ loaded_partition->data_size = image_size;
loaded_partition->data = image_buf;
image_buf = NULL;
}
@@ -930,6 +957,18 @@
AvbIOResult io_ret;
bool using_boot_for_vbmeta = false;
+ /* Fail early if we're missing the AvbOps needed for slot verification.
+ *
+ * For now, handle get_size_of_partition() not being implemented. In
+ * a later release we may change that.
+ */
+ avb_assert(ops->read_is_device_unlocked != NULL);
+ avb_assert(ops->read_from_partition != NULL);
+ avb_assert(ops->validate_vbmeta_public_key != NULL);
+ avb_assert(ops->read_rollback_index != NULL);
+ avb_assert(ops->get_unique_guid_for_partition != NULL);
+ /* avb_assert(ops->get_size_of_partition != NULL); */
+
if (out_data != NULL) {
*out_data = NULL;
}
diff --git a/libavb/avb_slot_verify.h b/libavb/avb_slot_verify.h
index 1c9aece..ff9d985 100644
--- a/libavb/avb_slot_verify.h
+++ b/libavb/avb_slot_verify.h
@@ -218,6 +218,11 @@
* returned if, and only if, there are no errors. This mode is needed
* to boot valid but unverified slots when the device is unlocked.
*
+ * Also, if |allow_verification_error| is true then the contents
+ * loaded from |requested_partition| will be the contents of the
+ * entire partition instead of just the size specified in the hash
+ * descriptor.
+ *
* Also note that |out_data| is never set if
* AVB_SLOT_VERIFY_RESULT_ERROR_OOM, AVB_SLOT_VERIFY_RESULT_ERROR_IO,
* or AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_METADATA is returned.
diff --git a/libavb_user/avb_ops_user.c b/libavb_user/avb_ops_user.c
index 1611450..eef4536 100644
--- a/libavb_user/avb_ops_user.c
+++ b/libavb_user/avb_ops_user.c
@@ -256,6 +256,39 @@
return AVB_IO_RESULT_OK;
}
+static AvbIOResult get_size_of_partition(AvbOps* ops,
+ const char* partition,
+ uint64_t* out_size_in_bytes) {
+ int fd;
+ AvbIOResult ret;
+
+ fd = open_partition(partition, O_WRONLY);
+ if (fd == -1) {
+ avb_errorv("Error opening \"", partition, "\" partition.\n", NULL);
+ ret = AVB_IO_RESULT_ERROR_IO;
+ goto out;
+ }
+
+ if (out_size_in_bytes != NULL) {
+ if (ioctl(fd, BLKGETSIZE64, out_size_in_bytes) != 0) {
+ avb_errorv(
+ "Error getting size of \"", partition, "\" partition.\n", NULL);
+ ret = AVB_IO_RESULT_ERROR_IO;
+ goto out;
+ }
+ }
+
+ ret = AVB_IO_RESULT_OK;
+
+out:
+ if (fd != -1) {
+ if (close(fd) != 0) {
+ avb_error("Error closing file descriptor.\n");
+ }
+ }
+ return ret;
+}
+
static AvbIOResult get_unique_guid_for_partition(AvbOps* ops,
const char* partition,
char* guid_buf,
@@ -290,6 +323,7 @@
ops->write_rollback_index = write_rollback_index;
ops->read_is_device_unlocked = read_is_device_unlocked;
ops->get_unique_guid_for_partition = get_unique_guid_for_partition;
+ ops->get_size_of_partition = get_size_of_partition;
ops->ab_ops->read_ab_metadata = avb_ab_data_read;
ops->ab_ops->write_ab_metadata = avb_ab_data_write;
diff --git a/libavb_user/avb_ops_user.h b/libavb_user/avb_ops_user.h
index 3e02295..95a56b6 100644
--- a/libavb_user/avb_ops_user.h
+++ b/libavb_user/avb_ops_user.h
@@ -36,11 +36,11 @@
*
* The returned AvbOps has the following characteristics:
*
- * - The read_from_partition() and write_to_partition() operations are
- * implemented, however for these operations to work the fstab file
- * on the device must have a /misc entry using a by-name device file
- * scheme and the containing by-name/ subdirectory must have files
- * for other partitions.
+ * - The read_from_partition(), write_to_partition(), and
+ * get_size_of_partition() operations are implemented, however for
+ * these operations to work the fstab file on the device must have a
+ * /misc entry using a by-name device file scheme and the containing
+ * by-name/ subdirectory must have files for other partitions.
*
* - The remaining operations are implemented and never fails and
* return the following values:
diff --git a/test/avb_atx_validate_unittest.cc b/test/avb_atx_validate_unittest.cc
index 1436b0b..08ddf8e 100644
--- a/test/avb_atx_validate_unittest.cc
+++ b/test/avb_atx_validate_unittest.cc
@@ -156,6 +156,13 @@
return AVB_IO_RESULT_ERROR_NO_SUCH_PARTITION;
}
+ AvbIOResult get_size_of_partition(AvbOps* ops,
+ const char* partition,
+ uint64_t* out_size) override {
+ // Expect method not used.
+ return AVB_IO_RESULT_ERROR_NO_SUCH_PARTITION;
+ }
+
AvbIOResult read_permanent_attributes(
AvbAtxPermanentAttributes* attributes) override {
if (fail_read_permanent_attributes_) {
@@ -604,6 +611,12 @@
ops, partition, guid_buf, guid_buf_size);
}
+ AvbIOResult get_size_of_partition(AvbOps* ops,
+ const char* partition,
+ uint64_t* out_size) override {
+ return ops_.get_size_of_partition(ops, partition, out_size);
+ }
+
AvbIOResult read_permanent_attributes(
AvbAtxPermanentAttributes* attributes) override {
return ops_.read_permanent_attributes(attributes);
diff --git a/test/avb_slot_verify_unittest.cc b/test/avb_slot_verify_unittest.cc
index 8d6b9cb..124ccdd 100644
--- a/test/avb_slot_verify_unittest.cc
+++ b/test/avb_slot_verify_unittest.cc
@@ -341,6 +341,73 @@
avb_slot_verify_data_free(slot_data);
}
+TEST_F(AvbSlotVerifyTest, LoadEntirePartitionIfAllowingVerificationError) {
+ const size_t boot_partition_size = 16 * 1024 * 1024;
+ const size_t boot_image_size = 5 * 1024 * 1024;
+ const size_t new_boot_image_size = 10 * 1024 * 1024;
+ base::FilePath boot_path = GenerateImage("boot_a.img", boot_image_size);
+
+ // If we're allowing verification errors then check that the whole
+ // partition is loaded. This is needed because in this mode for
+ // example the "boot" partition might be flashed with another
+ // boot.img that is larger than what the HashDescriptor in vbmeta
+ // says.
+ EXPECT_COMMAND(
+ 0,
+ "./avbtool add_hash_footer"
+ " --image %s"
+ " --rollback_index 0"
+ " --partition_name boot"
+ " --partition_size %zd"
+ " --kernel_cmdline 'cmdline in hash footer $(ANDROID_SYSTEM_PARTUUID)'"
+ " --salt deadbeef"
+ " --internal_release_string \"\"",
+ boot_path.value().c_str(),
+ boot_partition_size);
+
+ GenerateVBMetaImage(
+ "vbmeta_a.img",
+ "SHA256_RSA2048",
+ 4,
+ base::FilePath("test/data/testkey_rsa2048.pem"),
+ base::StringPrintf(
+ "--include_descriptors_from_image %s"
+ " --kernel_cmdline 'cmdline in vbmeta $(ANDROID_BOOT_PARTUUID)'"
+ " --internal_release_string \"\"",
+ boot_path.value().c_str()));
+
+ // Now replace the boot partition with something bigger and
+ // different. Because FakeOps's get_size_of_partition() operation
+ // just returns the file size it means that this is what is returned
+ // by get_size_of_partition().
+ //
+ // Also make sure this image will return a different digest by using
+ // a non-standard starting byte. This is to force avb_slot_verify()
+ // to return ERROR_VERIFICATION below.
+ GenerateImage("boot_a.img", new_boot_image_size, 1 /* start_byte */);
+
+ ops_.set_expected_public_key(
+ PublicKeyAVB(base::FilePath("test/data/testkey_rsa2048.pem")));
+
+ AvbSlotVerifyData* slot_data = NULL;
+ const char* requested_partitions[] = {"boot", NULL};
+ EXPECT_EQ(AVB_SLOT_VERIFY_RESULT_ERROR_VERIFICATION,
+ avb_slot_verify(ops_.avb_ops(),
+ requested_partitions,
+ "_a",
+ true /* allow_verification_error */,
+ &slot_data));
+ EXPECT_NE(nullptr, slot_data);
+
+ // Check that the loaded partition is actually
+ // |new_boot_image_size|.
+ EXPECT_EQ(size_t(1), slot_data->num_loaded_partitions);
+ EXPECT_EQ("boot",
+ std::string(slot_data->loaded_partitions[0].partition_name));
+ EXPECT_EQ(new_boot_image_size, slot_data->loaded_partitions[0].data_size);
+ avb_slot_verify_data_free(slot_data);
+}
+
TEST_F(AvbSlotVerifyTest, HashDescriptorInVBMeta) {
const size_t boot_partition_size = 16 * 1024 * 1024;
const size_t boot_image_size = 5 * 1024 * 1024;
diff --git a/test/avb_unittest_util.h b/test/avb_unittest_util.h
index 5f65180..abddb28 100644
--- a/test/avb_unittest_util.h
+++ b/test/avb_unittest_util.h
@@ -101,11 +101,13 @@
/* Generate a file with name |file_name| of size |image_size| with
* known content (0x00 0x01 0x02 .. 0xff 0x00 0x01 ..).
*/
- base::FilePath GenerateImage(const std::string file_name, size_t image_size) {
+ base::FilePath GenerateImage(const std::string file_name,
+ size_t image_size,
+ uint8_t start_byte = 0) {
std::vector<uint8_t> image;
image.resize(image_size);
for (size_t n = 0; n < image_size; n++) {
- image[n] = uint8_t(n);
+ image[n] = uint8_t(n + start_byte);
}
base::FilePath image_path = testdir_.Append(file_name);
EXPECT_EQ(image_size,
diff --git a/test/fake_avb_ops.cc b/test/fake_avb_ops.cc
index 6e346e1..c5563c8 100644
--- a/test/fake_avb_ops.cc
+++ b/test/fake_avb_ops.cc
@@ -223,6 +223,21 @@
return AVB_IO_RESULT_OK;
}
+AvbIOResult FakeAvbOps::get_size_of_partition(AvbOps* ops,
+ const char* partition,
+ uint64_t* out_size) {
+ base::FilePath path =
+ partition_dir_.Append(std::string(partition)).AddExtension("img");
+
+ int64_t file_size;
+ if (!base::GetFileSize(path, &file_size)) {
+ fprintf(stderr, "Error getting size of file '%s'\n", path.value().c_str());
+ return AVB_IO_RESULT_ERROR_IO;
+ }
+ *out_size = file_size;
+ return AVB_IO_RESULT_OK;
+}
+
AvbIOResult FakeAvbOps::read_permanent_attributes(
AvbAtxPermanentAttributes* attributes) {
*attributes = permanent_attributes_;
@@ -312,6 +327,14 @@
->get_unique_guid_for_partition(ops, partition, guid_buf, guid_buf_size);
}
+static AvbIOResult my_ops_get_size_of_partition(AvbOps* ops,
+ const char* partition,
+ uint64_t* out_size) {
+ return FakeAvbOps::GetInstanceFromAvbOps(ops)
+ ->delegate()
+ ->get_size_of_partition(ops, partition, out_size);
+}
+
static AvbIOResult my_ops_read_permanent_attributes(
AvbAtxOps* atx_ops, AvbAtxPermanentAttributes* attributes) {
return FakeAvbOps::GetInstanceFromAvbOps(atx_ops->ops)
@@ -337,6 +360,7 @@
avb_ops_.write_rollback_index = my_ops_write_rollback_index;
avb_ops_.read_is_device_unlocked = my_ops_read_is_device_unlocked;
avb_ops_.get_unique_guid_for_partition = my_ops_get_unique_guid_for_partition;
+ avb_ops_.get_size_of_partition = my_ops_get_size_of_partition;
// Just use the built-in A/B metadata read/write routines.
avb_ab_ops_.ops = &avb_ops_;
diff --git a/test/fake_avb_ops.h b/test/fake_avb_ops.h
index 0a7246d..612c62c 100644
--- a/test/fake_avb_ops.h
+++ b/test/fake_avb_ops.h
@@ -74,6 +74,10 @@
char* guid_buf,
size_t guid_buf_size) = 0;
+ virtual AvbIOResult get_size_of_partition(AvbOps* ops,
+ const char* partition,
+ uint64_t* out_size) = 0;
+
virtual AvbIOResult read_permanent_attributes(
AvbAtxPermanentAttributes* attributes) = 0;
@@ -185,6 +189,10 @@
char* guid_buf,
size_t guid_buf_size) override;
+ AvbIOResult get_size_of_partition(AvbOps* ops,
+ const char* partition,
+ uint64_t* out_size) override;
+
AvbIOResult read_permanent_attributes(
AvbAtxPermanentAttributes* attributes) override;
