From 46d6b0471231f8fc9549666acf75eb16334f593e Mon Sep 17 00:00:00 2001 From: Vadim Bendebury Date: Tue, 28 Feb 2017 18:16:42 -0800 Subject: [PATCH] g: mark RW INFO rollback map space to match the header infomap field The cr50 RO image compares INFO rollback map space against the contents of the RW image's infomap field. To ensure that no rollback is possible, the RW should verify that the INFO space state is consistent with the current RW and RW_B headers, and if not, update the INFO state to comply. This should happen only when running prod images, so that debug images could be rolled back if so desired. Also fixed the bug in functions enabling read and write access to the INFO1 region. Write access is now a superset of read access setting. BRANCH=cr50 BUG=b:35774863 TEST=as follows: - built and ran a new image, observed it start successfully; - modified the manifest to erase the first map location, built and ran a new image, observed it start successfully - restored the manifest, built and tried running a new image, observed that the earlier version is starting. Change-Id: I62253c3e98cd24ed24424b8bb9de22692a262d89 Signed-off-by: Vadim Bendebury Reviewed-on: https://chromium-review.googlesource.com/447966 Reviewed-by: Marius Schilder Reviewed-by: Mary Ruthven --- board/cr50/board.c | 2 + chip/g/flash.c | 18 +++---- chip/g/flash_info.h | 16 +++++++ chip/g/system.c | 110 ++++++++++++++++++++++++++++++++++++++++++- chip/g/system_chip.h | 6 +++ 5 files changed, 142 insertions(+), 10 deletions(-) diff --git a/board/cr50/board.c b/board/cr50/board.c index fa308ac4ca..8faf81bb55 100644 --- a/board/cr50/board.c +++ b/board/cr50/board.c @@ -542,6 +542,8 @@ static void board_init(void) /* Initialize the persistent storage. */ initvars(); + system_update_rollback_mask(); + /* Indication that firmware is running, for debug purposes. */ GREG32(PMU, PWRDN_SCRATCH16) = 0xCAFECAFE; diff --git a/chip/g/flash.c b/chip/g/flash.c index c0b1f72979..f1089656a1 100644 --- a/chip/g/flash.c +++ b/chip/g/flash.c @@ -371,20 +371,20 @@ static int valid_info_range(uint32_t offset, size_t size) } +/* Write access is a superset of read access. */ static int flash_info_configure_access(uint32_t offset, - size_t size, int read_mode) + size_t size, int write_mode) { int mask; if (!valid_info_range(offset, size)) return EC_ERROR_INVAL; - if (read_mode) - mask = GC_GLOBALSEC_FLASH_REGION6_CTRL_EN_MASK | - GC_GLOBALSEC_FLASH_REGION6_CTRL_RD_EN_MASK; - else - mask = GC_GLOBALSEC_FLASH_REGION6_CTRL_EN_MASK | - GC_GLOBALSEC_FLASH_REGION6_CTRL_WR_EN_MASK; + mask = GREG32(GLOBALSEC, FLASH_REGION6_CTRL); + mask |= GC_GLOBALSEC_FLASH_REGION6_CTRL_EN_MASK | + GC_GLOBALSEC_FLASH_REGION6_CTRL_RD_EN_MASK; + if (write_mode) + mask |= GC_GLOBALSEC_FLASH_REGION6_CTRL_WR_EN_MASK; GREG32(GLOBALSEC, FLASH_REGION6_BASE_ADDR) = FLASH_INFO_MEMORY_BASE + offset; @@ -397,12 +397,12 @@ static int flash_info_configure_access(uint32_t offset, int flash_info_read_enable(uint32_t offset, size_t size) { - return flash_info_configure_access(offset, size, 1); + return flash_info_configure_access(offset, size, 0); } int flash_info_write_enable(uint32_t offset, size_t size) { - return flash_info_configure_access(offset, size, 0); + return flash_info_configure_access(offset, size, 1); } void flash_info_write_disable(void) diff --git a/chip/g/flash_info.h b/chip/g/flash_info.h index 308446df4d..b80c6889ba 100644 --- a/chip/g/flash_info.h +++ b/chip/g/flash_info.h @@ -8,7 +8,23 @@ #include +#include "signed_header.h" + +/* + * Info1 space available to the app firmware is split in several areas. Of + * interest are the two spaces used for rollback prevention of RO and RW image + * versions. + * + * Each bit in the image infomap header section is mapped into a 4 byte word + * in the Info1 space. + */ +#define INFO_RO_MAP_OFFSET 0 +#define INFO_RO_MAP_SIZE (INFO_MAX * 4) +#define INFO_RW_MAP_OFFSET INFO_RO_MAP_SIZE +#define INFO_RW_MAP_SIZE (INFO_MAX * 4) + int flash_info_read_enable(uint32_t offset, size_t size); +/* This in fact enables both read and write. */ int flash_info_write_enable(uint32_t offset, size_t size); void flash_info_write_disable(void); int flash_info_physical_write(int byte_offset, int num_bytes, const char *data); diff --git a/chip/g/system.c b/chip/g/system.c index 1400fa7060..c9dc325688 100644 --- a/chip/g/system.c +++ b/chip/g/system.c @@ -8,9 +8,9 @@ #include "cpu.h" #include "cpu.h" #include "flash.h" +#include "flash_info.h" #include "printf.h" #include "registers.h" -#include "signed_header.h" #include "system.h" #include "system_chip.h" #include "task.h" @@ -513,3 +513,111 @@ const char *system_get_build_info(void) return combined_build_info; } + +void system_update_rollback_mask(void) +{ +#ifndef CR50_DEV + int updated_words_count = 0; + int i; + int write_enabled = 0; + uint32_t header_mask = 0; + const struct SignedHeader *header_a; + const struct SignedHeader *header_b; + + header_a = (const struct SignedHeader *) + get_program_memory_addr(SYSTEM_IMAGE_RW); + header_b = (const struct SignedHeader *) + get_program_memory_addr(SYSTEM_IMAGE_RW_B); + + /* + * Make sure INFO1 RW map space is readable. + */ + if (flash_info_read_enable(INFO_RW_MAP_OFFSET, INFO_RW_MAP_SIZE) != + EC_SUCCESS) { + ccprintf("%s: failed to enable read access to info\n", + __func__); + return; + } + + /* + * The infomap field in the image header has a matching space in the + * flash INFO1 section. + * + * The INFO1 space words which map into zeroed bits in the infomap + * header are ignored by the RO. + * + * Let's make sure that those words in the INFO1 space are erased. + * This in turn makes sure that attempts to load earlier RW images + * (where those bits in the header are not zeroed) will fail, thus + * ensuring rollback protection. + */ + /* For each bit in the header infomap field of the running image. */ + for (i = 0; i < INFO_MAX; i++) { + uint32_t bit; + uint32_t word; + int byte_offset; + + /* Read the next infomap word when done with the current one. */ + if (!(i % 32)) { + /* + * Not to shoot ourselves in the foot, let's zero only + * those words in the INFO1 space which have both A + * and B header's infomap bit set to zero. + */ + header_mask = + header_a->infomap[i/32] | + header_b->infomap[i/32]; + } + + /* Get the next bit value. */ + bit = !!(header_mask & (1 << (i % 32))); + if (bit) { + /* + * By convention zeroed bits are expected to be + * adjacent at the LSB of the info mask field. Stop as + * soon as a non-zeroed bit is encountered. + */ + ccprintf("%s: bailing out at bit %d\n", __func__, i); + break; + } + + byte_offset = (INFO_MAX + i) * sizeof(uint32_t); + + if (flash_physical_info_read_word(byte_offset, &word) != + EC_SUCCESS) { + ccprintf("failed to read info mask word %d\n", i); + continue; + } + + if (!word) + continue; /* This word has been zeroed already. */ + + if (!write_enabled) { + if (flash_info_write_enable( + INFO_RW_MAP_OFFSET, + INFO_RW_MAP_SIZE) != EC_SUCCESS) { + ccprintf("%s: failed to enable write access to" + " info\n", __func__); + return; + } + write_enabled = 1; + } + + word = 0; + if (flash_info_physical_write(byte_offset, + sizeof(word), + (const char *) &word) != + EC_SUCCESS) { + ccprintf("failed to write info mask word %d\n", i); + continue; + } + updated_words_count++; + + } + if (!write_enabled) + return; + + flash_info_write_disable(); + ccprintf("updated %d info map words\n", updated_words_count); +#endif /* CR50_DEV ^^^^^^^^ NOT defined. */ +} diff --git a/chip/g/system_chip.h b/chip/g/system_chip.h index 0f21ba066b..4d6b3ec7c9 100644 --- a/chip/g/system_chip.h +++ b/chip/g/system_chip.h @@ -51,4 +51,10 @@ int system_rollback_detected(void); */ int system_battery_cutoff_support_required(void); +/** + * Modify info1 RW rollback mask to match currently executing RW image's + * header. + */ +void system_update_rollback_mask(void); + #endif /* __CROS_EC_SYSTEM_CHIP_H */