From dc4a5b6490bfa40a97bb09bb12139d1ecd0fa801 Mon Sep 17 00:00:00 2001 From: Chris Frantz Date: Tue, 22 Jul 2025 10:13:58 -0700 Subject: [PATCH] [otbn] Patch an error in the OTBN boot services program There is an error in the OTBN boot services program loaded by the ROM that can affect the random share values. This bug is caused by a simple transposition of operands in the `p256_base` source code. Since the OTBN program is stored in ROM and loaded into OTBN by the ROM, we patch it in the immutable section before we use that portion of the program. DO NOT cherrypick this change to the `master` branch. Signed-off-by: Chris Frantz --- sw/device/silicon_creator/lib/drivers/otbn.c | 17 +++++++++++++++++ sw/device/silicon_creator/lib/drivers/otbn.h | 12 ++++++++++++ .../silicon_creator/rom_ext/imm_section/BUILD | 1 + .../rom_ext/imm_section/imm_section.c | 8 ++++++++ 4 files changed, 38 insertions(+) diff --git a/sw/device/silicon_creator/lib/drivers/otbn.c b/sw/device/silicon_creator/lib/drivers/otbn.c index a359038d79af6..20d5099537397 100644 --- a/sw/device/silicon_creator/lib/drivers/otbn.c +++ b/sw/device/silicon_creator/lib/drivers/otbn.c @@ -264,3 +264,20 @@ rom_error_t sc_otbn_load_app(const sc_otbn_app_t app) { } return kErrorOk; } + +void sc_otbn_patch(void) { + enum { + // Offset in the code of the bad instruction: + kBugOffset = 0x958, + // The bad instruction is: + // bn.rshi w16, w20, w31 >> 192 + kBadInstr = 0xc1fa387b, + // The good instruction is: + // bn.rshi w16, w31, w20 >> 192 + kGoodInstr = 0xc14fb87b, + }; + uint32_t inst = abs_mmio_read32(kBase + OTBN_IMEM_REG_OFFSET + kBugOffset); + if (inst == kBadInstr) { + abs_mmio_write32(kBase + OTBN_IMEM_REG_OFFSET + kBugOffset, kGoodInstr); + } +} diff --git a/sw/device/silicon_creator/lib/drivers/otbn.h b/sw/device/silicon_creator/lib/drivers/otbn.h index 24d6aab7a1c17..98cbe66d7c058 100644 --- a/sw/device/silicon_creator/lib/drivers/otbn.h +++ b/sw/device/silicon_creator/lib/drivers/otbn.h @@ -322,6 +322,18 @@ rom_error_t sc_otbn_imem_sec_wipe(void); OT_WARN_UNUSED_RESULT rom_error_t sc_otbn_dmem_sec_wipe(void); +/** + * Patch a bad instruction in OTBN imem. + * + * There is a bug in the OTBN boot services program loaded by ROM. There is a + * single instruction with mistakenly transposed operands which can affect the + * random share values. This bug DOES NOT affect the correctness of the OTBN + * calculations. + * + * This function patches the bad instruction with the correctly instruction. + */ +void sc_otbn_patch(void); + #ifdef __cplusplus } #endif diff --git a/sw/device/silicon_creator/rom_ext/imm_section/BUILD b/sw/device/silicon_creator/rom_ext/imm_section/BUILD index d02953bf11f5c..e5855cdb9994b 100644 --- a/sw/device/silicon_creator/rom_ext/imm_section/BUILD +++ b/sw/device/silicon_creator/rom_ext/imm_section/BUILD @@ -48,6 +48,7 @@ cc_library( "//sw/device/silicon_creator/lib/base:static_dice", "//sw/device/silicon_creator/lib/cert:dice_chain", "//sw/device/silicon_creator/lib/drivers:flash_ctrl", + "//sw/device/silicon_creator/lib/drivers:otbn", "//sw/device/silicon_creator/lib/drivers:rnd", "//sw/device/silicon_creator/lib/ownership:ownership_key", "//sw/device/silicon_creator/rom_ext:rom_ext_manifest", diff --git a/sw/device/silicon_creator/rom_ext/imm_section/imm_section.c b/sw/device/silicon_creator/rom_ext/imm_section/imm_section.c index 786e9406ba8da..752b2a605b39c 100644 --- a/sw/device/silicon_creator/rom_ext/imm_section/imm_section.c +++ b/sw/device/silicon_creator/rom_ext/imm_section/imm_section.c @@ -11,6 +11,7 @@ #include "sw/device/silicon_creator/lib/base/sec_mmio.h" #include "sw/device/silicon_creator/lib/cert/dice_chain.h" #include "sw/device/silicon_creator/lib/drivers/flash_ctrl.h" +#include "sw/device/silicon_creator/lib/drivers/otbn.h" #include "sw/device/silicon_creator/lib/drivers/rnd.h" #include "sw/device/silicon_creator/lib/epmp_state.h" #include "sw/device/silicon_creator/lib/error.h" @@ -21,6 +22,13 @@ OT_WARN_UNUSED_RESULT static rom_error_t imm_section_start(void) { + // Patch a bug in the OTBN ECDSA-P256 program. + // + // There is a single instruction with mistakenly transposed operands which + // can affect the random share values. This bug DOES NOT affect the + // correctness of the OTBN calculations. + sc_otbn_patch(); + // Check the ePMP state. HARDENED_RETURN_IF_ERROR(epmp_state_check()); // Check sec_mmio expectations.