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 <[email protected]>
  • Loading branch information
@nasahlpa
nasahlpa committed Dec 22, 2023
1 parent 38da151 commit 16c514e
Show file tree
Hide file tree
Showing 4 changed files with 322 additions and 25 deletions.
107 changes: 102 additions & 5 deletions rtl/ibex_register_file_ff.sv
View file
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,11 @@ module ibex_register_file_ff #(
logic [DataWidth-1:0] rf_reg [NUM_WORDS];
logic [NUM_WORDS-1:0] we_a_dec;

logic [NUM_WORDS-1:0] raddr_onehot_a, raddr_onehot_b;
logic [NUM_WORDS-1:0] raddr_onehot_a_buf, raddr_onehot_b_buf;

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 +84,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 +136,99 @@ 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.
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)
);

Check warning on line 213 in rtl/ibex_register_file_ff.sv

View workflow job for this annotation

GitHub Actions / verible-lint 

[verible-verilog-lint] reported by reviewdog 🐶
Remove trailing spaces. [Style: trailing-spaces] [no-trailing-spaces]

Raw Output:
message:"Remove trailing spaces. [Style: trailing-spaces] [no-trailing-spaces]"  location:{path:"./rtl/ibex_register_file_ff.sv"  range:{start:{line:213  column:1}}}  severity:WARNING  source:{name:"verible-verilog-lint"  url:"https://github.com/chipsalliance/verible"}
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
View file
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 [NUM_WORDS-1:0] raddr_onehot_a, raddr_onehot_b;
logic [NUM_WORDS-1:0] raddr_onehot_a_buf, raddr_onehot_b_buf;
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.
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 (mem_o_a)
);

Check warning on line 133 in rtl/ibex_register_file_fpga.sv

View workflow job for this annotation

GitHub Actions / verible-lint 

[verible-verilog-lint] reported by reviewdog 🐶
Remove trailing spaces. [Style: trailing-spaces] [no-trailing-spaces]

Raw Output:
message:"Remove trailing spaces. [Style: trailing-spaces] [no-trailing-spaces]"  location:{path:"./rtl/ibex_register_file_fpga.sv"  range:{start:{line:133  column:1}}}  severity:WARNING  source:{name:"verible-verilog-lint"  url:"https://github.com/chipsalliance/verible"}
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 (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 16c514e

Please sign in to comment.