Skip to content

Commit

Permalink
[rtl] Fix FI vulnerability in RF
Browse files Browse the repository at this point in the history
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 <nasahlpa@lowrisc.org>
  • Loading branch information
nasahlpa committed Dec 22, 2023
1 parent 38da151 commit 457d502
Show file tree
Hide file tree
Showing 7 changed files with 354 additions and 25 deletions.
27 changes: 27 additions & 0 deletions dv/uvm/core_ibex/common/prim/prim_and2.sv
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions dv/uvm/core_ibex/ibex_dv.f
Original file line number Diff line number Diff line change
Expand Up @@ -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_generic_and2.sv

// Shared lowRISC code
${LOWRISC_IP_DIR}/ip/prim/rtl/prim_cipher_pkg.sv
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions ibex_top.core
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
106 changes: 101 additions & 5 deletions rtl/ibex_register_file_ff.sv
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
);

Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
113 changes: 106 additions & 7 deletions rtl/ibex_register_file_fpga.sv
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
);

Expand All @@ -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;
Expand All @@ -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
Expand Down
Loading

0 comments on commit 457d502

Please sign in to comment.