Hi Masami,
I looked into the issue Sashiko reported for riscv_probe_decode_insn(). Sharing the analysis below; the short version is that this read predates this series, and I'm happy to send a separate patch if the maintainers think it is worth fixing. First, the context: "probe_opcode_t insn = *addr;" is pre-existing code. This series only adds two RISCV_INSN_REJECTED() entries, which consume the already-loaded value and do not change how memory is accessed. On the unaligned part: with default configurations, misaligned accesses are emulated, either by the firmware or by the kernel itself (RISCV_SCALAR_MISALIGNED), so an oops would require a NONPORTABLE kernel built with RISCV_SLOW/EFFICIENT_UNALIGNED_ACCESS running on hardware that does not actually support unaligned accesses. On the page-crossing part: this one is real in theory, though the window is narrow. It cannot happen for built-in kernel text (the linear mapping is contiguous). For modules, it needs a compressed instruction at the last two bytes of a module text page followed by the vmalloc guard page, plus a kprobe registered exactly there. A 32-bit instruction cannot sit at that location on a working module (its encoding would cross into the unmapped guard page, so it could never be fetched), which is also why the two-u16-step copy in arch_prepare_kprobe() is safe. If it is worth fixing, the cleanest approach seems to be passing the opcode by value instead of a pointer: arch_prepare_kprobe() already holds a safely copied p->opcode, and the uprobes caller passes a stack variable, so both call sites would be covered. There is a third similar raw read in uprobes.c (the auprobe->insn[] dereference) that could be folded in as well. Either way, this doesn't need to be handled in the series: the upcoming v7 will only address Nam's review comments. Thanks, Xiaofeng > -----原始邮件----- > 发件人: [email protected] > 发送时间:2026-10-16 16:29:42 (星期五) > 收件人: "Xiaofeng Yuan" <[email protected]> > 抄送: [email protected] > 主题: Re: [PATCH v6 1/2] riscv: kprobes: reject probes inside LR/SC sequences > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [High] riscv: kprobes: unsafe 32-bit read in riscv_probe_decode_insn can > page fault > -- > > commit 6f940578d2f096970e9abd6cc2894de008e29f6f > Author: Xiaofeng Yuan <[email protected]> > > riscv: kprobes: reject probes inside LR/SC sequences > > This patch rejects kprobes placed inside LR/SC sequences or on the LR/SC > instructions themselves to avoid livelock. It updates > riscv_probe_decode_insn() > to reject the instructions and adds instruction walking logic in > arch_check_kprobe() to reject addresses within a sequence. > > [ ... ] > > > diff --git a/arch/riscv/kernel/probes/decode-insn.c > > b/arch/riscv/kernel/probes/decode-insn.c > > index 433d9035b8882..e6a8894677b10 100644 > > --- a/arch/riscv/kernel/probes/decode-insn.c > > +++ b/arch/riscv/kernel/probes/decode-insn.c > > @@ -23,6 +23,8 @@ riscv_probe_decode_insn(probe_opcode_t *addr, struct > > arch_probe_insn *api) > > */ > > RISCV_INSN_REJECTED(system, insn); > > RISCV_INSN_REJECTED(fence, insn); > > + RISCV_INSN_REJECTED(lr, insn); > > + RISCV_INSN_REJECTED(sc, insn); > > [Severity: High] > Does evaluating these macros rely on the insn variable, which is read > unconditionally as a native 32-bit integer earlier in the function? > > arch/riscv/kernel/probes/decode-insn.c:riscv_probe_decode_insn() { > probe_opcode_t insn = *addr; > ... > > If addr is 2-byte aligned, will this native 32-bit dereference perform > an unaligned memory access, potentially oopsing on RISC-V hardware that > lacks kernel-mode unaligned access support? > > Furthermore, if addr points to a 16-bit compressed instruction at the > end of a mapped page and the next page is an unmapped vmalloc guard > page, could this 32-bit read cross the page boundary into unmapped > memory? > > Would this trigger a kernel page fault and panic when tracing tools > register a kprobe at such an address? > > -- > Sashiko AI review · > https://sashiko.dev/#/patchset/[email protected]?part=1
