From a04a310913100b6c04eb25746b49f5cd56aa9e31 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Mon, 17 Apr 2017 15:14:30 +0800 Subject: rwsig/update_fw: Prevent race in rollback protection There is a window where the rollback information in RW could potentially be updated during RW signature verification. We make sure this cannot happen by: - Preventing update over USB while RWSIG is running - When system is locked, only update rollback information if RW region is locked: this guarantees that RW cannot be modified from boot until RW is validated, and then until rollback information is updated. Also, remove rollback_lock() in rwsig_check_signature: rwsig_jump_now() protects all flash, which also protects rollback. This reduces the number of required reboots on rollback update. BRANCH=none BUG=b:35586219 BUG=b:35587171 TEST=Add long delay in rwsig_check_signature, make sure EC cannot be updated while verification is in progress. Change-Id: I7a51fad8a64b7e258b3a7e15d75b3dab64ce1c94 Reviewed-on: https://chromium-review.googlesource.com/479176 Commit-Ready: Nicolas Boichat Tested-by: Nicolas Boichat Reviewed-by: Randall Spangler --- common/rwsig.c | 13 ++++++------- common/update_fw.c | 8 ++++++++ include/update_fw.h | 1 + 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/common/rwsig.c b/common/rwsig.c index a4ba19e937..600c3eeaad 100644 --- a/common/rwsig.c +++ b/common/rwsig.c @@ -185,8 +185,13 @@ int rwsig_check_signature(void) /* * Signature verified: we know that rw_rollback_version is valid, check * if rollback information should be updated. + * + * When system is locked, we only increment the rollback if RW is + * currently protected. */ - if (rw_rollback_version != min_rollback_version) { + if (rw_rollback_version != min_rollback_version && + ((!system_is_locked() || + flash_get_protect() & EC_FLASH_PROTECT_RW_NOW))) { /* * This will fail if the rollback block is protected (RW image * will unprotect that block later on). @@ -201,12 +206,6 @@ int rwsig_check_signature(void) good = 0; } } - - /* - * Lock the ROLLBACK region, this will cause the board to reboot if the - * region is not already protected. - */ - rollback_lock(); #endif out: CPRINTS("RW verify %s", good ? "OK" : "FAILED"); diff --git a/common/update_fw.c b/common/update_fw.c index 7a51e6977f..c08e8322b5 100644 --- a/common/update_fw.c +++ b/common/update_fw.c @@ -145,6 +145,14 @@ void fw_update_start(struct first_response_pdu *rpdu) vb21_key = (const struct vb21_packed_key *)CONFIG_RO_PUBKEY_ADDR; rpdu->common.key_version = htobe32(vb21_key->key_version); #endif + +#ifdef HAS_TASK_RWSIG + /* Do not allow the update to start if RWSIG is still running. */ + if (rwsig_get_status() == RWSIG_IN_PROGRESS) { + CPRINTF("RWSIG in progress\n"); + rpdu->return_value = htobe32(UPDATE_RWSIG_BUSY); + } +#endif } void fw_update_command_handler(void *body, diff --git a/include/update_fw.h b/include/update_fw.h index e575500b19..5788779cb2 100644 --- a/include/update_fw.h +++ b/include/update_fw.h @@ -185,6 +185,7 @@ enum { UPDATE_MALLOC_ERROR = 7, UPDATE_ROLLBACK_ERROR = 8, UPDATE_RATE_LIMIT_ERROR = 9, + UPDATE_RWSIG_BUSY = 10, }; #endif /* ! __CROS_EC_UPDATE_FW_H */ -- cgit v1.2.1