[rtl] Fix FI vulnerability in RF

As described in #20715, a single fault-induced bit-flip inside the register file could change which of the register file value is provided to Ibex. This PR fixes this issue by (i) encoding raddr_a/b to one-hot encoded signals, (ii) checking these signals for faults, and (iii) using an one-hot encoded MUX to select which register file value is forwarded to rdata_a/b. Area increases by ~1% (Yosys + Nangate45 synthesis). I conducted a formal fault injection verification at the Yosys netlist to ensure that the issue really is fixed. Signed-off-by: Pascal Nasahl <[email protected]>
lowRISC · Dec 22, 2023 · 6582167 · 6582167
1 parent 38da151
commit 6582167
Show file tree

Hide file tree

Showing 7 changed files with 354 additions and 25 deletions.
diff --git a/dv/uvm/core_ibex/common/prim/prim_and2.sv b/dv/uvm/core_ibex/common/prim/prim_and2.sv
@@ -0,0 +1,27 @@
+// Copyright lowRISC contributors.
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+
+// Abstract primitives wrapper.
+//
+// This file is a stop-gap until the DV file list is generated by FuseSoC.
+// Its contents are taken from the file which would be generated by FuseSoC.
+// https://github.com/lowRISC/ibex/issues/893
+
+module prim_and2 #(
+  parameter int Width = 1
+) (
+  input        [Width-1:0] in0_i,
+  input        [Width-1:0] in1_i,
+  output logic [Width-1:0] out_o
+);
+
+if (1) begin : gen_generic
+  prim_generic_and2 #(
+    .Width(Width)
+  ) u_impl_generic (
+    .*
+  );
+end
+
+endmodule
diff --git a/dv/uvm/core_ibex/ibex_dv.f b/dv/uvm/core_ibex/ibex_dv.f
@@ -45,6 +45,8 @@
 ${PRJ_DIR}/dv/uvm/core_ibex/common/prim/prim_clock_mux2.sv
 ${LOWRISC_IP_DIR}/ip/prim_generic/rtl/prim_generic_flop.sv
 ${PRJ_DIR}/dv/uvm/core_ibex/common/prim/prim_flop.sv
+${LOWRISC_IP_DIR}/ip/prim_generic/rtl/prim_generic_and2.sv
+${PRJ_DIR}/dv/uvm/core_ibex/common/prim/prim_and2.sv
 
 // Shared lowRISC code
 ${LOWRISC_IP_DIR}/ip/prim/rtl/prim_cipher_pkg.sv
@@ -64,6 +66,9 @@
 ${LOWRISC_IP_DIR}/ip/prim/rtl/prim_secded_72_64_enc.sv
 ${LOWRISC_IP_DIR}/ip/prim/rtl/prim_secded_72_64_dec.sv
 ${LOWRISC_IP_DIR}/ip/prim/rtl/prim_onehot_check.sv
+${LOWRISC_IP_DIR}/ip/prim/rtl/prim_onehot_check.sv
+${LOWRISC_IP_DIR}/ip/prim/rtl/prim_onehot_enc.sv
+${LOWRISC_IP_DIR}/ip/prim/rtl/prim_onehot_mux.sv
 ${LOWRISC_IP_DIR}/ip/prim/rtl/prim_mubi_pkg.sv
 
 // ibex CORE RTL files

diff --git a/ibex_top.core b/ibex_top.core
@@ -10,11 +10,13 @@ filesets:
     depend:
       - lowrisc:ibex:ibex_pkg
       - lowrisc:ibex:ibex_core
+      - lowrisc:prim:and2
       - lowrisc:prim:buf
       - lowrisc:prim:clock_mux2
       - lowrisc:prim:flop
       - lowrisc:prim:ram_1p_scr
       - lowrisc:prim:onehot_check
+      - lowrisc:prim:onehot
     files:
       - rtl/ibex_register_file_ff.sv # generic FF-based
       - rtl/ibex_register_file_fpga.sv # FPGA

diff --git a/rtl/ibex_register_file_ff.sv b/rtl/ibex_register_file_ff.sv
@@ -15,6 +15,7 @@ module ibex_register_file_ff #(
   parameter int unsigned          DataWidth         = 32,
   parameter bit                   DummyInstructions = 0,
   parameter bit                   WrenCheck         = 0,
+  parameter bit                   RdataMuxCheck     = 0,
   parameter logic [DataWidth-1:0] WordZeroVal       = '0
 ) (
   // Clock and Reset
@@ -39,7 +40,7 @@ module ibex_register_file_ff #(
   input  logic [DataWidth-1:0] wdata_a_i,
   input  logic                 we_a_i,
 
-  // This indicates whether spurious WE are detected.
+  // This indicates whether spurious WE or one-hot encoded raddr are detected.
   output logic                 err_o
 );
 
@@ -49,6 +50,8 @@ module ibex_register_file_ff #(
   logic [DataWidth-1:0] rf_reg   [NUM_WORDS];
   logic [NUM_WORDS-1:0] we_a_dec;
 
+  logic oh_raddr_a_err, oh_raddr_b_err, oh_we_err;
+
   always_comb begin : we_a_decoder
     for (int unsigned i = 0; i < NUM_WORDS; i++) begin
       we_a_dec[i] = (waddr_a_i == 5'(i)) ? we_a_i : 1'b0;
@@ -78,12 +81,12 @@ module ibex_register_file_ff #(
       .oh_i(we_a_dec_buf),
       .addr_i(waddr_a_i),
       .en_i(we_a_i),
-      .err_o
+      .err_o(oh_we_err)
     );
   end else begin : gen_no_wren_check
     logic unused_strobe;
     assign unused_strobe = we_a_dec[0]; // this is never read from in this case
-    assign err_o = 1'b0;
+    assign oh_we_err = 1'b0;
   end
 
   // No flops for R0 as it's hard-wired to 0
@@ -130,8 +133,101 @@ module ibex_register_file_ff #(
     assign rf_reg[0] = WordZeroVal;
   end
 
-  assign rdata_a_o = rf_reg[raddr_a_i];
-  assign rdata_b_o = rf_reg[raddr_b_i];
+  if (RdataMuxCheck) begin : gen_rdata_mux_check
+    // Encode raddr_a/b into one-hot encoded signals.
+    logic [NUM_WORDS-1:0] raddr_onehot_a, raddr_onehot_b;
+    logic [NUM_WORDS-1:0] raddr_onehot_a_buf, raddr_onehot_b_buf;
+    prim_onehot_enc #(
+      .OneHotWidth(NUM_WORDS)
+    ) u_prim_onehot_enc_raddr_a (
+      .in_i  (raddr_a_i),
+      .en_i  (1'b1),
+      .out_o (raddr_onehot_a)
+    );
+
+    prim_onehot_enc #(
+      .OneHotWidth(NUM_WORDS)
+    ) u_prim_onehot_enc_raddr_b (
+      .in_i  (raddr_b_i),
+      .en_i  (1'b1),
+      .out_o (raddr_onehot_b)
+    );
+
+    // Buffer the one-hot encoded signals so that the checkers
+    // are not optimized.
+    prim_buf #(
+      .Width(NUM_WORDS)
+    ) u_prim_buf_raddr_a (
+      .in_i (raddr_onehot_a),
+      .out_o(raddr_onehot_a_buf)
+    );
+
+    prim_buf #(
+      .Width(NUM_WORDS)
+    ) u_prim_buf_raddr_b (
+      .in_i (raddr_onehot_b),
+      .out_o(raddr_onehot_b_buf)
+    );
+
+    // Check the one-hot encoded signals for glitches.
+    prim_onehot_check #(
+      .AddrWidth(ADDR_WIDTH),
+      .OneHotWidth(NUM_WORDS),
+      .AddrCheck(1),
+      .EnableCheck(0)
+    ) u_prim_onehot_check_raddr_a (
+      .clk_i,
+      .rst_ni,
+      .oh_i   (raddr_onehot_a_buf),
+      .addr_i (raddr_a_i),
+      .en_i   (1'b0),
+      .err_o  (oh_raddr_a_err)
+    );
+
+    prim_onehot_check #(
+      .AddrWidth(ADDR_WIDTH),
+      .OneHotWidth(NUM_WORDS),
+      .AddrCheck(1),
+      .EnableCheck(0)
+    ) u_prim_onehot_check_raddr_b (
+      .clk_i,
+      .rst_ni,
+      .oh_i   (raddr_onehot_b_buf),
+      .addr_i (raddr_b_i),
+      .en_i   (1'b0),
+      .err_o  (oh_raddr_b_err)
+    );
+
+    // MUX register to rdata_a/b_o according to raddr_a/b_onehot.
+    prim_onehot_mux  #(
+      .Width(DataWidth),
+      .Inputs(NUM_WORDS)
+    ) u_rdata_a_mux (
+      .clk_i,
+      .rst_ni,
+      .in_i  (rf_reg),
+      .sel_i (raddr_onehot_a),
+      .out_o (rdata_a_o)
+    );
+
+    prim_onehot_mux  #(
+      .Width(DataWidth),
+      .Inputs(NUM_WORDS)
+    ) u_rdata_b_mux (
+      .clk_i,
+      .rst_ni,
+      .in_i  (rf_reg),
+      .sel_i (raddr_onehot_b),
+      .out_o (rdata_b_o)
+    );
+  end else begin : gen_no_rdata_mux_check
+    assign rdata_a_o = rf_reg[raddr_a_i];
+    assign rdata_b_o = rf_reg[raddr_b_i];
+    assign oh_raddr_a_err = 1'b0;
+    assign oh_raddr_b_err = 1'b0;
+  end
+
+  assign err_o = oh_raddr_a_err || oh_raddr_b_err || oh_we_err;
 
   // Signal not used in FF register file
   logic unused_test_en;

diff --git a/rtl/ibex_register_file_fpga.sv b/rtl/ibex_register_file_fpga.sv
@@ -16,6 +16,7 @@ module ibex_register_file_fpga #(
     parameter int unsigned          DataWidth         = 32,
     parameter bit                   DummyInstructions = 0,
     parameter bit                   WrenCheck         = 0,
+    parameter bit                   RdataMuxCheck     = 0,
     parameter logic [DataWidth-1:0] WordZeroVal       = '0
 ) (
   // Clock and Reset
@@ -37,7 +38,7 @@ module ibex_register_file_fpga #(
   input  logic [DataWidth-1:0] wdata_a_i,
   input  logic                 we_a_i,
 
-  // This indicates whether spurious WE are detected.
+  // This indicates whether spurious WE or one-hot encoded raddr are detected.
   output logic                 err_o
 );
 
@@ -47,11 +48,109 @@ module ibex_register_file_fpga #(
   logic [DataWidth-1:0] mem[NUM_WORDS];
   logic we; // write enable if writing to any register other than R0
 
-  // async_read a
-  assign rdata_a_o = (raddr_a_i == '0) ? '0 : mem[raddr_a_i];
+  logic [DataWidth-1:0] mem_o_a, mem_o_b;
 
-  // async_read b
-  assign rdata_b_o = (raddr_b_i == '0) ? '0 : mem[raddr_b_i];
+  // WE strobe and one-hot encoded raddr alert.
+  logic oh_raddr_a_err, oh_raddr_b_err, oh_we_err;
+  assign err_o = oh_raddr_a_err || oh_raddr_b_err || oh_we_err;
+
+  if (RdataMuxCheck) begin : gen_rdata_mux_check
+    // Encode raddr_a/b into one-hot encoded signals.
+    logic [NUM_WORDS-1:0] raddr_onehot_a, raddr_onehot_b;
+    logic [NUM_WORDS-1:0] raddr_onehot_a_buf, raddr_onehot_b_buf;
+    prim_onehot_enc #(
+      .OneHotWidth(NUM_WORDS)
+    ) u_prim_onehot_enc_raddr_a (
+      .in_i  (raddr_a_i),
+      .en_i  (1'b1),
+      .out_o (raddr_onehot_a)
+    );
+
+    prim_onehot_enc #(
+      .OneHotWidth(NUM_WORDS)
+    ) u_prim_onehot_enc_raddr_b (
+      .in_i  (raddr_b_i),
+      .en_i  (1'b1),
+      .out_o (raddr_onehot_b)
+    );
+
+    // Buffer the one-hot encoded signals so that the checkers
+    // are not optimized.
+    prim_buf #(
+      .Width(NUM_WORDS)
+    ) u_prim_buf_raddr_a (
+      .in_i (raddr_onehot_a),
+      .out_o(raddr_onehot_a_buf)
+    );
+
+    prim_buf #(
+      .Width(NUM_WORDS)
+    ) u_prim_buf_raddr_b (
+      .in_i (raddr_onehot_b),
+      .out_o(raddr_onehot_b_buf)
+    );
+
+    // Check the one-hot encoded signals for glitches.
+    prim_onehot_check #(
+      .AddrWidth(ADDR_WIDTH),
+      .OneHotWidth(NUM_WORDS),
+      .AddrCheck(1),
+      .EnableCheck(0)
+    ) u_prim_onehot_check_raddr_a (
+      .clk_i,
+      .rst_ni,
+      .oh_i   (raddr_onehot_a_buf),
+      .addr_i (raddr_a_i),
+      .en_i   (1'b0),
+      .err_o  (oh_raddr_a_err)
+    );
+
+    prim_onehot_check #(
+      .AddrWidth(ADDR_WIDTH),
+      .OneHotWidth(NUM_WORDS),
+      .AddrCheck(1),
+      .EnableCheck(0)
+    ) u_prim_onehot_check_raddr_b (
+      .clk_i,
+      .rst_ni,
+      .oh_i   (raddr_onehot_b_buf),
+      .addr_i (raddr_b_i),
+      .en_i   (1'b0),
+      .err_o  (oh_raddr_b_err)
+    );
+
+    // MUX register to rdata_a/b_o according to raddr_a/b_onehot.
+    prim_onehot_mux  #(
+      .Width(DataWidth),
+      .Inputs(NUM_WORDS)
+    ) u_rdata_a_mux (
+      .clk_i,
+      .rst_ni,
+      .in_i  (mem),
+      .sel_i (raddr_onehot_a),
+      .out_o (mem_o_a)
+    );
+
+    prim_onehot_mux  #(
+      .Width(DataWidth),
+      .Inputs(NUM_WORDS)
+    ) u_rdata_b_mux (
+      .clk_i,
+      .rst_ni,
+      .in_i  (mem),
+      .sel_i (raddr_onehot_b),
+      .out_o (mem_o_b)
+    );
+
+    assign rdata_a_o = (raddr_a_i == '0) ? '0 : mem_o_a;
+    assign rdata_b_o = (raddr_b_i == '0) ? '0 : mem_o_b;
+  end else begin : gen_no_rdata_mux_check
+    // async_read a
+    assign rdata_a_o = (raddr_a_i == '0) ? '0 : mem[raddr_a_i];
+
+    // async_read b
+    assign rdata_b_o = (raddr_b_i == '0) ? '0 : mem[raddr_b_i];
+  end
 
   // we select
   assign we = (waddr_a_i == '0) ? 1'b0 : we_a_i;
@@ -60,9 +159,9 @@ module ibex_register_file_fpga #(
   // This checks for spurious WE strobes on the regfile.
   if (WrenCheck) begin : gen_wren_check
     // Since the FPGA uses a memory macro, there is only one write-enable strobe to check.
-    assign err_o = we && !we_a_i;
+    assign oh_we_err = we && !we_a_i;
   end else begin : gen_no_wren_check
-    assign err_o = 1'b0;
+    assign oh_we_err = 1'b0;
   end
 
   // Note that the SystemVerilog LRM requires variables on the LHS of assignments within