Skip to content

Commit dc4a5b6

Browse files
committed
[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 <cfrantz@google.com>
1 parent 573efe9 commit dc4a5b6

File tree

4 files changed

+38
-0
lines changed

4 files changed

+38
-0
lines changed

sw/device/silicon_creator/lib/drivers/otbn.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,3 +264,20 @@ rom_error_t sc_otbn_load_app(const sc_otbn_app_t app) {
264264
}
265265
return kErrorOk;
266266
}
267+
268+
void sc_otbn_patch(void) {
269+
enum {
270+
// Offset in the code of the bad instruction:
271+
kBugOffset = 0x958,
272+
// The bad instruction is:
273+
// bn.rshi w16, w20, w31 >> 192
274+
kBadInstr = 0xc1fa387b,
275+
// The good instruction is:
276+
// bn.rshi w16, w31, w20 >> 192
277+
kGoodInstr = 0xc14fb87b,
278+
};
279+
uint32_t inst = abs_mmio_read32(kBase + OTBN_IMEM_REG_OFFSET + kBugOffset);
280+
if (inst == kBadInstr) {
281+
abs_mmio_write32(kBase + OTBN_IMEM_REG_OFFSET + kBugOffset, kGoodInstr);
282+
}
283+
}

sw/device/silicon_creator/lib/drivers/otbn.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,18 @@ rom_error_t sc_otbn_imem_sec_wipe(void);
322322
OT_WARN_UNUSED_RESULT
323323
rom_error_t sc_otbn_dmem_sec_wipe(void);
324324

325+
/**
326+
* Patch a bad instruction in OTBN imem.
327+
*
328+
* There is a bug in the OTBN boot services program loaded by ROM. There is a
329+
* single instruction with mistakenly transposed operands which can affect the
330+
* random share values. This bug DOES NOT affect the correctness of the OTBN
331+
* calculations.
332+
*
333+
* This function patches the bad instruction with the correctly instruction.
334+
*/
335+
void sc_otbn_patch(void);
336+
325337
#ifdef __cplusplus
326338
}
327339
#endif

sw/device/silicon_creator/rom_ext/imm_section/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ cc_library(
4848
"//sw/device/silicon_creator/lib/base:static_dice",
4949
"//sw/device/silicon_creator/lib/cert:dice_chain",
5050
"//sw/device/silicon_creator/lib/drivers:flash_ctrl",
51+
"//sw/device/silicon_creator/lib/drivers:otbn",
5152
"//sw/device/silicon_creator/lib/drivers:rnd",
5253
"//sw/device/silicon_creator/lib/ownership:ownership_key",
5354
"//sw/device/silicon_creator/rom_ext:rom_ext_manifest",

sw/device/silicon_creator/rom_ext/imm_section/imm_section.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "sw/device/silicon_creator/lib/base/sec_mmio.h"
1212
#include "sw/device/silicon_creator/lib/cert/dice_chain.h"
1313
#include "sw/device/silicon_creator/lib/drivers/flash_ctrl.h"
14+
#include "sw/device/silicon_creator/lib/drivers/otbn.h"
1415
#include "sw/device/silicon_creator/lib/drivers/rnd.h"
1516
#include "sw/device/silicon_creator/lib/epmp_state.h"
1617
#include "sw/device/silicon_creator/lib/error.h"
@@ -21,6 +22,13 @@
2122

2223
OT_WARN_UNUSED_RESULT
2324
static rom_error_t imm_section_start(void) {
25+
// Patch a bug in the OTBN ECDSA-P256 program.
26+
//
27+
// There is a single instruction with mistakenly transposed operands which
28+
// can affect the random share values. This bug DOES NOT affect the
29+
// correctness of the OTBN calculations.
30+
sc_otbn_patch();
31+
2432
// Check the ePMP state.
2533
HARDENED_RETURN_IF_ERROR(epmp_state_check());
2634
// Check sec_mmio expectations.

0 commit comments

Comments
 (0)