Skip to content

Commit 8410bbd

Browse files
authored
RV: Protect the .trap section (#4225)
* RV: Protect the .trap section * Honor a connected debugger * Refactor * Fixes and improvements * Fix feature gate * Fix * Make CI happy * Lock * Avoid weird behavior * Remove the .critical_data section * Fix * CHANGELOG.md * Apply `stack_guard_monitoring_with_debugger_connected` also to `write_vec_table_monitoring`
1 parent c4ba10b commit 8410bbd

File tree

13 files changed

+294
-33
lines changed

13 files changed

+294
-33
lines changed

esp-hal/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4040
- `ram(reclaimed)` as an alias for `link_section = ".dram2_uninit"` (#4245)
4141
- `rmt::MAX_TX_LOOPCOUNT` and `rmt::MAX_RX_IDLE_THRESHOLD` constants have been added (#4276)
4242
- Added support for `embedded-io 0.7` (#4280)
43+
- A new option `ESP_HAL_CONFIG_WRITE_VEC_TABLE_MONITORING` (disabled by default) to check that no unintentional writes to a very vital memory area are made. (Only RISC-V) (#4225)
4344

4445
### Changed
4546

esp-hal/esp_config.yml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,14 @@ options:
112112
default:
113113
- value: true
114114

115+
- name: write-vec-table-monitoring
116+
description: Use a data watchpoint to check that the vector table was not unintentionally overwritten.
117+
default:
118+
- value: false
119+
active: 'chip == "esp32c2" || chip == "esp32c3" || chip == "esp32c6" || chip == "esp32h2"'
120+
115121
- name: stack-guard-monitoring-with-debugger-connected
116-
description: Enable the stack guard also with a debugger connected.
122+
description: Enable the stack guard also with a debugger connected. Also applies to `write-vec-table-monitoring`.
117123
default:
118124
- value: true
119125

esp-hal/ld/esp32c2/esp32c2.x

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,8 @@ INCLUDE "metadata.x"
5454
INCLUDE "eh_frame.x"
5555
/* End of Shared sections */
5656

57-
_dram_origin = ORIGIN( DRAM );
57+
#IF ESP_HAL_CONFIG_FLIP_LINK
58+
_dram_data_start = ORIGIN( DRAM );
59+
#ELSE
60+
_dram_data_start = ORIGIN( DRAM ) + SIZEOF(.trap) + SIZEOF(.rwtext);
61+
#ENDIF

esp-hal/ld/esp32c3/esp32c3.x

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,4 +55,8 @@ INCLUDE "metadata.x"
5555
INCLUDE "eh_frame.x"
5656
/* End of Shared sections */
5757

58-
_dram_origin = ORIGIN( DRAM );
58+
#IF ESP_HAL_CONFIG_FLIP_LINK
59+
_dram_data_start = ORIGIN( DRAM );
60+
#ELSE
61+
_dram_data_start = ORIGIN( DRAM ) + SIZEOF(.trap) + SIZEOF(.rwtext);
62+
#ENDIF

esp-hal/ld/esp32c6/esp32c6.x

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,8 @@ INCLUDE "metadata.x"
3939
INCLUDE "eh_frame.x"
4040
/* End of Shared sections #2 */
4141

42-
_dram_origin = ORIGIN( RAM );
42+
#IF ESP_HAL_CONFIG_FLIP_LINK
43+
_dram_data_start = ORIGIN( RAM );
44+
#ELSE
45+
_dram_data_start = ORIGIN( RAM ) + SIZEOF(.trap) + SIZEOF(.rwtext);
46+
#ENDIF

esp-hal/ld/esp32h2/esp32h2.x

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,8 @@ INCLUDE "metadata.x"
3737
INCLUDE "eh_frame.x"
3838
/* End of Shared sections #2 */
3939

40-
_dram_origin = ORIGIN( RAM );
40+
#IF ESP_HAL_CONFIG_FLIP_LINK
41+
_dram_data_start = ORIGIN( RAM );
42+
#ELSE
43+
_dram_data_start = ORIGIN( RAM ) + SIZEOF(.trap) + SIZEOF(.rwtext);
44+
#ENDIF

esp-hal/ld/sections/rwtext.x

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#IF riscv
22
.trap : ALIGN(4)
33
{
4+
_trap_section_origin = .;
45
KEEP(*(.trap));
56
*(.trap.*);
67
} > RWTEXT
@@ -27,4 +28,6 @@
2728
*( .wifiextrairam.* )
2829
*( .coexiram.* )
2930
. = ALIGN(4);
31+
32+
_rwtext_len = . - ORIGIN(RWTEXT);
3033
} > RWTEXT

esp-hal/src/debugger.rs

Lines changed: 180 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -53,26 +53,187 @@ pub unsafe fn set_stack_watchpoint(addr: usize) {
5353
);
5454
}
5555
} else {
56-
let addr = (addr & !0b11) | 0b01;
57-
58-
let id = 0; // breakpoint 0
59-
let tdata = (1 << 3) | (1 << 6) | (1 << 1); // bits: 0 = load, 1 = store, 6 = m-mode, 3 = u-mode
60-
let tcontrol = 1 << 3; // M-mode trigger
61-
62-
unsafe {
63-
core::arch::asm!(
64-
"
65-
csrw 0x7a0, {id} // tselect
66-
csrrs {tcontrol}, 0x7a5, {tcontrol} // tcontrol
67-
csrrs {tdata}, 0x7a1, {tdata} // tdata1
68-
csrw 0x7a2, {addr} // tdata2
69-
", id = in(reg) id,
70-
addr = in(reg) addr,
71-
tdata = in(reg) tdata,
72-
tcontrol = in(reg) tcontrol,
73-
);
74-
}
56+
unsafe { set_watchpoint(0, addr, 4); }
7557
}
7658
}
7759
}
7860
}
61+
62+
#[cfg(riscv)]
63+
pub(crate) static DEBUGGER_LOCK: esp_sync::RawMutex = esp_sync::RawMutex::new();
64+
65+
#[cfg(riscv)]
66+
const NAPOT_MATCH: u8 = 1;
67+
68+
#[cfg(riscv)]
69+
bitfield::bitfield! {
70+
/// Only match type (0x2) triggers are supported.
71+
#[derive(Clone, Copy, Default)]
72+
pub(crate) struct Tdata1(u32);
73+
74+
/// Set this for configuring the selected trigger to fire right before a load operation with matching
75+
/// data address is executed by the CPU.
76+
pub bool, load, set_load: 0;
77+
78+
/// Set this for configuring the selected trigger to fire right before a store operation with matching
79+
/// data address is executed by the CPU.
80+
pub bool, store, set_store: 1;
81+
82+
/// Set this for configuring the selected trigger to fire right before an instruction with matching
83+
/// virtual address is executed by the CPU.
84+
pub bool, execute, set_execute: 2;
85+
86+
/// Set this for enabling selected trigger to operate in user mode.
87+
pub bool, u, set_u: 3;
88+
89+
/// Set this for enabling selected trigger to operate in machine mode.
90+
pub bool, m, set_m: 6;
91+
92+
/// Configures the selected trigger to perform one of the available matching operations on a
93+
/// data/instruction address. Valid options are:
94+
/// 0x0: exact byte match, i.e. address corresponding to one of the bytes in an access must match
95+
/// the value of maddress exactly.
96+
/// 0x1: NAPOT match, i.e. at least one of the bytes of an access must lie in the NAPOT region
97+
/// specified in maddress.
98+
/// Note: Writing a larger value will clip it to the largest possible value 0x1.
99+
pub u8, _match, set_match: 10, 7;
100+
101+
/// Configures the selected trigger to perform one of the available actions when firing. Valid
102+
/// options are:
103+
/// 0x0: cause breakpoint exception.
104+
/// 0x1: enter debug mode (only valid when dmode = 1)
105+
/// Note: Writing an invalid value will set this to the default value 0x0.
106+
pub u8, action, set_action: 15, 12;
107+
108+
/// This is found to be 1 if the selected trigger had fired previously. This bit is to be cleared manually.
109+
pub bool, hit, set_hit: 20;
110+
111+
/// 0: Both Debug and M mode can write the tdata1 and tdata2 registers at the selected tselect.
112+
/// 1: Only Debug Mode can write the tdata1 and tdata2 registers at the selected tselect. Writes from
113+
/// other modes are ignored.
114+
/// Note: Only writable from debug mode.
115+
pub bool, dmode, set_dmode: 27;
116+
}
117+
118+
#[cfg(riscv)]
119+
bitfield::bitfield! {
120+
/// Only match type (0x2) triggers are supported.
121+
#[derive(Clone, Copy, Default)]
122+
pub(crate) struct Tcontrol(u32);
123+
124+
/// Current M mode trigger enable bit
125+
pub bool, mte, set_mte: 3;
126+
127+
/// Previous M mode trigger enable bit
128+
pub bool, mpte, set_mpte: 7;
129+
130+
}
131+
132+
#[cfg(riscv)]
133+
pub(crate) struct WatchPoint {
134+
tdata1: u32,
135+
tdata2: u32,
136+
}
137+
138+
/// Clear the watchpoint
139+
#[cfg(riscv)]
140+
pub(crate) unsafe fn clear_watchpoint(id: u8) -> WatchPoint {
141+
assert!(id < 4);
142+
143+
// tdata1 is a WARL(write any read legal) register. We can just write 0 to it.
144+
let mut tdata1 = 0;
145+
let mut tdata2 = 0;
146+
147+
DEBUGGER_LOCK.lock(|| unsafe {
148+
core::arch::asm!(
149+
"
150+
csrw 0x7a0, {id} // tselect
151+
csrrw {tdata1}, 0x7a1, {tdata2} // tdata1
152+
csrr {tdata2}, 0x7a2 // tdata2
153+
", id = in(reg) id,
154+
tdata1 = inout(reg) tdata1,
155+
tdata2 = out(reg) tdata2,
156+
);
157+
});
158+
159+
WatchPoint { tdata1, tdata2 }
160+
}
161+
162+
/// Clear the watchpoint
163+
#[cfg(riscv)]
164+
pub(crate) unsafe fn restore_watchpoint(id: u8, watchpoint: WatchPoint) {
165+
DEBUGGER_LOCK.lock(|| unsafe {
166+
core::arch::asm!(
167+
"
168+
csrw 0x7a0, {id} // tselect
169+
csrw 0x7a1, {tdata1} // tdata1
170+
csrw 0x7a2, {tdata2} // tdata2
171+
", id = in(reg) id,
172+
tdata1 = in(reg) watchpoint.tdata1,
173+
tdata2 = in(reg) watchpoint.tdata2,
174+
);
175+
});
176+
}
177+
178+
/// Clear the watchpoint
179+
#[cfg(all(riscv, feature = "exception-handler"))]
180+
pub(crate) unsafe fn watchpoint_hit(id: u8) -> bool {
181+
assert!(id < 4);
182+
let mut tdata = Tdata1::default();
183+
184+
DEBUGGER_LOCK.lock(|| unsafe {
185+
core::arch::asm!(
186+
"
187+
csrw 0x7a0, {id} // tselect
188+
csrr {tdata}, 0x7a1 // tdata1
189+
", id = in(reg) id,
190+
tdata = out(reg) tdata.0,
191+
);
192+
});
193+
194+
tdata.hit()
195+
}
196+
197+
/// Set watchpoint and enable triggers.
198+
#[cfg(riscv)]
199+
pub(crate) unsafe fn set_watchpoint(id: u8, addr: usize, len: usize) {
200+
assert!(id < 4);
201+
assert!(len.is_power_of_two());
202+
assert!(addr.is_multiple_of(len));
203+
204+
let z = len.trailing_zeros();
205+
let mask = {
206+
let mut mask: usize = 0;
207+
for i in 0..z {
208+
mask |= 1 << i;
209+
}
210+
mask
211+
};
212+
213+
let napot_encoding = { mask & !(1 << (z - 1)) };
214+
let addr = (addr & !mask) | napot_encoding;
215+
216+
let mut tdata = Tdata1::default();
217+
tdata.set_m(true);
218+
tdata.set_store(true);
219+
tdata.set_match(NAPOT_MATCH);
220+
let tdata: u32 = tdata.0;
221+
222+
let mut tcontrol = Tcontrol::default();
223+
tcontrol.set_mte(true);
224+
let tcontrol: u32 = tcontrol.0;
225+
226+
DEBUGGER_LOCK.lock(|| unsafe {
227+
core::arch::asm!(
228+
"
229+
csrw 0x7a0, {id} // tselect
230+
csrw 0x7a5, {tcontrol} // tcontrol
231+
csrw 0x7a1, {tdata} // tdata1
232+
csrw 0x7a2, {addr} // tdata2
233+
", id = in(reg) id,
234+
addr = in(reg) addr,
235+
tdata = in(reg) tdata,
236+
tcontrol = in(reg) tcontrol,
237+
);
238+
});
239+
}

esp-hal/src/exception_handler/mod.rs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,22 @@ unsafe extern "C" fn ExceptionHandler(context: &TrapFrame) -> ! {
7070
mepc, context.ra
7171
)
7272
} else {
73-
panic!(
74-
"Breakpoint exception at 0x{:08x}, mtval=0x{:08x}\n{:?}",
75-
mepc, mtval, context
76-
);
73+
if unsafe { crate::debugger::watchpoint_hit(1) } {
74+
panic!(
75+
"Detected a write to the trap/rwtext segment at 0x{:x}, possibly called by 0x{:x}",
76+
mepc, context.ra
77+
);
78+
} else if unsafe { crate::debugger::watchpoint_hit(0) } {
79+
panic!(
80+
"Detected a write to a stack guard value at 0x{:x}, possibly called by 0x{:x}",
81+
mepc, context.ra
82+
);
83+
} else {
84+
panic!(
85+
"Breakpoint exception at 0x{:08x}, mtval=0x{:08x}\n{:?}",
86+
mepc, mtval, context
87+
);
88+
}
7789
}
7890
}
7991

esp-hal/src/interrupt/riscv.rs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,16 @@ pub fn enable_direct(
260260

261261
let instr = encode_jal_x0(handler as usize, int_slot)?;
262262

263-
core::ptr::write_volatile(int_slot as *mut u32, instr);
263+
if crate::debugger::debugger_connected() {
264+
core::ptr::write_volatile(int_slot as *mut u32, instr);
265+
} else {
266+
crate::debugger::DEBUGGER_LOCK.lock(|| {
267+
let wp = crate::debugger::clear_watchpoint(1);
268+
core::ptr::write_volatile(int_slot as *mut u32, instr);
269+
crate::debugger::restore_watchpoint(1, wp);
270+
});
271+
}
272+
264273
core::arch::asm!("fence.i");
265274

266275
enable_cpu_interrupt(cpu_interrupt);
@@ -452,7 +461,16 @@ mod vectored {
452461
unsafe {
453462
let ptr =
454463
&pac::__EXTERNAL_INTERRUPTS[interrupt as usize]._handler as *const _ as *mut usize;
455-
ptr.write_volatile(handler.raw_value());
464+
465+
if crate::debugger::debugger_connected() {
466+
ptr.write_volatile(handler.raw_value());
467+
} else {
468+
crate::debugger::DEBUGGER_LOCK.lock(|| {
469+
let wp = crate::debugger::clear_watchpoint(1);
470+
ptr.write_volatile(handler.raw_value());
471+
crate::debugger::restore_watchpoint(1, wp);
472+
});
473+
}
456474
}
457475
}
458476

0 commit comments

Comments
 (0)