hitHuang opened a new pull request, #19468:
URL: https://github.com/apache/nuttx/pull/19468
## Summary
qemu-rv's S-mode/non-SMP `setintstack` macro
(`arch/risc-v/src/qemu-rv/chip.h`) unconditionally reloaded `sp` to the top of
the per-cpu interrupt stack on every trap entry. If a trap is taken while
already running on the interrupt stack (e.g. a fault occurring inside an
interrupt handler), this rewinds `sp` back to the same fixed top-of-stack
address, causing the nested trap's frame to be laid down on top of the
still-live outer trap's frame and overwrite it.
The canonical `setintstack` in `riscv_macros.S` already guards this reload
with a bounds check: it only moves `sp` when `sp` is outside the interrupt
stack range. This change ports that same logic to qemu-rv's per-cpu
`RISCV_PERCPU_IRQSTACK` variant, and factors the shared bounds-check sequence
out into a `setintstack_bounds` macro reused by both the SMP and S-mode/non-SMP
`setintstack` variants in `chip.h`.
Other vendor `chip.h` files (litex, mpfs, and the non-SMP `setintstack`
variants of jh7110/k230/sg2000) have the same unconditional-reload pattern and
are left for a follow-up, since they cannot be exercised on QEMU.
### Why other vendor chips aren't fixed in this PR
Tracing the history of `setintstack` on qemu-rv specifically: its S-mode
variant was added in 2022 (`3193aa3c97`), as a plain unconditional `REGLOAD sp,
RISCV_PERCPU_IRQSTACK(...)` — the nested-trap bounds-check concept didn't exist
yet. The bounds check itself was introduced later, in 2024, by `b4174952843`
("arch/risc-v: support backtrace dump during IRQ"). That commit only touched
two places: the canonical macro in `riscv_macros.S`, and qemu-rv's *SMP*
`setintstack` variant that already existed in `chip.h` at the time. Its own
commit message ("Tested with SMP, no SMP and no interrupt stack") shows the
S-mode/non-SMP combination simply wasn't in the test matrix, so qemu-rv's
S-mode variant was left with the 2022 unconditional-reload code until this PR.
There is no shared/canonical `setintstack` for S-mode or SMP the way there
is for the plain single-core M-mode case in `riscv_macros.S` — each chip
vendor's `chip.h` reimplements both variants independently, copying the same
style of code. Checking the other 9 vendor `chip.h` files (litex, mpfs, jh7110,
k230, sg2000, eic7700x, bl808, k210, rp23xx-rv) shows every one of them still
has the plain unconditional reload in *both* their SMP and S-mode variants —
the 2024 bounds-check fix was never propagated to any of them, not just their
S-mode paths. That's the actual shape of the current fragmentation: one
single-file fix landed on qemu-rv only, and nothing else was ever touched. I
don't have any of that hardware on hand to reproduce and verify a fix, so
they're left untouched for now rather than changed without being able to test
them.
## Impact
- Affects only `arch/risc-v/src/qemu-rv/chip.h`, specifically the
S-mode/non-SMP `setintstack` macro used when `CONFIG_ARCH_USE_S_MODE=y &&
!CONFIG_SMP && CONFIG_ARCH_INTERRUPTSTACK > 15`.
- No Kconfig/API changes. No effect on builds without a dedicated interrupt
stack, or on the SMP variant (already had the bounds check).
- Fixes real stack-memory corruption on nested traps in this configuration;
no behavior change for the non-nested-trap case.
## Testing
Config: `CONFIG_BUILD_KERNEL=y`, non-SMP, `CONFIG_ARCH_INTERRUPTSTACK > 15`.
Boards: qemu `rv-virt:knsh` (RV32) and `rv-virt:knsh64` (RV64).
To reliably reproduce a fault occurring inside an interrupt handler (so the
nested-trap path through `setintstack` is exercised), the mtimer interrupt
handler was temporarily instrumented to dereference a NULL pointer when a test
flag is set via gdb:
```
huang@ubuntu:~/workspace/nuttx-fork/nuttx$ git diff
arch/risc-v/src/common/riscv_mtimer.c
diff --git a/arch/risc-v/src/common/riscv_mtimer.c
b/arch/risc-v/src/common/riscv_mtimer.c
index 3c29088667..7140d89f5a 100644
--- a/arch/risc-v/src/common/riscv_mtimer.c
+++ b/arch/risc-v/src/common/riscv_mtimer.c
@@ -26,9 +26,13 @@
#include <nuttx/arch.h>
#include <nuttx/irq.h>
+#include <nuttx/sched.h>
+#include <sched.h>
#include "riscv_mtimer.h"
+volatile int g_mtimer_test = 0;
+
/****************************************************************************
* Private Types
****************************************************************************/
@@ -132,6 +136,12 @@ static int riscv_mtimer_interrupt(int irq, void
*context, void *arg)
{
struct riscv_mtimer_lowerhalf_s *priv = &g_riscv_mtime_lowerhalf;
+ if (g_mtimer_test == 1)
+ {
+ g_mtimer_test = 0;
+ *(volatile uint32_t *)0 = 0;
+ }
+
riscv_mtimer_set_mtimecmp(priv, UINT64_MAX);
oneshot_process_callback(&priv->lower);
```
Test method: set breakpoints on `riscv_mtimer_interrupt()` and
`riscv_exception()` via gdb, and capture `sp` at each breakpoint to check
whether the nested trap's entry into `riscv_exception()` clobbers the
still-live outer (mtimer interrupt) frame.
**Before fix:**
```
0x00001000 in ?? ()
add symbol table from file "../../nuttx-fork/nuttx/nuttx"
(gdb) c
Continuing.
^C
Program received signal SIGINT, Interrupt.
0x80003ecc in ?? ()
(gdb) b riscv_exception
Breakpoint 1 at 0x8020c730: file common/riscv_exception.c, line 148.
(gdb) b riscv_mtimer_interrupt
Breakpoint 2 at 0x8020cc3a: file common/riscv_mtimer.c, line 139.
(gdb) c
Continuing.
Breakpoint 2, riscv_mtimer_interrupt (irq=21, context=0x80608564,
arg=0x80600138 <g_riscv_mtime_lowerhalf>) at common/riscv_mtimer.c:139
139 if (g_mtimer_test == 1)
(gdb) info registers sp fp
sp 0x806067a8 0x806067a8 <g_intstackalloc+1960>
fp 0x806067b8 0x806067b8 <g_intstackalloc+1976>
(gdb) set variable g_mtimer_test=1
(gdb) c
Continuing.
Breakpoint 1, riscv_exception (mcause=7, regs=0x8060661c
<g_intstackalloc+1564>, args=0x0) at common/riscv_exception.c:148
148 uintreg_t cause = mcause & RISCV_IRQ_MASK;
(gdb) info registers sp fp
sp 0x806067a8 0x806067a8 <g_intstackalloc+1960>
fp 0x806067b8 0x806067b8 <g_intstackalloc+1976>
(gdb) bt
#0 riscv_exception (mcause=7, regs=0x8060661c <g_intstackalloc+1564>,
args=0x0) at common/riscv_exception.c:148
#1 0x80201fec in irq_dispatch (irq=irq@entry=7, context=<optimized out>) at
irq/irq_dispatch.c:143
#2 0x8020162a in riscv_doirq (irq=7, regs=0x8060661c
<g_intstackalloc+1564>) at common/riscv_doirq.c:112
#3 0x80201262 in riscv_dispatch_irq (vector=<optimized out>,
regs=<optimized out>) at chip/qemu_rv_irq_dispatch.c:140
#4 0x802001d2 in handle_irq () at common/riscv_exception_common.S:238
#5 0x00000000 in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)
```
This shows that for the exception caused by the NULL dereference inside
riscv_mtimer_interrupt(), sp upon entering riscv_exception() is again at the
top of the interrupt stack.
**After fix:**
```
0x00001000 in ?? ()
add symbol table from file "../../nuttx-fork/nuttx/nuttx"
(gdb) c
Continuing.
^C
Program received signal SIGINT, Interrupt.
0x8020cf7e in up_idle () at common/riscv_idle.c:77
77 }
(gdb) b riscv_exception
Breakpoint 1 at 0x8020c730: file common/riscv_exception.c, line 148.
(gdb) b riscv_mtimer_interrupt
Breakpoint 2 at 0x8020cc3a: file common/riscv_mtimer.c, line 139.
(gdb) c
Continuing.
Breakpoint 2, riscv_mtimer_interrupt (irq=21, context=0x80608564,
arg=0x80600138 <g_riscv_mtime_lowerhalf>) at common/riscv_mtimer.c:139
139 if (g_mtimer_test == 1)
(gdb) info registers sp fp
sp 0x806067a8 0x806067a8 <g_intstackalloc+1960>
fp 0x806067b8 0x806067b8 <g_intstackalloc+1976>
(gdb) set variable g_mtimer_test=1
(gdb) c
Continuing.
Breakpoint 1, riscv_exception (mcause=7, regs=0x8060661c
<g_intstackalloc+1564>, args=0x0) at common/riscv_exception.c:148
148 uintreg_t cause = mcause & RISCV_IRQ_MASK;
(gdb) info registers sp fp
sp 0x806065c4 0x806065c4 <g_intstackalloc+1476>
fp 0x806065d4 0x806065d4 <g_intstackalloc+1492>
(gdb) bt
#0 riscv_exception (mcause=7, regs=0x8060661c <g_intstackalloc+1564>,
args=0x0) at common/riscv_exception.c:148
#1 0x80201fec in irq_dispatch (irq=irq@entry=7, context=<optimized out>) at
irq/irq_dispatch.c:143
#2 0x8020162a in riscv_doirq (irq=7, regs=0x8060661c
<g_intstackalloc+1564>) at common/riscv_doirq.c:112
#3 0x80201262 in riscv_dispatch_irq (vector=<optimized out>,
regs=<optimized out>) at chip/qemu_rv_irq_dispatch.c:140
#4 0x802001e8 in handle_irq () at common/riscv_exception_common.S:238
#5 0x8020cc4c in riscv_mtimer_interrupt (irq=7, context=0x8060661c
<g_intstackalloc+1564>, arg=0x0) at common/riscv_mtimer.c:141
Backtrace stopped: frame did not save the PC
```
After the fix, sp at the riscv_exception() breakpoint now has a sane value:
still on the interrupt stack, but no longer at the top.
Also regression-tested on knsh64 to confirm the SMP variant is unaffected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
