From b688d42ad10e99b2afad6fa3ad2d4179cecb19dd Mon Sep 17 00:00:00 2001 From: Furquan Shaikh Date: Thu, 15 Jun 2017 11:04:49 -0700 Subject: [PATCH] futility: Fix issues with validation of recovery MRC cache 1. Current assumption in the validation function is that there is only 1 metadata block present in the cache. However, this is not always true (e.g. KBL boards). Thus, update the check to ensure that only 1 metadata block is actually used if multiple such blocks are present. 2. Add a check to ensure that the offset provided is not greater than the file size. BUG=b:62654773 BRANCH=None TEST=Verified that "futility validate_rec_mrc" works fine with the image provided in bug. Also, verified this works fine for poppy. Change-Id: I84b55d1daf884326a2e970e2ac73110c5eeeaa45 Signed-off-by: Furquan Shaikh Reviewed-on: https://chromium-review.googlesource.com/537074 Reviewed-by: Aaron Durbin --- futility/cmd_validate_rec_mrc.c | 52 +++++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/futility/cmd_validate_rec_mrc.c b/futility/cmd_validate_rec_mrc.c index 1fdd47c194..6c068069c8 100644 --- a/futility/cmd_validate_rec_mrc.c +++ b/futility/cmd_validate_rec_mrc.c @@ -43,10 +43,11 @@ struct mrc_metadata { uint32_t version; } __attribute__((packed)); -#define MRC_DATA_SIGNATURE (('M'<<0)|('R'<<8)|('C'<<16)|('D'<<24)) -#define REGF_BLOCK_SHIFT 4 -#define REGF_UNALLOCATED_BLOCK 0xffff - +#define MRC_DATA_SIGNATURE (('M'<<0)|('R'<<8)|('C'<<16)|('D'<<24)) +#define REGF_BLOCK_SHIFT 4 +#define REGF_BLOCK_GRANULARITY (1 << REGF_BLOCK_SHIFT) +#define REGF_METADATA_BLOCK_SIZE REGF_BLOCK_GRANULARITY +#define REGF_UNALLOCATED_BLOCK 0xffff unsigned long compute_ip_checksum(const void *addr, unsigned long length) { @@ -116,18 +117,37 @@ static int verify_mrc_slot(struct mrc_metadata *md, unsigned long slot_len) return 0; } +static int block_offset_unallocated(uint16_t offset) +{ + return offset == REGF_UNALLOCATED_BLOCK; +} + +static uint8_t *get_next_mb(uint8_t *curr_mb) +{ + return curr_mb + REGF_METADATA_BLOCK_SIZE; +} + static int get_mrc_data_slot(uint16_t *mb, uint32_t *data_offset, uint32_t *data_size) { + uint16_t num_metadata_blocks = *mb; + + if (block_offset_unallocated(*mb)) { + fprintf(stderr, "MRC cache is empty!!\n"); + return 1; + } + /* * First block offset in metadata block tells the total number of * metadata blocks. - * Currently, we expect only 1 metadata block to be present. + * Currently, we expect only 1 metadata block to be used. */ - if (*mb != 1) { - fprintf(stderr, "Only 1 metadata block is expected. " - "Actual %x\n", *mb); - return 1; + if (num_metadata_blocks != 1) { + uint16_t *next_mb = (uint16_t *)get_next_mb((uint8_t *)mb); + if (!block_offset_unallocated(*next_mb)) { + fprintf(stderr, "More than 1 valid metadata block!!"); + return 1; + } } /* @@ -136,11 +156,11 @@ static int get_mrc_data_slot(uint16_t *mb, uint32_t *data_offset, * cache slot. */ mb++; - *data_offset = (1 << REGF_BLOCK_SHIFT); - *data_size = (*mb - 1) << REGF_BLOCK_SHIFT; + *data_offset = (1 << REGF_BLOCK_SHIFT) * num_metadata_blocks; + *data_size = (*mb - num_metadata_blocks) << REGF_BLOCK_SHIFT; mb++; - if (*mb != REGF_UNALLOCATED_BLOCK) { + if (!block_offset_unallocated(*mb)) { fprintf(stderr, "More than 1 slot in recovery mrc cache.\n"); return 1; } @@ -209,6 +229,14 @@ static int do_validate_rec_mrc(int argc, char *argv[]) return 1; } + if (offset > file_size) { + fprintf(stderr, "File size(0x%x) smaller than offset(0x%x)\n", + file_size, offset); + futil_unmap_file(fd, MAP_RO, buff, file_size); + close(fd); + return 1; + } + if (get_mrc_data_slot((uint16_t *)(buff + offset), &data_offset, &data_size)) { fprintf(stderr, "Metadata block error\n");