Skip to content

Commit

Permalink
[rom_ext] Lock the correct ROM_EXT region
Browse files Browse the repository at this point in the history
Ownership flash lockdown was protecting and locking all regions in the
same slot that booted the owner code.  However, the ROM_EXT and owner
code don't have to boot from the same side of the flash.

1. Protect the ROM_EXT and owner regions each according to their boot slot.
2. Test each combination of ROM_EXT slot and owner slot in the
   `flash_permission_test`.

Fixes #25435.

Signed-off-by: Chris Frantz <[email protected]>
  • Loading branch information
cfrantz committed Jan 18, 2025
1 parent 1290004 commit 2e18d36
Show file tree
Hide file tree
Showing 10 changed files with 233 additions and 64 deletions.
1 change: 1 addition & 0 deletions sw/device/silicon_creator/lib/ownership/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ cc_library(
":ownership_key",
"//sw/device/lib/base:hardened_memory",
"//sw/device/silicon_creator/lib:boot_data",
"//sw/device/silicon_creator/lib:boot_log",
"//sw/device/silicon_creator/lib:dbg_print",
"//sw/device/silicon_creator/lib/drivers:flash_ctrl",
"//sw/device/silicon_creator/lib/drivers:lifecycle",
Expand Down
94 changes: 74 additions & 20 deletions sw/device/silicon_creator/lib/ownership/owner_block.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ owner_page_status_t owner_page_valid[2];
enum {
kFlashBankSize = FLASH_CTRL_PARAM_REG_PAGES_PER_BANK,
kFlashPageSize = FLASH_CTRL_PARAM_BYTES_PER_PAGE,

kRomExtAStart = 0 / kFlashPageSize,
kRomExtAEnd = CHIP_ROM_EXT_SIZE_MAX / kFlashPageSize,
kRomExtBStart = kFlashBankSize + kRomExtAStart,
kRomExtBEnd = kFlashBankSize + kRomExtAEnd,
};

hardened_bool_t owner_block_newversion_mode(void) {
Expand Down Expand Up @@ -150,27 +155,48 @@ rom_error_t owner_block_parse(const owner_block_t *block,
return kErrorOk;
}

// These functions aren't defined in owner_block.h because they aren't meant
// to be public APIs. They aren't static so we can access the symbols in the
// unit test program.

// Checks if the half-open range [start..end) overlaps with the ROM_EXT region.
// The RomExt_Start/End constants are also expressed as half-open ranges.
hardened_bool_t rom_ext_flash_overlap(uint32_t start, uint32_t end) {
return (start < kRomExtAEnd && end > kRomExtAStart) ||
(start < kRomExtBEnd && end > kRomExtBStart)
? kHardenedBoolTrue
: kHardenedBoolFalse;
}

// Checks if the half-open range [start..end) is exclusively within the ROM_EXT
// region. The RomExt_Start/End constants are also expressed as half-open
// ranges.
hardened_bool_t rom_ext_flash_exclusive(uint32_t start, uint32_t end) {
return (kRomExtAStart <= start && start < kRomExtAEnd &&
kRomExtAStart < end && end <= kRomExtAEnd) ||
(kRomExtBStart <= start && start < kRomExtBEnd &&
kRomExtBStart < end && end <= kRomExtBEnd)
? kHardenedBoolTrue
: kHardenedBoolFalse;
}

rom_error_t owner_block_flash_check(const owner_flash_config_t *flash) {
size_t len = (flash->header.length - sizeof(owner_flash_config_t)) /
sizeof(owner_flash_region_t);
if (len >= 8) {
return kErrorOwnershipFlashConfigLenth;
}

const uint32_t kRomExtAStart = 0 / kFlashPageSize;
const uint32_t kRomExtAEnd = CHIP_ROM_EXT_SIZE_MAX / kFlashPageSize;
const uint32_t kRomExtBStart = kFlashBankSize + kRomExtAStart;
const uint32_t kRomExtBEnd = kFlashBankSize + kRomExtAEnd;

const owner_flash_region_t *config = flash->config;
uint32_t crypt = 0;
for (size_t i = 0; i < len; ++i, ++config, crypt += 0x11111111) {
uint32_t start = config->start;
uint32_t end = start + config->size;
if ((kRomExtAStart >= start && kRomExtAStart < end) ||
(kRomExtAEnd > start && kRomExtAEnd <= end) ||
(kRomExtBStart >= start && kRomExtBStart < end) ||
(kRomExtBEnd > start && kRomExtBEnd <= end)) {
// When checking the flash configuration, a region is a ROM_EXT region if
// it overlaps the ROM_EXT bounds. This detects if the defined ROM_EXT
// region has the same ecc/scrambling parameters as the default flash
// configuration.
if (rom_ext_flash_overlap(start, end) == kHardenedBoolTrue) {
uint32_t val = config->properties ^ crypt;
flash_ctrl_cfg_t cfg = {
.scrambling = bitfield_field32_read(val, FLASH_CONFIG_SCRAMBLE),
Expand All @@ -195,7 +221,9 @@ rom_error_t owner_block_flash_check(const owner_flash_config_t *flash) {
}

rom_error_t owner_block_flash_apply(const owner_flash_config_t *flash,
uint32_t config_side, uint32_t lockdown) {
uint32_t config_side,
uint32_t creator_lockdown,
uint32_t owner_lockdown) {
if ((hardened_bool_t)flash == kHardenedBoolFalse)
return kErrorOk;
// TODO: Hardening: lockdown should be one of kBootSlotA, kBootSlotB or
Expand Down Expand Up @@ -229,19 +257,45 @@ rom_error_t owner_block_flash_apply(const owner_flash_config_t *flash,
.erase = bitfield_field32_read(val, FLASH_CONFIG_ERASE),
};

if (lockdown == config_side) {
if (bitfield_field32_read(val, FLASH_CONFIG_PROTECT_WHEN_PRIMARY) !=
kMultiBitBool4False) {
uint32_t pwp =
bitfield_field32_read(val, FLASH_CONFIG_PROTECT_WHEN_PRIMARY);
hardened_bool_t lock =
bitfield_field32_read(val, FLASH_CONFIG_LOCK) != kMultiBitBool4False
? kHardenedBoolTrue
: kHardenedBoolFalse;
uint32_t region_start = config->start;
uint32_t region_end = region_start + config->size;

// When applying the flash configuration, a region is a ROM_EXT region if
// it is exclusively within the ROM_EXT bounds. This sets the creator
// the lockdown policy for a region that is exclusively the creator
// region.
if (rom_ext_flash_exclusive(region_start, region_end) ==
kHardenedBoolTrue) {
// Flash region is a ROM_EXT region.
// If the config_side is the same as the creator lockdown side, and
// protect_when_primary is requested, deny write/erase to the region.
if (config_side == creator_lockdown && pwp != kMultiBitBool4False) {
perm.write = kMultiBitBool4False;
perm.erase = kMultiBitBool4False;
}
}

hardened_bool_t lock = kHardenedBoolFalse;
if (lockdown != kHardenedBoolFalse) {
if (bitfield_field32_read(val, FLASH_CONFIG_LOCK) !=
kMultiBitBool4False) {
lock = kHardenedBoolTrue;
// If we aren't in a lockdown state, then do not lock the region
// configuration via the flash_ctrl regwen bits.
if (creator_lockdown == kHardenedBoolFalse) {
lock = kHardenedBoolFalse;
}
} else {
// Flash region is an owner region.
// If the config_side is the same as the owner lockdown side, and
// protect_when_primary is requested, deny write/erase to the region.
if (config_side == owner_lockdown && pwp != kMultiBitBool4False) {
perm.write = kMultiBitBool4False;
perm.erase = kMultiBitBool4False;
}
// If we aren't in a lockdown state, then do not lock the region
// configuration via the flash_ctrl regwen bits.
if (owner_lockdown == kHardenedBoolFalse) {
lock = kHardenedBoolFalse;
}
}
flash_ctrl_data_region_protect(i, config->start, config->size, perm, cfg,
Expand Down
13 changes: 9 additions & 4 deletions sw/device/silicon_creator/lib/ownership/owner_block.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,18 @@ rom_error_t owner_block_flash_check(const owner_flash_config_t *flash);
*
* @param flash A pointer to a flash configuration struct.
* @param config_side Which side of the flash to configure.
* @param lockdown Apply any special lockdown configuration to the specified
* side of the flash. May use kHardenedBoolFalse to skip
* lockdown.
* @param creator_lockdown Apply any special lockdown configuration to
* silicon_creator regions on the specified side of the
* flash. May use kHardenedBoolFalse to skip lockdown.
* @param owner_lockdown Apply any special lockdown configuration to
* silicon_owner regions on the specified side of the
* flash. May use kHardenedBoolFalse to skip lockdown.
* @return error code.
*/
rom_error_t owner_block_flash_apply(const owner_flash_config_t *flash,
uint32_t config_side, uint32_t lockdown);
uint32_t config_side,
uint32_t creator_lockdown,
uint32_t owner_lockdown);

/**
* Apply the flash info configuration parameters from the owner block.
Expand Down
78 changes: 67 additions & 11 deletions sw/device/silicon_creator/lib/ownership/owner_block_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,9 @@ const owner_flash_info_config_t info_config = {
};

TEST_F(OwnerBlockTest, FlashConfigApplyBad) {
rom_error_t error = owner_block_flash_apply(&bad_flash_config, kBootSlotA, 0);
rom_error_t error = owner_block_flash_apply(&bad_flash_config, kBootSlotA,
/*creator_lockdown=*/0,
/*owner_lockdown=*/0);
EXPECT_EQ(error, kErrorOwnershipFlashConfigLenth);
}

Expand Down Expand Up @@ -269,15 +271,16 @@ TEST_F(OwnerBlockTest, FlashConfigApplySideA) {
kMultiBitBool4True),
kHardenedBoolFalse));

rom_error_t error =
owner_block_flash_apply(&simple_flash_config, kBootSlotA, 0);
rom_error_t error = owner_block_flash_apply(&simple_flash_config, kBootSlotA,
/*creator_lockdown=*/0,
/*owner_lockdown=*/0);
EXPECT_EQ(error, kErrorOk);
}

// Tests that the flash parameters get applied for side A and the
// ProtectWhenPrimary disables erase and program on the ROM_EXT and FIRMWARE
// ProtectWhenActive disables erase and program on the ROM_EXT and FIRMWARE
// regions.
TEST_F(OwnerBlockTest, FlashConfigApplySideAPrimary) {
TEST_F(OwnerBlockTest, FlashConfigApplySideA_Active) {
EXPECT_CALL(
flash_ctrl_,
DataRegionProtect(0, 0, 32,
Expand All @@ -303,13 +306,16 @@ TEST_F(OwnerBlockTest, FlashConfigApplySideAPrimary) {
kMultiBitBool4True),
kHardenedBoolFalse));

rom_error_t error =
owner_block_flash_apply(&simple_flash_config, kBootSlotA, kBootSlotA);
rom_error_t error = owner_block_flash_apply(&simple_flash_config, kBootSlotA,
/*creator_lockdown=*/kBootSlotA,
/*owner_lockdown=*/kBootSlotA);
EXPECT_EQ(error, kErrorOk);
}

// Tests that the flash parameters get applied for side B.
TEST_F(OwnerBlockTest, FlashConfigApplySideB) {
// Tests that the flash parameters get applied for side B when B is not the
// active slot. Check that ProtectWhenActive does not change the write/erase
// permissions for slot B.
TEST_F(OwnerBlockTest, FlashConfigApplySideB_NotActive) {
EXPECT_CALL(
flash_ctrl_,
DataRegionProtect(3, 256 + 0, 32,
Expand All @@ -335,8 +341,9 @@ TEST_F(OwnerBlockTest, FlashConfigApplySideB) {
kMultiBitBool4True),
kHardenedBoolFalse));

rom_error_t error =
owner_block_flash_apply(&simple_flash_config, kBootSlotB, 0);
rom_error_t error = owner_block_flash_apply(&simple_flash_config, kBootSlotB,
/*creator_lockdown=*/kBootSlotA,
/*owner_lockdown=*/kBootSlotA);
EXPECT_EQ(error, kErrorOk);
}

Expand Down Expand Up @@ -723,4 +730,53 @@ INSTANTIATE_TEST_SUITE_P(AllCases, RomExtFlashConfigTest,
&invalid_flash_2, &invalid_flash_3,
&invalid_flash_4));

struct FlashRegion {
uint32_t start;
uint32_t end;
bool overlap;
bool exclusive;
};

class RomExtFlashBoundsTest : public OwnerBlockTest,
public testing::WithParamInterface<FlashRegion> {
};

extern "C" {
// These functions aren't defined in owner_block.h because they aren't meant
// to be public APIs. They aren't static so we can access the symbols here
// for testing.
hardened_bool_t rom_ext_flash_overlap(uint32_t, uint32_t);
hardened_bool_t rom_ext_flash_exclusive(uint32_t, uint32_t);
}

// Test the flash bounds checking functions.
TEST_P(RomExtFlashBoundsTest, FlashBoundsTest) {
FlashRegion p = GetParam();
hardened_bool_t overlap = rom_ext_flash_overlap(p.start, p.end);
hardened_bool_t exclusive = rom_ext_flash_exclusive(p.start, p.end);
EXPECT_EQ(overlap, p.overlap ? kHardenedBoolTrue : kHardenedBoolFalse);
EXPECT_EQ(exclusive, p.exclusive ? kHardenedBoolTrue : kHardenedBoolFalse);
}

// clang-format off
INSTANTIATE_TEST_SUITE_P(AllCases, RomExtFlashBoundsTest,
testing::Values(
// The ROM_EXT A region is blocks [0x00 .. 0x20).
// The ROM_EXT B region is blocks [0x100 .. 0x120).
// The ranges expressed are half-open: [start .. end).
FlashRegion{0x000, 0x020, /*overlap=*/true, /*exclusive=*/true}, // Full ROM_EXT A region.
FlashRegion{0x100, 0x120, /*overlap=*/true, /*exclusive=*/true}, // Full ROM_EXT B region.
FlashRegion{0x005, 0x010, /*overlap=*/true, /*exclusive=*/true}, // Partial ROM_EXT A region.
FlashRegion{0x105, 0x110, /*overlap=*/true, /*exclusive=*/true}, // Partial ROM_EXT B region.
FlashRegion{0x000, 0x040, /*overlap=*/true, /*exclusive=*/false}, // Overlapped ROM_EXT A region.
FlashRegion{0x100, 0x140, /*overlap=*/true, /*exclusive=*/false}, // Overlapped ROM_EXT B region.
FlashRegion{0x020, 0x040, /*overlap=*/false, /*exclusive=*/false}, // Not ROM_EXT A region.
FlashRegion{0x120, 0x140, /*overlap=*/false, /*exclusive=*/false}, // Not ROM_EXT B region.
FlashRegion{0x005, 0x105, /*overlap=*/true, /*exclusive=*/false}, // Mid RX_A to mid RX_B.
FlashRegion{0x020, 0x100, /*overlap=*/false, /*exclusive=*/false}, // Full not ROM_EXT side A.
FlashRegion{0x020, 0x101, /*overlap=*/true, /*exclusive=*/false}, // Side A intrudes in to RX_B.
FlashRegion{0x000, 0x200, /*overlap=*/true, /*exclusive=*/false} // Full flash region.
));
// clang-format on

} // namespace
28 changes: 18 additions & 10 deletions sw/device/silicon_creator/lib/ownership/ownership.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "sw/device/lib/base/macros.h"
#include "sw/device/lib/base/memory.h"
#include "sw/device/silicon_creator/lib/boot_data.h"
#include "sw/device/silicon_creator/lib/boot_log.h"
#include "sw/device/silicon_creator/lib/dbg_print.h"
#include "sw/device/silicon_creator/lib/drivers/flash_ctrl.h"
#include "sw/device/silicon_creator/lib/drivers/hmac.h"
Expand Down Expand Up @@ -125,10 +126,12 @@ static rom_error_t locked_owner_init(boot_data_t *bootdata,
HARDENED_RETURN_IF_ERROR(owner_block_parse(&owner_page[0], config, keyring));
HARDENED_RETURN_IF_ERROR(
owner_block_flash_apply(config->flash, kBootSlotA,
/*lockdown=*/kHardenedBoolFalse));
/*creator_lockdown=*/kHardenedBoolFalse,
/*owner_lockdown=*/kHardenedBoolFalse));
HARDENED_RETURN_IF_ERROR(
owner_block_flash_apply(config->flash, kBootSlotB,
/*lockdown=*/kHardenedBoolFalse));
/*creator_lockdown=*/kHardenedBoolFalse,
/*owner_lockdown=*/kHardenedBoolFalse));
HARDENED_RETURN_IF_ERROR(owner_block_info_apply(config->info));
return kErrorOk;
}
Expand All @@ -153,7 +156,8 @@ static rom_error_t unlocked_init(boot_data_t *bootdata, owner_config_t *config,
owner_block_parse(&owner_page[0], config, keyring));
HARDENED_RETURN_IF_ERROR(
owner_block_flash_apply(config->flash, bootdata->primary_bl0_slot,
/*lockdown=*/kHardenedBoolFalse));
/*creator_lockdown=*/kHardenedBoolFalse,
/*owner_lockdown=*/kHardenedBoolFalse));
}

if (owner_block_page1_valid_for_transfer(bootdata) == kHardenedBoolTrue) {
Expand All @@ -171,7 +175,8 @@ static rom_error_t unlocked_init(boot_data_t *bootdata, owner_config_t *config,
}
HARDENED_RETURN_IF_ERROR(
owner_block_flash_apply(config->flash, secondary,
/*lockdown=*/kHardenedBoolFalse));
/*creator_lockdown=*/kHardenedBoolFalse,
/*owner_lockdown=*/kHardenedBoolFalse));
HARDENED_RETURN_IF_ERROR(owner_block_info_apply(config->info));
return kErrorOk;
}
Expand Down Expand Up @@ -271,14 +276,17 @@ rom_error_t ownership_init(boot_data_t *bootdata, owner_config_t *config,
return error;
}

rom_error_t ownership_flash_lockdown(boot_data_t *bootdata,
uint32_t active_slot,
rom_error_t ownership_flash_lockdown(boot_data_t *bootdata, boot_log_t *bootlog,
const owner_config_t *config) {
if (bootdata->ownership_state == kOwnershipStateLockedOwner) {
HARDENED_RETURN_IF_ERROR(owner_block_flash_apply(config->flash, kBootSlotA,
/*lockdown=*/active_slot));
HARDENED_RETURN_IF_ERROR(owner_block_flash_apply(config->flash, kBootSlotB,
/*lockdown=*/active_slot));
HARDENED_RETURN_IF_ERROR(
owner_block_flash_apply(config->flash, kBootSlotA,
/*creator_lockdown=*/bootlog->rom_ext_slot,
/*owner_lockdown=*/bootlog->bl0_slot));
HARDENED_RETURN_IF_ERROR(
owner_block_flash_apply(config->flash, kBootSlotB,
/*creator_lockdown=*/bootlog->rom_ext_slot,
/*owner_lockdown=*/bootlog->bl0_slot));
} else {
HARDENED_CHECK_NE(bootdata->ownership_state, kOwnershipStateLockedOwner);
}
Expand Down
4 changes: 2 additions & 2 deletions sw/device/silicon_creator/lib/ownership/ownership.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "sw/device/lib/base/hardened.h"
#include "sw/device/silicon_creator/lib/boot_data.h"
#include "sw/device/silicon_creator/lib/boot_log.h"
#include "sw/device/silicon_creator/lib/error.h"
#include "sw/device/silicon_creator/lib/ownership/datatypes.h"
#include "sw/device/silicon_creator/lib/ownership/owner_block.h"
Expand All @@ -25,8 +26,7 @@ rom_error_t ownership_init(boot_data_t *bootdata, owner_config_t *config,
* @param config The current owner configuration.
* @return error state.
*/
rom_error_t ownership_flash_lockdown(boot_data_t *bootdata,
uint32_t active_slot,
rom_error_t ownership_flash_lockdown(boot_data_t *bootdata, boot_log_t *bootlog,
const owner_config_t *config);

/**
Expand Down
Loading

0 comments on commit 2e18d36

Please sign in to comment.