rollback: Prevent rollback region readback using MPU

We want to prevent easy readout of the rollback region, so we
protect it using the MPU. There is a short duration of time where
the region is unprotected (when we actually need to read the
information back), but we shorten it by disabling interrupts.

BRANCH=none
BUG=b:111330723
TEST=flashread 0xe0000, rw 0x80e0020, md 0x80e0020,
     ectool flashread 0xc0000 0x1000 x
     => All cause EC to crash and reboot
TEST=rollbackinfo still works

Change-Id: I85ee757b3e261de392af03bd958b36d140a1080a
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1143106
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Nicolas Norvez <norvez@chromium.org>

4 files changed, 54 insertions, 2 deletions

diff --git a/common/rollback.c b/common/rollback.c
index 374194abcf..cc5d27cb12 100644
--- a/common/rollback.c
+++ b/common/rollback.c
@@ -10,9 +10,13 @@
 #include "flash.h"
 #include "hooks.h"
 #include "host_command.h"
+#ifdef CONFIG_MPU
+#include "mpu.h"
+#endif
 #include "rollback.h"
 #include "sha256.h"
 #include "system.h"
+#include "task.h"
 #include "trng.h"
 #include "util.h"
 
@@ -45,16 +49,39 @@ static uintptr_t get_rollback_offset(int region)
 	return CONFIG_ROLLBACK_OFF + region * CONFIG_FLASH_ERASE_SIZE;
 }
 
+/*
+ * When MPU is available, read rollback with interrupts disabled, to minimize
+ * time protection is left open.
+ */
+static void lock_rollback(void)
+{
+#ifdef CONFIG_ROLLBACK_MPU_PROTECT
+	mpu_lock_rollback(1);
+	interrupt_enable();
+#endif
+}
+
+static void unlock_rollback(void)
+{
+#ifdef CONFIG_ROLLBACK_MPU_PROTECT
+	interrupt_disable();
+	mpu_lock_rollback(0);
+#endif
+}
+
 static int read_rollback(int region, struct rollback_data *data)
 {
 	uintptr_t offset;
+	int ret = EC_SUCCESS;
 
 	offset = get_rollback_offset(region);
 
+	unlock_rollback();
 	if (flash_read(offset, sizeof(*data), (char *)data))
-		return EC_ERROR_UNKNOWN;
+		ret = EC_ERROR_UNKNOWN;
+	lock_rollback();
 
-	return EC_SUCCESS;
+	return ret;
 }
 
 /*
diff --git a/core/cortex-m/include/mpu.h b/core/cortex-m/include/mpu.h
index 049f32f4f3..2dcf8be660 100644
--- a/core/cortex-m/include/mpu.h
+++ b/core/cortex-m/include/mpu.h
@@ -31,6 +31,8 @@ enum mpu_region {
 	REGION_CHIP_RESERVED = 7,	/* Reserved for use in chip/ */
 	/* only for chips with MPU supporting 16 regions */
 	REGION_UNCACHED_RAM = 8,        /* For uncached data RAM */
+	REGION_UNCACHED_RAM2 = 9,       /* Second region for unaligned size */
+	REGION_ROLLBACK = 10,           /* For rollback */
 };
 
 #define MPU_TYPE		REG32(0xe000ed90)
@@ -104,6 +106,11 @@ int mpu_lock_ro_flash(void);
 int mpu_lock_rw_flash(void);
 
 /**
+ * Protect/unprotect rollback region readback.
+ */
+int mpu_lock_rollback(int lock);
+
+/**
  * Initialize MPU.
  * It disables all regions if MPU is implemented. Otherwise, returns
  * EC_ERROR_UNIMPLEMENTED.
diff --git a/core/cortex-m/mpu.c b/core/cortex-m/mpu.c
index d99540287c..795f809eca 100644
--- a/core/cortex-m/mpu.c
+++ b/core/cortex-m/mpu.c
@@ -211,6 +211,16 @@ int mpu_lock_rw_flash(void)
 }
 #endif /* !CONFIG_EXTERNAL_STORAGE */
 
+#ifdef CONFIG_ROLLBACK_MPU_PROTECT
+int mpu_lock_rollback(int lock)
+{
+	return mpu_config_region(REGION_ROLLBACK,
+			CONFIG_MAPPED_STORAGE_BASE + CONFIG_ROLLBACK_OFF,
+			CONFIG_ROLLBACK_SIZE, MPU_ATTR_XN | MPU_ATTR_NO_NO,
+			lock);
+}
+#endif
+
 #ifdef CONFIG_CHIP_UNCACHED_REGION
 /* Store temporarily the regions ranges to use them for the MPU configuration */
 #define REGION(_name, _flag, _start, _size) \
@@ -237,6 +247,10 @@ int mpu_pre_init(void)
 	for (i = 0; i < MPU_TYPE_REG_COUNT(mpu_type); ++i)
 		mpu_config_region(i, CONFIG_RAM_BASE, CONFIG_RAM_SIZE, 0, 0);
 
+#ifdef CONFIG_ROLLBACK_MPU_PROTECT
+	mpu_lock_rollback(1);
+#endif
+
 #ifdef CONFIG_ARMV7M_CACHE
 #ifdef CONFIG_CHIP_UNCACHED_REGION
 	mpu_config_region(REGION_UNCACHED_RAM,
diff --git a/include/config.h b/include/config.h
index 78eef35dfe..a44a14fc8c 100644
--- a/include/config.h
+++ b/include/config.h
@@ -1513,6 +1513,10 @@
 /* If defined, add support for storing some entropy in the rollback region. */
 #undef CONFIG_ROLLBACK_SECRET_SIZE
 
+/* If defined, protect rollback region readback using MPU. */
+#undef CONFIG_ROLLBACK_MPU_PROTECT
+
+
 /*
  * If defined, inject some locally generated entropy when secret is updated,
  * using board_get_entropy function.