cortex-m mpu: illegal shift fixstabilize-15245.B-main

Shifting left by 32 is undefined behavior. An AND operation with
mask of 0xffffffff is meaningless, so just avoid it.

BUG=b:64477774
BRANCH=none
TEST=none

Signed-off-by: Boris Mittelberg <bmbm@google.com>
Change-Id: Ibcb3359f453345caee01936c074a9c0ae5aff7dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4021135
Tested-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Code-Coverage: Zoss <zoss-cl-coverage@prod.google.com>

3 files changed, 38 insertions, 2 deletions

diff --git a/core/cortex-m/include/mpu_private.h b/core/cortex-m/include/mpu_private.h
index eca474e14d..eb9de4c395 100644
--- a/core/cortex-m/include/mpu_private.h
+++ b/core/cortex-m/include/mpu_private.h
@@ -21,5 +21,6 @@ int mpu_update_region(uint8_t region, uint32_t addr, uint8_t size_bit,
 int mpu_config_region(uint8_t region, uint32_t addr, uint32_t size,
 		      uint16_t attr, uint8_t enable);
 struct mpu_rw_regions mpu_get_rw_regions(void);
+uint32_t align_down_to_bits(uint32_t addr, uint8_t addr_bits);
 
 #endif /* __CROS_EC_MPU_PRIVATE_H */
diff --git a/core/cortex-m/mpu.c b/core/cortex-m/mpu.c
index c0793180dc..db03936dfa 100644
--- a/core/cortex-m/mpu.c
+++ b/core/cortex-m/mpu.c
@@ -95,6 +95,17 @@ int mpu_update_region(uint8_t region, uint32_t addr, uint8_t size_bit,
 }
 
 /*
+ * Align address to a maximum of 31 bits
+ */
+uint32_t align_down_to_bits(uint32_t addr, uint8_t addr_bits)
+{
+	if (addr_bits < 32)
+		return addr & ~((1u << addr_bits) - 1);
+	else
+		return addr;
+}
+
+/*
  * Greedily configure the largest possible part of the given region from the
  * base address.
  *
@@ -140,7 +151,7 @@ static int mpu_config_region_greedy(uint8_t region, uint32_t addr,
 		 * disabling if it is not completely contained in the requested
 		 * range.
 		 */
-		subregion_base = addr & ~((1 << natural_alignment) - 1);
+		subregion_base = align_down_to_bits(addr, natural_alignment);
 		subregion_size = 1 << (natural_alignment - 3);
 		*consumed = 0;
 		for (sr_idx = 0; sr_idx < 8; sr_idx++) {
@@ -159,7 +170,8 @@ static int mpu_config_region_greedy(uint8_t region, uint32_t addr,
 		*consumed = 1 << natural_alignment;
 	}
 
-	return mpu_update_region(region, addr & ~((1 << natural_alignment) - 1),
+	return mpu_update_region(region,
+				 align_down_to_bits(addr, natural_alignment),
 				 natural_alignment, attr, enable,
 				 subregion_disable);
 }
diff --git a/test/mpu.c b/test/mpu.c
index 2193c0c617..957111b901 100644
--- a/test/mpu.c
+++ b/test/mpu.c
@@ -3,6 +3,14 @@
  * found in the LICENSE file.
  */
 
+/* This test file meant to be executed on a real device. Example:
+ * 1. make tests BOARD=bloonchipper
+ * 2. servod --board=bloonchipper
+ * 3. flash_ec --board bloonchipper --image build/bloonchipper/test-mpu.bin
+ * 4. Open console via dut-control raw_fpmcu_console_uart_pty
+ * 5. runtest on console
+ */
+
 #include <stdbool.h>
 #include "mpu.h"
 #include "mpu_private.h"
@@ -172,6 +180,19 @@ test_static int test_mpu_get_rw_regions(void)
 	return EC_SUCCESS;
 }
 
+test_static int test_align_down_to_bits(void)
+{
+	uint32_t addr = 0x87654321;
+
+	TEST_EQ(align_down_to_bits(addr, 0), addr, "%d");
+	TEST_EQ(align_down_to_bits(addr, 1), 0x87654320, "%d");
+	TEST_EQ(align_down_to_bits(addr, 30), 0x80000000, "%d");
+	TEST_EQ(align_down_to_bits(addr, 31), 0x80000000, "%d");
+	TEST_EQ(align_down_to_bits(addr, 32), addr, "%d");
+	TEST_EQ(align_down_to_bits(addr, 33), addr, "%d");
+	return EC_SUCCESS;
+}
+
 void run_test(int argc, const char **argv)
 {
 	enum ec_image cur_image = system_get_image_copy();
@@ -211,6 +232,8 @@ void run_test(int argc, const char **argv)
 	RUN_TEST(reset_mpu);
 	RUN_TEST(test_mpu_get_rw_regions);
 	RUN_TEST(reset_mpu);
+	RUN_TEST(test_align_down_to_bits);
+	RUN_TEST(reset_mpu);
 	/* This test must be last because it generates a panic */
 	RUN_TEST(test_mpu_update_region_valid_region);
 	RUN_TEST(reset_mpu);