From 833bf63bc8d8245a1888ed10edd2d1fed2c8d0a8 Mon Sep 17 00:00:00 2001 From: Daniel Jobson Date: Fri, 15 Nov 2024 11:19:40 +0100 Subject: [PATCH] PoC: Make sensitive assets only readable/writable before system_mode is set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the first time system_mode is set to one, the assets will no longer be read- or writeable, even if system_mode is set to zero at a later syscall. This is to make sure syscalls does not have the same privilege as the firmware has at first boot. We need to monitor when system_mode is set to one, otherwise we might accedentially lock the assets before actually leaving firmware, for example if firmware would use a function set in any of the registers used in system_mode_ctrl. Co-authored-by: Mikael Ågren --- hw/application_fpga/README.md | 29 +++++++++++++++---- hw/application_fpga/core/tk1/rtl/tk1.v | 21 ++++++++------ hw/application_fpga/core/uds/rtl/uds.v | 5 ++-- hw/application_fpga/rtl/application_fpga.v | 7 ++--- hw/application_fpga/tb/application_fpga_sim.v | 6 ++-- 5 files changed, 44 insertions(+), 24 deletions(-) diff --git a/hw/application_fpga/README.md b/hw/application_fpga/README.md index ee25709..1ebeb5a 100644 --- a/hw/application_fpga/README.md +++ b/hw/application_fpga/README.md @@ -132,17 +132,36 @@ Interrupts can be enabled/disabled using the PicoRV32 specific The following table shows resource availablility for each execution mode: -| *Execution Mode* | *ROM* | *FW RAM* | *SPI* | -|---------------------|--------|----------|-------| -| Firmware mode | r/x | r/w | r/w | -| IRQ_SYSCALL | r/x | r/w | r/w | -| Application mode | r | i | i | +| *Execution Mode* | *ROM* | *FW RAM* | *SPI* | *Sensitive assets* | +|------------------|--------|----------|-------|--------------------| +| Firmware mode | r/x | r/w | r/w | r/w* | +| IRQ_SYSCALL | r/x | r/w | r/w | r* | +| Application mode | r | i | i | r* | Legend: r = readable w = writeable x = executable i = invisible +* = read-/writeability varies, see below + +These sensitive assets are only readable and/or writeable in firmware +mode: +- APP_START +- APP_SIZE +- CDI_FIRST +- CDI_LAST +- RAM_ADDR_RAND +- RAM_DATA_RAND +- UDI_FIRST +- UDI_LAST +- UDS_FIRST +- UDS_LAST + +Note that these assets have different properties, some are read-only +and some are write-only. The list above only shows if they are +restricted in app mode. See each individual API to find out more about +their properties. ## `tk1` diff --git a/hw/application_fpga/core/tk1/rtl/tk1.v b/hw/application_fpga/core/tk1/rtl/tk1.v index cd710b1..523e61e 100644 --- a/hw/application_fpga/core/tk1/rtl/tk1.v +++ b/hw/application_fpga/core/tk1/rtl/tk1.v @@ -20,7 +20,7 @@ module tk1 #( input wire reset_n, input wire cpu_trap, - output wire system_mode, + output wire rw_locked, input wire [31 : 0] cpu_addr, input wire cpu_instr, @@ -185,14 +185,14 @@ module tk1 #( wire rom_exec_en; + wire system_mode; + //---------------------------------------------------------------- // Concurrent connectivity for ports etc. //---------------------------------------------------------------- assign read_data = tmp_read_data; assign ready = tmp_ready; - assign system_mode = system_mode_reg; - assign force_trap = force_trap_reg; assign gpio3 = gpio3_reg; @@ -203,9 +203,12 @@ module tk1 #( assign system_reset = system_reset_reg; + assign system_mode = system_mode_reg; + assign rom_exec_en = !system_mode | access_level_hi; assign fw_ram_en = !system_mode | access_level_hi; assign spi_access_en = !system_mode | access_level_hi; + assign rw_locked = system_mode; //---------------------------------------------------------------- // Module instance. @@ -539,13 +542,13 @@ module tk1 #( end if (address == ADDR_APP_START) begin - if (!system_mode_reg) begin + if (!rw_locked) begin app_start_we = 1'h1; end end if (address == ADDR_APP_SIZE) begin - if (!system_mode_reg) begin + if (!rw_locked) begin app_size_we = 1'h1; end end @@ -555,19 +558,19 @@ module tk1 #( end if ((address >= ADDR_CDI_FIRST) && (address <= ADDR_CDI_LAST)) begin - if (!system_mode_reg) begin + if (!rw_locked) begin cdi_mem_we = 1'h1; end end if (address == ADDR_RAM_ADDR_RAND) begin - if (!system_mode_reg) begin + if (!rw_locked) begin ram_addr_rand_we = 1'h1; end end if (address == ADDR_RAM_DATA_RAND) begin - if (!system_mode_reg) begin + if (!rw_locked) begin ram_data_rand_we = 1'h1; end end @@ -645,7 +648,7 @@ module tk1 #( end if ((address >= ADDR_UDI_FIRST) && (address <= ADDR_UDI_LAST)) begin - if (!system_mode_reg) begin + if (!rw_locked) begin tmp_read_data = udi_rdata; end end diff --git a/hw/application_fpga/core/uds/rtl/uds.v b/hw/application_fpga/core/uds/rtl/uds.v index 2aaa112..8c8f1a3 100644 --- a/hw/application_fpga/core/uds/rtl/uds.v +++ b/hw/application_fpga/core/uds/rtl/uds.v @@ -17,8 +17,7 @@ module uds ( input wire clk, input wire reset_n, - input wire system_mode, - + input wire en, input wire cs, input wire [ 2 : 0] address, output wire [31 : 0] read_data, @@ -89,7 +88,7 @@ module uds ( if (cs) begin tmp_ready = 1'h1; - if (!system_mode) begin + if (en) begin if (uds_rd_reg[address[2 : 0]] == 1'h0) begin uds_rd_we = 1'h1; end diff --git a/hw/application_fpga/rtl/application_fpga.v b/hw/application_fpga/rtl/application_fpga.v index c1eb269..3e084bf 100644 --- a/hw/application_fpga/rtl/application_fpga.v +++ b/hw/application_fpga/rtl/application_fpga.v @@ -154,7 +154,7 @@ module application_fpga ( reg [31 : 0] tk1_write_data; wire [31 : 0] tk1_read_data; wire tk1_ready; - wire system_mode; + wire rw_locked; wire force_trap; wire [14 : 0] ram_addr_rand; wire [31 : 0] ram_data_rand; @@ -256,7 +256,6 @@ module application_fpga ( .reset_n(reset_n), .en(fw_ram_en), - .cs(fw_ram_cs), .we(fw_ram_we), .address(fw_ram_address), @@ -295,7 +294,7 @@ module application_fpga ( .clk(clk), .reset_n(reset_n), - .system_mode(system_mode), + .en(~rw_locked), .cs(uds_cs), .address(uds_address), @@ -341,7 +340,7 @@ module application_fpga ( .clk(clk), .reset_n(reset_n), - .system_mode(system_mode), + .rw_locked(rw_locked), .cpu_addr (cpu_addr), .cpu_instr (cpu_instr), diff --git a/hw/application_fpga/tb/application_fpga_sim.v b/hw/application_fpga/tb/application_fpga_sim.v index 5a21ac7..c3e41fc 100644 --- a/hw/application_fpga/tb/application_fpga_sim.v +++ b/hw/application_fpga/tb/application_fpga_sim.v @@ -166,7 +166,7 @@ module application_fpga_sim ( reg [31 : 0] tk1_write_data; wire [31 : 0] tk1_read_data; wire tk1_ready; - wire system_mode; + wire rw_locked; wire force_trap; wire [14 : 0] ram_addr_rand; wire [31 : 0] ram_data_rand; @@ -305,7 +305,7 @@ module application_fpga_sim ( .clk(clk), .reset_n(reset_n), - .system_mode(system_mode), + .en(~rw_locked), .cs(uds_cs), .address(uds_address), @@ -353,7 +353,7 @@ module application_fpga_sim ( .clk(clk), .reset_n(reset_n), - .system_mode(system_mode), + .rw_locked(rw_locked), .cpu_addr (cpu_addr), .cpu_instr (cpu_instr),