Unpack AftlIcpEntry structure
Having an unpacked structure allows us to dereference pointer members
without running the risk of violating any alignment rule. We
previously did a partial read on the header to determine the size of the
proofs, declared them as a flexible array member and packed the
structure (see b09b12a). Enforce a manual allocation of the proof member
to avoid having this extra pre-read.
Move the AftlImageHeader closer to the AftlImage definition.
Test: m libavb_host_unittest && ./out/host/linux-x86/nativetest/libavb_host_unittest/libavb_host_unittest
Test: m libavb_host_unittest && ./out/host/linux-x86/nativetest64/libavb_host_unittest/libavb_host_unittest
Bug: 154303414
Change-Id: Iccb473a4600327811e35857f884001b37afdf361
diff --git a/libavb_aftl/avb_aftl_types.h b/libavb_aftl/avb_aftl_types.h
index 09610f3..c56b465 100644
--- a/libavb_aftl/avb_aftl_types.h
+++ b/libavb_aftl/avb_aftl_types.h
@@ -105,15 +105,6 @@
#define AVB_AFTL_MAX_TLRD_SIZE \
(AVB_AFTL_MIN_TLRD_SIZE + AVB_AFTL_MAX_METADATA_SIZE)
-/* Data structure containing AFTL header information. */
-typedef struct AftlImageHeader {
- uint32_t magic;
- uint32_t required_icp_version_major;
- uint32_t required_icp_version_minor;
- uint32_t image_size; /* Total size of the AftlImage, including this header */
- uint16_t icp_count;
-} AVB_ATTR_PACKED AftlImageHeader;
-
/* Data structure containing a Trillian LogRootDescriptor, from
https://github.com/google/trillian/blob/master/trillian.proto#L255
The log_root_signature is calculated over this structure. */
@@ -152,14 +143,23 @@
TrillianLogRootDescriptor log_root_descriptor;
FirmwareInfo fw_info_leaf;
uint8_t* log_root_signature;
- uint8_t proofs[/*proof_hash_count*/][AVB_AFTL_HASH_SIZE];
-} AVB_ATTR_PACKED AftlIcpEntry;
+ uint8_t (*proofs)[AVB_AFTL_HASH_SIZE];
+} AftlIcpEntry;
+
+/* Data structure containing AFTL header information. */
+typedef struct AftlImageHeader {
+ uint32_t magic;
+ uint32_t required_icp_version_major;
+ uint32_t required_icp_version_minor;
+ uint32_t image_size; /* Total size of the AftlImage, including this header */
+ uint16_t icp_count;
+} AVB_ATTR_PACKED AftlImageHeader;
/* Main data structure for an AFTL image. */
typedef struct AftlImage {
AftlImageHeader header;
AftlIcpEntry** entries;
-} AVB_ATTR_PACKED AftlImage;
+} AftlImage;
#ifdef __cplusplus
}
diff --git a/libavb_aftl/avb_aftl_util.c b/libavb_aftl/avb_aftl_util.c
index e3747e1..98387d1 100644
--- a/libavb_aftl/avb_aftl_util.c
+++ b/libavb_aftl/avb_aftl_util.c
@@ -624,27 +624,15 @@
The blob is expected to be pointing to the beginning of a
serialized AftlIcpEntry structure. */
AftlIcpEntry* parse_icp_entry(uint8_t** aftl_blob, size_t* remaining_size) {
- AftlIcpEntry *icp_entry, *tmp_icp_entry;
- uint32_t proof_size;
+ AftlIcpEntry* icp_entry;
uint64_t parsed_size;
- /* Make a temp AftlIcpEntry to get the inclusion proof size
- for memory allocation purposes.*/
- tmp_icp_entry = (AftlIcpEntry*)*aftl_blob;
- proof_size = avb_be32toh(tmp_icp_entry->inc_proof_size);
-
- /* Ensure the calculated size is sane. */
- if (proof_size > AVB_AFTL_MAX_PROOF_SIZE) {
- avb_error("Invalid inclusion proof size.\n");
- return NULL;
- }
-
- if (*remaining_size < proof_size + AVB_AFTL_MIN_AFTL_ICP_ENTRY_SIZE) {
+ if (*remaining_size < AVB_AFTL_MIN_AFTL_ICP_ENTRY_SIZE) {
avb_error("Invalid AftlImage\n");
return NULL;
}
- icp_entry = (AftlIcpEntry*)avb_calloc(proof_size + sizeof(AftlIcpEntry));
+ icp_entry = (AftlIcpEntry*)avb_calloc(sizeof(AftlIcpEntry));
if (!icp_entry) {
avb_error("Failure allocating AftlIcpEntry\n");
return NULL;
@@ -718,8 +706,9 @@
*aftl_blob,
avb_aftl_member_size(AftlIcpEntry, inc_proof_size));
icp_entry->inc_proof_size = avb_be32toh(icp_entry->inc_proof_size);
- if (icp_entry->inc_proof_size !=
- icp_entry->proof_hash_count * AVB_AFTL_HASH_SIZE) {
+ if ((icp_entry->inc_proof_size !=
+ icp_entry->proof_hash_count * AVB_AFTL_HASH_SIZE) ||
+ (icp_entry->inc_proof_size > AVB_AFTL_MAX_PROOF_SIZE)) {
avb_error("Invalid inclusion proof size.\n");
avb_free(icp_entry);
return NULL;
@@ -808,6 +797,11 @@
return NULL;
}
+ icp_entry->proofs = avb_calloc(icp_entry->inc_proof_size);
+ if (!icp_entry->proofs) {
+ free_aftl_icp_entry(icp_entry);
+ return NULL;
+ }
/* Finally, copy the proof hash data from the blob to the AftlImage. */
avb_memcpy(icp_entry->proofs, *aftl_blob, icp_entry->inc_proof_size);
*aftl_blob += icp_entry->inc_proof_size;
@@ -887,6 +881,7 @@
avb_free(icp_entry->log_root_descriptor.metadata);
if (icp_entry->log_root_descriptor.root_hash)
avb_free(icp_entry->log_root_descriptor.root_hash);
+ if (icp_entry->proofs) avb_free(icp_entry->proofs);
/* Finally, free the AftlIcpEntry. */
avb_free(icp_entry);
}
diff --git a/test/avb_aftl_validate_unittest.cc b/test/avb_aftl_validate_unittest.cc
index 84a723d..8a8e057 100644
--- a/test/avb_aftl_validate_unittest.cc
+++ b/test/avb_aftl_validate_unittest.cc
@@ -82,8 +82,7 @@
if (!log_sig_bytes_) return;
base::ReadFile(
base::FilePath(kAftlLogSigPath), (char*)log_sig_bytes_, log_sig_size_);
- icp_entry_ =
- (AftlIcpEntry*)avb_malloc(sizeof(AftlIcpEntry) + AVB_AFTL_HASH_SIZE);
+ icp_entry_ = (AftlIcpEntry*)avb_malloc(sizeof(AftlIcpEntry));
if (!icp_entry_) return;
icp_entry_->log_root_descriptor.version = 1;
icp_entry_->log_root_descriptor.tree_size = 3;
@@ -118,6 +117,8 @@
icp_entry_->fw_info_leaf_size);
icp_entry_->leaf_index = 2;
+ icp_entry_->proofs =
+ (uint8_t(*)[AVB_AFTL_HASH_SIZE])avb_calloc(AVB_AFTL_HASH_SIZE);
memcpy(icp_entry_->proofs[0],
"\xfa\xc5\x42\x03\xe7\xcc\x69\x6c\xf0\xdf\xcb\x42\xc9\x2a\x1d\x9d"
"\xba\xf7\x0a\xd9\xe6\x21\xf4\xbd\x8d\x98\x66\x2f\x00\xe3\xc1\x25",
@@ -140,6 +141,7 @@
avb_free(icp_entry_->fw_info_leaf.vbmeta_hash);
if (icp_entry_->log_root_descriptor.root_hash != NULL)
avb_free(icp_entry_->log_root_descriptor.root_hash);
+ if (icp_entry_->proofs != NULL) avb_free(icp_entry_->proofs);
avb_free(icp_entry_);
}
avb_free(key_bytes_);
