[sw/silicon_creator] Move length checks from manifest to boot_policy

Minimum and maximum allowed lengths for ROM_EXT and first owner boot
stage images will be different. This change adds individual constants
for both stages and moves related checks from manifest.h to mask ROM and
ROM_EXT boot policy implementations.

Signed-off-by: Alphan Ulusoy <alphan@google.com>

sw/device/silicon_creator/lib/error.h

@@ -78,9 +78,8 @@ enum module_ {
   X(kErrorSigverifyBadOtpValue,       ERROR_(4, kModuleSigverify, kInternal)), \
   X(kErrorSigverifyBadLcState,        ERROR_(5, kModuleSigverify, kInternal)), \
   X(kErrorKeymgrInternal,             ERROR_(1, kModuleKeymgr, kInternal)), \
-  X(kErrorManifestBadLength,          ERROR_(1, kModuleManifest, kInternal)), \
-  X(kErrorManifestBadEntryPoint,      ERROR_(2, kModuleManifest, kInternal)), \
-  X(kErrorManifestBadCodeRegion,      ERROR_(3, kModuleManifest, kInternal)), \
+  X(kErrorManifestBadEntryPoint,      ERROR_(1, kModuleManifest, kInternal)), \
+  X(kErrorManifestBadCodeRegion,      ERROR_(2, kModuleManifest, kInternal)), \
   X(kErrorAlertBadIndex,              ERROR_(1, kModuleAlertHandler, kInvalidArgument)), \
   X(kErrorAlertBadClass,              ERROR_(2, kModuleAlertHandler, kInvalidArgument)), \
   X(kErrorAlertBadEnable,             ERROR_(3, kModuleAlertHandler, kInvalidArgument)), \
@@ -104,7 +103,8 @@ enum module_ {
   X(kErrorSecMmioWriteCountFault,     ERROR_(5, kModuleSecMmio, kInternal)), \
   X(kErrorSecMmioCheckCountFault,     ERROR_(6, kModuleSecMmio, kInternal)), \
   X(kErrorBootPolicyBadIdentifier,    ERROR_(1, kModuleBootPolicy, kInternal)), \
-  X(kErrorBootPolicyRollback,         ERROR_(2, kModuleBootPolicy, kInternal)), \
+  X(kErrorBootPolicyBadLength,        ERROR_(2, kModuleBootPolicy, kInternal)), \
+  X(kErrorBootPolicyRollback,         ERROR_(3, kModuleBootPolicy, kInternal)), \
   X(kErrorRetSramLocked,              ERROR_(1, kModuleRetSram, kInternal)), \
   X(kErrorBootstrapSpiDevice,         ERROR_(1, KModuleBootstrap, kInternal)), \
   X(kErrorBootstrapErase,             ERROR_(2, KModuleBootstrap, kInternal)), \

sw/device/silicon_creator/lib/manifest.c

@@ -4,6 +4,23 @@
 
 #include "sw/device/silicon_creator/lib/manifest.h"
 
+#include "hw/top_earlgrey/sw/autogen/top_earlgrey.h"
+
+static_assert(MANIFEST_LENGTH_FIELD_ROM_EXT_MIN >= MANIFEST_SIZE,
+              "`MANIFEST_LENGTH_FIELD_ROM_EXT_MIN` is too small");
+static_assert(MANIFEST_LENGTH_FIELD_ROM_EXT_MAX >=
+                  MANIFEST_LENGTH_FIELD_ROM_EXT_MIN,
+              "`MANIFEST_LENGTH_FIELD_ROM_EXT_MAX` is too small");
+static_assert(MANIFEST_LENGTH_FIELD_OWNER_STAGE_MIN >= MANIFEST_SIZE,
+              "`MANIFEST_LENGTH_FIELD_OWNER_STAGE_MIN` is too small");
+static_assert(MANIFEST_LENGTH_FIELD_OWNER_STAGE_MAX >=
+                  MANIFEST_LENGTH_FIELD_OWNER_STAGE_MIN,
+              "`MANIFEST_LENGTH_FIELD_OWNER_STAGE_MAX` is too small");
+static_assert(MANIFEST_LENGTH_FIELD_OWNER_STAGE_MAX <=
+                  ((TOP_EARLGREY_EFLASH_SIZE_BYTES / 2) -
+                   MANIFEST_LENGTH_FIELD_ROM_EXT_MAX),
+              "`MANIFEST_LENGTH_FIELD_OWNER_STAGE_MAX` is too large");
+
 // Extern declarations for the inline functions in the manifest header.
 extern rom_error_t manifest_check(const manifest_t *manifest);
 extern manifest_digest_region_t manifest_digest_region_get(

sw/device/silicon_creator/lib/manifest.h

@@ -243,18 +243,33 @@ typedef struct manifest_digest_region {
 } manifest_digest_region_t;
 
 /**
- * Allowed bounds for the `length` field.
+ * ROM_EXT manifest identifier (ASCII "OTRE").
+ */
+#define MANIFEST_IDENTIFIER_ROM_EXT 0x4552544f
+
+/**
+ * First owner boot stage manifest identifier (ASCII "OTSO").
+ */
+#define MANIFEST_IDENTIFIER_OWNER_STAGE 0x4f53544f
+
+/**
+ * Allowed bounds for the `length` field of a ROM_EXT manifest.
  */
 // FIXME: Update min value after we have a fairly representative ROM_EXT.
-#define MANIFEST_LENGTH_FIELD_MIN MANIFEST_SIZE
-#define MANIFEST_LENGTH_FIELD_MAX 65536
+#define MANIFEST_LENGTH_FIELD_ROM_EXT_MIN MANIFEST_SIZE
+#define MANIFEST_LENGTH_FIELD_ROM_EXT_MAX 0x10000
+
+/**
+ * Allowed bounds for the `length` field of a first owner boot stage manifest.
+ */
+#define MANIFEST_LENGTH_FIELD_OWNER_STAGE_MIN MANIFEST_SIZE
+#define MANIFEST_LENGTH_FIELD_OWNER_STAGE_MAX 0x70000
 
 #ifndef OT_OFF_TARGET_TEST
 /**
  * Checks the fields of a manifest.
  *
  * This function performs several basic checks to ensure that
- * - Length of the image is within a reasonable range,
  * - Executable region is non-empty, inside the image, located after the
  * manifest, and word aligned, and
  * - Entry point is inside the executable region and word aligned.
@@ -263,12 +278,6 @@ typedef struct manifest_digest_region {
  * @return Result of the operation.
  */
 inline rom_error_t manifest_check(const manifest_t *manifest) {
-  // Length of the image must be within bounds.
-  if (manifest->length < MANIFEST_LENGTH_FIELD_MIN ||
-      manifest->length > MANIFEST_LENGTH_FIELD_MAX) {
-    return kErrorManifestBadLength;
-  }
-
   // Executable region must be empty, inside the image, located after the
   // manifest, and word aligned.
   if (manifest->code_start >= manifest->code_end ||

sw/device/silicon_creator/lib/manifest_unittest.cc

@@ -51,14 +51,6 @@ TEST_F(ManifestTest, EntryPointGet) {
             reinterpret_cast<uintptr_t>(&manifest_) + manifest_.entry_point);
 }
 
-TEST_F(ManifestTest, BadLength) {
-  manifest_.length = MANIFEST_LENGTH_FIELD_MAX + 1;
-  EXPECT_EQ(manifest_check(&manifest_), kErrorManifestBadLength);
-
-  manifest_.length = MANIFEST_LENGTH_FIELD_MIN - 1;
-  EXPECT_EQ(manifest_check(&manifest_), kErrorManifestBadLength);
-}
-
 TEST_F(ManifestTest, CodeRegionEmpty) {
   manifest_.code_start = manifest_.code_end;
   EXPECT_EQ(manifest_check(&manifest_), kErrorManifestBadCodeRegion);

sw/device/silicon_creator/mask_rom/boot_policy.c

@@ -21,10 +21,14 @@ boot_policy_manifests_t boot_policy_manifests_get(void) {
 }
 
 rom_error_t boot_policy_manifest_check(const manifest_t *manifest) {
-  RETURN_IF_ERROR(manifest_check(manifest));
-  if (manifest->identifier != kBootPolicyRomExtIdentifier) {
+  if (manifest->identifier != MANIFEST_IDENTIFIER_ROM_EXT) {
     return kErrorBootPolicyBadIdentifier;
   }
+  if (manifest->length < MANIFEST_LENGTH_FIELD_ROM_EXT_MIN ||
+      manifest->length > MANIFEST_LENGTH_FIELD_ROM_EXT_MAX) {
+    return kErrorBootPolicyBadLength;
+  }
+  RETURN_IF_ERROR(manifest_check(manifest));
   // TODO(#7879): Implement anti-rollback.
   uint32_t min_security_version = 0;
   if (manifest->security_version < min_security_version) {

sw/device/silicon_creator/mask_rom/boot_policy.h

@@ -12,13 +12,6 @@
 extern "C" {
 #endif  // __cplusplus
 
-enum {
-  /**
-   * ROM_EXT manifest identifier (ASCII "OTRE").
-   */
-  kBootPolicyRomExtIdentifier = 0x4552544f,
-};
-
 /**
  * Type alias for the ROM_EXT entry point.
  *

sw/device/silicon_creator/mask_rom/boot_policy_unittest.cc

@@ -21,29 +21,37 @@ class BootPolicyTest : public mask_rom_test::MaskRomTest {
 
 TEST_F(BootPolicyTest, ManifestCheck) {
   manifest_t manifest{};
-  manifest.identifier = kBootPolicyRomExtIdentifier;
+  manifest.identifier = MANIFEST_IDENTIFIER_ROM_EXT;
 
+  manifest.length = MANIFEST_LENGTH_FIELD_ROM_EXT_MIN;
   EXPECT_CALL(mock_manifest_, Check(&manifest)).WillOnce(Return(kErrorOk));
+  EXPECT_EQ(boot_policy_manifest_check(&manifest), kErrorOk);
 
+  manifest.length = MANIFEST_LENGTH_FIELD_ROM_EXT_MAX >> 1;
+  EXPECT_CALL(mock_manifest_, Check(&manifest)).WillOnce(Return(kErrorOk));
+  EXPECT_EQ(boot_policy_manifest_check(&manifest), kErrorOk);
+
+  manifest.length = MANIFEST_LENGTH_FIELD_ROM_EXT_MAX;
+  EXPECT_CALL(mock_manifest_, Check(&manifest)).WillOnce(Return(kErrorOk));
   EXPECT_EQ(boot_policy_manifest_check(&manifest), kErrorOk);
 }
 
-TEST_F(BootPolicyTest, ManifestCheckOutOfBounds) {
+TEST_F(BootPolicyTest, ManifestCheckBadIdentifier) {
   manifest_t manifest{};
 
-  EXPECT_CALL(mock_manifest_, Check(&manifest))
-      .WillOnce(Return(kErrorManifestBadLength));
-
-  EXPECT_EQ(boot_policy_manifest_check(&manifest), kErrorManifestBadLength);
+  EXPECT_EQ(boot_policy_manifest_check(&manifest),
+            kErrorBootPolicyBadIdentifier);
 }
 
-TEST_F(BootPolicyTest, ManifestCheckBadIdentifier) {
+TEST_F(BootPolicyTest, ManifestCheckBadLength) {
   manifest_t manifest{};
+  manifest.identifier = MANIFEST_IDENTIFIER_ROM_EXT;
 
-  EXPECT_CALL(mock_manifest_, Check(&manifest)).WillOnce(Return(kErrorOk));
+  manifest.length = MANIFEST_LENGTH_FIELD_ROM_EXT_MIN - 1;
+  EXPECT_EQ(boot_policy_manifest_check(&manifest), kErrorBootPolicyBadLength);
 
-  EXPECT_EQ(boot_policy_manifest_check(&manifest),
-            kErrorBootPolicyBadIdentifier);
+  manifest.length = MANIFEST_LENGTH_FIELD_ROM_EXT_MAX + 1;
+  EXPECT_EQ(boot_policy_manifest_check(&manifest), kErrorBootPolicyBadLength);
 }
 
 struct ManifestOrderTestCase {

sw/device/silicon_creator/rom_exts/rom_ext_boot_policy.c

@@ -21,10 +21,14 @@ rom_ext_boot_policy_manifests_t rom_ext_boot_policy_manifests_get(void) {
 }
 
 rom_error_t rom_ext_boot_policy_manifest_check(const manifest_t *manifest) {
-  RETURN_IF_ERROR(manifest_check(manifest));
-  if (manifest->identifier != kRomExtBootPolicyOwnerStageIdentifier) {
+  if (manifest->identifier != MANIFEST_IDENTIFIER_OWNER_STAGE) {
     return kErrorBootPolicyBadIdentifier;
   }
+  if (manifest->length < MANIFEST_LENGTH_FIELD_OWNER_STAGE_MIN ||
+      manifest->length > MANIFEST_LENGTH_FIELD_OWNER_STAGE_MAX) {
+    return kErrorBootPolicyBadLength;
+  }
+  RETURN_IF_ERROR(manifest_check(manifest));
   // TODO(#7879): Implement anti-rollback.
   uint32_t min_security_version = 0;
   if (manifest->security_version < min_security_version) {

sw/device/silicon_creator/rom_exts/rom_ext_boot_policy.h

@@ -12,13 +12,6 @@
 extern "C" {
 #endif  // __cplusplus
 
-enum {
-  /**
-   * First owner boot stage manifest identifier (ASCII "OTSO").
-   */
-  kRomExtBootPolicyOwnerStageIdentifier = 0x4f53544f,
-};
-
 /**
  * Type alias for the first owner boot stage entry point.
  *

sw/device/silicon_creator/rom_exts/rom_ext_boot_policy_ptrs.h

@@ -27,7 +27,7 @@ static_assert((TOP_EARLGREY_EFLASH_SIZE_BYTES % 2) == 0,
 // TODO(#7893): Should these be volatile?
 inline const manifest_t *rom_ext_boot_policy_manifest_a_get(void) {
   return (const manifest_t *)(TOP_EARLGREY_EFLASH_BASE_ADDR +
-                              MANIFEST_LENGTH_FIELD_MAX);
+                              MANIFEST_LENGTH_FIELD_ROM_EXT_MAX);
 }
 
 /**
@@ -40,7 +40,7 @@ inline const manifest_t *rom_ext_boot_policy_manifest_a_get(void) {
 inline const manifest_t *rom_ext_boot_policy_manifest_b_get(void) {
   return (const manifest_t *)(TOP_EARLGREY_EFLASH_BASE_ADDR +
                               (TOP_EARLGREY_EFLASH_SIZE_BYTES / 2) +
-                              MANIFEST_LENGTH_FIELD_MAX);
+                              MANIFEST_LENGTH_FIELD_ROM_EXT_MAX);
 }
 #else
 /**

sw/device/silicon_creator/rom_exts/rom_ext_boot_policy_unittest.cc

@@ -21,30 +21,39 @@ class RomExtBootPolicyTest : public mask_rom_test::MaskRomTest {
 
 TEST_F(RomExtBootPolicyTest, ManifestCheck) {
   manifest_t manifest{};
-  manifest.identifier = kRomExtBootPolicyOwnerStageIdentifier;
+  manifest.identifier = MANIFEST_IDENTIFIER_OWNER_STAGE;
 
+  manifest.length = MANIFEST_LENGTH_FIELD_OWNER_STAGE_MIN;
   EXPECT_CALL(mock_manifest_, Check(&manifest)).WillOnce(Return(kErrorOk));
+  EXPECT_EQ(rom_ext_boot_policy_manifest_check(&manifest), kErrorOk);
+
+  manifest.length = MANIFEST_LENGTH_FIELD_OWNER_STAGE_MAX >> 1;
+  EXPECT_CALL(mock_manifest_, Check(&manifest)).WillOnce(Return(kErrorOk));
+  EXPECT_EQ(rom_ext_boot_policy_manifest_check(&manifest), kErrorOk);
 
+  manifest.length = MANIFEST_LENGTH_FIELD_OWNER_STAGE_MAX;
+  EXPECT_CALL(mock_manifest_, Check(&manifest)).WillOnce(Return(kErrorOk));
   EXPECT_EQ(rom_ext_boot_policy_manifest_check(&manifest), kErrorOk);
 }
 
-TEST_F(RomExtBootPolicyTest, ManifestCheckOutOfBounds) {
+TEST_F(RomExtBootPolicyTest, ManifestCheckBadIdentifier) {
   manifest_t manifest{};
 
-  EXPECT_CALL(mock_manifest_, Check(&manifest))
-      .WillOnce(Return(kErrorManifestBadLength));
-
   EXPECT_EQ(rom_ext_boot_policy_manifest_check(&manifest),
-            kErrorManifestBadLength);
+            kErrorBootPolicyBadIdentifier);
 }
 
-TEST_F(RomExtBootPolicyTest, ManifestCheckBadIdentifier) {
+TEST_F(RomExtBootPolicyTest, ManifestCheckBadLength) {
   manifest_t manifest{};
+  manifest.identifier = MANIFEST_IDENTIFIER_OWNER_STAGE;
 
-  EXPECT_CALL(mock_manifest_, Check(&manifest)).WillOnce(Return(kErrorOk));
+  manifest.length = MANIFEST_LENGTH_FIELD_OWNER_STAGE_MIN - 1;
+  EXPECT_EQ(rom_ext_boot_policy_manifest_check(&manifest),
+            kErrorBootPolicyBadLength);
 
+  manifest.length = MANIFEST_LENGTH_FIELD_OWNER_STAGE_MAX + 1;
   EXPECT_EQ(rom_ext_boot_policy_manifest_check(&manifest),
-            kErrorBootPolicyBadIdentifier);
+            kErrorBootPolicyBadLength);
 }
 
 struct ManifestOrderTestCase {