* [PATCH v4 0/3] riscv: probes: reject probes inside an LR/SC sequence
@ 2026-11-15 12:37 ` Vladimir Kondratiev
0 siblings, 0 replies; 9+ messages in thread
From: Vladimir Kondratiev @ 2026-11-15 12:37 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Guo Ren,
Pekka Enberg, Patrick Stählin
Cc: linux-riscv, linux-kernel, linux-trace-kernel, Palmer Dabbelt,
Vladimir Kondratiev, Sashiko, Xiaofeng Yuan
Patch 1 fixes page fault reported by sashiko-bot@kernel.org that
existed before this patch series
Patch 2 rejects probes inside the LR/SC sequence. This commit was
initially submitted as patch 1 of the series [1].
Detaching it from that series because it is not depending on other stuff.
Patch 3 adds KUnit test, suggested by
Xiaofeng Yuan <yuanxiaofeng@eswincomputing.com> in [2]
[1] https://patch.msgid.link/20261110-lrsc-only-v6-0-bafc3cc0fcae@mobileye.com
[2] https://lore.kernel.org/linux-riscv/20261019134121.1763-3-yuanxiaofeng@eswincomputing.com/
---
Changes in v4:
- Add KUnit test as suggested by Guo Ren <guoren@kernel.org>, based on
work by Xiaofeng Yuan <yuanxiaofeng@eswincomputing.com>
- Scan for SC up to maximum possible sequence length. SPU does not know
about symbols, it is not correct to stop scanning at the end of
symbol. Globally visible symbol placed within LR/SC sequence should
not turn probe within the sequence into acceptable.
- Link to v3: https://patch.msgid.link/20261112-probes-in-lrsc-v3-0-a95089852a17@mobileye.com
Changes in v3:
- Fix lr/sc loop detection logic for lr at probe position
- Add "Fixes" trailers as suggested by sashiko-bot@kernel.org
- Link to v2: https://patch.msgid.link/20261111-probes-in-lrsc-v2-0-b2525b7e1e8e@mobileye.com
Changes in v2:
- Allow kprobe on the LR itself unless it is part of the LR/SC loop.
- Fix error (reported by sashiko-bot@kernel.org) where
arch_prepare_kprobe() returns -EILSEQ for LR/SC probes.
Make codes returned by arch_check_kprobe to indicate -EINVAL
if probe falls within the LR/SC loop, consistent with the case when
INSN_REJECTED returned by riscv_probe_decode_insn
- add patch fixing out-of-bounds read in riscv_probe_decode_insn()
reported by sashiko-bot@kernel.org (as patch 1)
- Link to v1: https://patch.msgid.link/20261111-probes-in-lrsc-v1-1-4f99ddea5f68@mobileye.com
To: Paul Walmsley <pjw@kernel.org>
To: Palmer Dabbelt <palmer@dabbelt.com>
To: Albert Ou <aou@eecs.berkeley.edu>
To: Alexandre Ghiti <alex@ghiti.fr>
To: Masami Hiramatsu <mhiramat@kernel.org>
To: Oleg Nesterov <oleg@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
To: Guo Ren <guoren@kernel.org>
To: Pekka Enberg <penberg@kernel.org>
To: Patrick Stählin <me@packi.ch>
Cc: linux-riscv@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-trace-kernel@vger.kernel.org
Cc: Palmer Dabbelt <palmerdabbelt@google.com>
Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
---
Vladimir Kondratiev (3):
riscv: kprobes: fix out-of-bounds read in riscv_probe_decode_insn
riscv: probes: reject probes inside an LR/SC sequence
riscv: kprobes: add KUnit test for LR/SC sequence rejection
arch/riscv/include/asm/insn.h | 28 +++++++
arch/riscv/kernel/probes/decode-insn.c | 91 ++++++++++++++++++++++
arch/riscv/kernel/probes/decode-insn.h | 7 ++
arch/riscv/kernel/probes/kprobes.c | 28 +++++--
arch/riscv/kernel/probes/uprobes.c | 31 ++++++++
arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S | 58 ++++++++++++++
arch/riscv/kernel/tests/kprobes/test-kprobes.c | 32 ++++++++
arch/riscv/kernel/tests/kprobes/test-kprobes.h | 6 ++
8 files changed, 274 insertions(+), 7 deletions(-)
---
base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
change-id: 20261111-probes-in-lrsc-4ccde6cbf436
Best regards,
--
Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v4 0/3] riscv: probes: reject probes inside an LR/SC sequence @ 2026-11-15 12:37 ` Vladimir Kondratiev 0 siblings, 0 replies; 9+ messages in thread From: Vladimir Kondratiev @ 2026-11-15 12:37 UTC (permalink / raw) To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Guo Ren, Pekka Enberg, Patrick Stählin Cc: linux-riscv, linux-kernel, linux-trace-kernel, Palmer Dabbelt, Vladimir Kondratiev, Sashiko, Xiaofeng Yuan Patch 1 fixes page fault reported by sashiko-bot@kernel.org that existed before this patch series Patch 2 rejects probes inside the LR/SC sequence. This commit was initially submitted as patch 1 of the series [1]. Detaching it from that series because it is not depending on other stuff. Patch 3 adds KUnit test, suggested by Xiaofeng Yuan <yuanxiaofeng@eswincomputing.com> in [2] [1] https://patch.msgid.link/20261110-lrsc-only-v6-0-bafc3cc0fcae@mobileye.com [2] https://lore.kernel.org/linux-riscv/20261019134121.1763-3-yuanxiaofeng@eswincomputing.com/ --- Changes in v4: - Add KUnit test as suggested by Guo Ren <guoren@kernel.org>, based on work by Xiaofeng Yuan <yuanxiaofeng@eswincomputing.com> - Scan for SC up to maximum possible sequence length. SPU does not know about symbols, it is not correct to stop scanning at the end of symbol. Globally visible symbol placed within LR/SC sequence should not turn probe within the sequence into acceptable. - Link to v3: https://patch.msgid.link/20261112-probes-in-lrsc-v3-0-a95089852a17@mobileye.com Changes in v3: - Fix lr/sc loop detection logic for lr at probe position - Add "Fixes" trailers as suggested by sashiko-bot@kernel.org - Link to v2: https://patch.msgid.link/20261111-probes-in-lrsc-v2-0-b2525b7e1e8e@mobileye.com Changes in v2: - Allow kprobe on the LR itself unless it is part of the LR/SC loop. - Fix error (reported by sashiko-bot@kernel.org) where arch_prepare_kprobe() returns -EILSEQ for LR/SC probes. Make codes returned by arch_check_kprobe to indicate -EINVAL if probe falls within the LR/SC loop, consistent with the case when INSN_REJECTED returned by riscv_probe_decode_insn - add patch fixing out-of-bounds read in riscv_probe_decode_insn() reported by sashiko-bot@kernel.org (as patch 1) - Link to v1: https://patch.msgid.link/20261111-probes-in-lrsc-v1-1-4f99ddea5f68@mobileye.com To: Paul Walmsley <pjw@kernel.org> To: Palmer Dabbelt <palmer@dabbelt.com> To: Albert Ou <aou@eecs.berkeley.edu> To: Alexandre Ghiti <alex@ghiti.fr> To: Masami Hiramatsu <mhiramat@kernel.org> To: Oleg Nesterov <oleg@redhat.com> To: Peter Zijlstra <peterz@infradead.org> To: Guo Ren <guoren@kernel.org> To: Pekka Enberg <penberg@kernel.org> To: Patrick Stählin <me@packi.ch> Cc: linux-riscv@lists.infradead.org Cc: linux-kernel@vger.kernel.org Cc: linux-trace-kernel@vger.kernel.org Cc: Palmer Dabbelt <palmerdabbelt@google.com> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> --- Vladimir Kondratiev (3): riscv: kprobes: fix out-of-bounds read in riscv_probe_decode_insn riscv: probes: reject probes inside an LR/SC sequence riscv: kprobes: add KUnit test for LR/SC sequence rejection arch/riscv/include/asm/insn.h | 28 +++++++ arch/riscv/kernel/probes/decode-insn.c | 91 ++++++++++++++++++++++ arch/riscv/kernel/probes/decode-insn.h | 7 ++ arch/riscv/kernel/probes/kprobes.c | 28 +++++-- arch/riscv/kernel/probes/uprobes.c | 31 ++++++++ arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S | 58 ++++++++++++++ arch/riscv/kernel/tests/kprobes/test-kprobes.c | 32 ++++++++ arch/riscv/kernel/tests/kprobes/test-kprobes.h | 6 ++ 8 files changed, 274 insertions(+), 7 deletions(-) --- base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222 change-id: 20261111-probes-in-lrsc-4ccde6cbf436 Best regards, -- Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 1/3] riscv: kprobes: fix out-of-bounds read in riscv_probe_decode_insn 2026-11-15 12:37 ` Vladimir Kondratiev @ 2026-11-15 12:37 ` Vladimir Kondratiev -1 siblings, 0 replies; 9+ messages in thread From: Vladimir Kondratiev @ 2026-11-15 12:37 UTC (permalink / raw) To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Guo Ren, Pekka Enberg, Patrick Stählin Cc: linux-riscv, linux-kernel, linux-trace-kernel, Palmer Dabbelt, Vladimir Kondratiev, Sashiko riscv_probe_decode_insn() can trigger an out-of-bounds page fault when probing a 16-bit compressed instruction at the end of a page. arch_prepare_kprobe() safely reads 16-bit parcels into p->opcode, but then passes p->addr to riscv_probe_decode_insn(), which unconditionally dereferences *addr as a 32-bit probe_opcode_t If a 16-bit instruction sits in the last two bytes of a mapped page next to an unmapped page, the 32-bit read of *addr crosses the page boundary and faults. Passing &p->opcode instead of p->addr to riscv_probe_decode_insn() (as arch_uprobe_analyze_insn() already does) avoid reading past the instruction. Reported-by: Sashiko <sashiko-bot@kernel.org> Closes: https://sashiko.dev/#/bug/linux-812af091-20d1-4915-97a6-741f51e07022 Fixes: c22b0bcb1dd0 ("riscv: Add kprobes supported") Reviewed-by: GUO Ren (XuanTie) <guoren@kernel.org> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> --- arch/riscv/kernel/probes/kprobes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c index 9e2afabf94e8..63ee5066def5 100644 --- a/arch/riscv/kernel/probes/kprobes.c +++ b/arch/riscv/kernel/probes/kprobes.c @@ -85,7 +85,7 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p) p->opcode |= (kprobe_opcode_t)(*insn) << 16; /* decode instruction */ - switch (riscv_probe_decode_insn(p->addr, &p->ainsn.api)) { + switch (riscv_probe_decode_insn(&p->opcode, &p->ainsn.api)) { case INSN_REJECTED: /* insn not supported */ return -EINVAL; -- 2.43.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 1/3] riscv: kprobes: fix out-of-bounds read in riscv_probe_decode_insn @ 2026-11-15 12:37 ` Vladimir Kondratiev 0 siblings, 0 replies; 9+ messages in thread From: Vladimir Kondratiev @ 2026-11-15 12:37 UTC (permalink / raw) To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Guo Ren, Pekka Enberg, Patrick Stählin Cc: linux-riscv, linux-kernel, linux-trace-kernel, Palmer Dabbelt, Vladimir Kondratiev, Sashiko riscv_probe_decode_insn() can trigger an out-of-bounds page fault when probing a 16-bit compressed instruction at the end of a page. arch_prepare_kprobe() safely reads 16-bit parcels into p->opcode, but then passes p->addr to riscv_probe_decode_insn(), which unconditionally dereferences *addr as a 32-bit probe_opcode_t If a 16-bit instruction sits in the last two bytes of a mapped page next to an unmapped page, the 32-bit read of *addr crosses the page boundary and faults. Passing &p->opcode instead of p->addr to riscv_probe_decode_insn() (as arch_uprobe_analyze_insn() already does) avoid reading past the instruction. Reported-by: Sashiko <sashiko-bot@kernel.org> Closes: https://sashiko.dev/#/bug/linux-812af091-20d1-4915-97a6-741f51e07022 Fixes: c22b0bcb1dd0 ("riscv: Add kprobes supported") Reviewed-by: GUO Ren (XuanTie) <guoren@kernel.org> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> --- arch/riscv/kernel/probes/kprobes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c index 9e2afabf94e8..63ee5066def5 100644 --- a/arch/riscv/kernel/probes/kprobes.c +++ b/arch/riscv/kernel/probes/kprobes.c @@ -85,7 +85,7 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p) p->opcode |= (kprobe_opcode_t)(*insn) << 16; /* decode instruction */ - switch (riscv_probe_decode_insn(p->addr, &p->ainsn.api)) { + switch (riscv_probe_decode_insn(&p->opcode, &p->ainsn.api)) { case INSN_REJECTED: /* insn not supported */ return -EINVAL; -- 2.43.0 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 2/3] riscv: probes: reject probes inside an LR/SC sequence 2026-11-15 12:37 ` Vladimir Kondratiev @ 2026-11-15 12:37 ` Vladimir Kondratiev -1 siblings, 0 replies; 9+ messages in thread From: Vladimir Kondratiev @ 2026-11-15 12:37 UTC (permalink / raw) To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Guo Ren, Pekka Enberg, Patrick Stählin Cc: linux-riscv, linux-kernel, linux-trace-kernel, Palmer Dabbelt, Vladimir Kondratiev A kprobe or uprobe replaces an instruction with a breakpoint. The trap clears the hart's reservation, so if the breakpoint sits on the lr, on the sc, or on any instruction between them, the sc fails, the loop branches back to the lr and the probe fires again. The sequence never completes. This has always been possible on RISC-V, because cmpxchg and the conditional atomics are LR/SC loops. arm64 has the same problem and rejects probes inside an exclusive-load/store-exclusive region. On the other hand, if lr is not paired with sc, breakpoit at lr or after it is OK. lr/wrs sequence is an example - breakpoint can be set anywhere For the instructions between LR and SC, use the rules for a constrained LR/SC sequence: at most 16 instructions between lr and sc, all base integer instructions other than loads, stores, JALR, FENCE, SYSTEM and backward jumps or branches, or their compressed forms. Scan forward from the probed instruction. Reaching an sc through permitted instructions only means the probe sits inside a sequence; any other instruction, an lr, the end of the window or the end of the readable text means it does not. Only a sequence that follows these rules is guaranteed to make progress, so a probe inside any other one is already on unsupported ground. Scanning forward rather than tracking state from the start of the function also keeps an lr that has no sc, such as the one in __cmpwait() before wrs.nto, from affecting probes later in the same function. kprobes scan the kernel text up to the end of the enclosing symbol. uprobes have only the probed instruction copied, so they read the text that follows it from the task, which the caller's mmap_lock allows. Fixes: c22b0bcb1dd0 ("riscv: Add kprobes supported") Fixes: 74784081aac8 ("riscv: Add uprobes supported") Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> --- arch/riscv/include/asm/insn.h | 28 +++++++++++ arch/riscv/kernel/probes/decode-insn.c | 91 ++++++++++++++++++++++++++++++++++ arch/riscv/kernel/probes/decode-insn.h | 7 +++ arch/riscv/kernel/probes/kprobes.c | 26 +++++++--- arch/riscv/kernel/probes/uprobes.c | 31 ++++++++++++ 5 files changed, 177 insertions(+), 6 deletions(-) diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h index c3005573e8c9..f5de81753bec 100644 --- a/arch/riscv/include/asm/insn.h +++ b/arch/riscv/include/asm/insn.h @@ -10,11 +10,16 @@ #define RV_INSN_FUNCT3_MASK GENMASK(14, 12) #define RV_INSN_FUNCT3_OPOFF 12 +#define RV_INSN_FUNCT5_MASK GENMASK(31, 27) +#define RV_INSN_FUNCT5_OPOFF 27 +#define RV_INSN_FUNCT7_MASK GENMASK(31, 25) +#define RV_INSN_FUNCT7_OPOFF 25 #define RV_INSN_OPCODE_MASK GENMASK(6, 0) #define RV_INSN_OPCODE_OPOFF 0 #define RV_INSN_FUNCT12_OPOFF 20 #define RV_ENCODE_FUNCT3(f_) (RVG_FUNCT3_##f_ << RV_INSN_FUNCT3_OPOFF) +#define RV_ENCODE_FUNCT5(f_) (RVG_FUNCT5_##f_ << RV_INSN_FUNCT5_OPOFF) #define RV_ENCODE_FUNCT12(f_) (RVG_FUNCT12_##f_ << RV_INSN_FUNCT12_OPOFF) /* The bit field of immediate value in I-type instruction */ @@ -136,7 +141,13 @@ /* parts of opcode for RVG*/ #define RVG_OPCODE_FENCE 0x0f +#define RVG_OPCODE_OP_IMM 0x13 #define RVG_OPCODE_AUIPC 0x17 +#define RVG_OPCODE_OP_IMM_32 0x1b +#define RVG_OPCODE_AMO 0x2f +#define RVG_OPCODE_OP 0x33 +#define RVG_OPCODE_LUI 0x37 +#define RVG_OPCODE_OP_32 0x3b #define RVG_OPCODE_BRANCH 0x63 #define RVG_OPCODE_JALR 0x67 #define RVG_OPCODE_JAL 0x6f @@ -188,6 +199,17 @@ #define RVG_FUNCT12_EBREAK 0x1 #define RVG_FUNCT12_SRET 0x102 +/* parts of funct5 code for A extension */ +#define RVG_FUNCT5_LR 0x02 +#define RVG_FUNCT5_SC 0x03 + +/* + * The AMO funct3 field holds the access width: 010 for .w, 011 for .d. + * Matching bits 14:13 alone accepts both and nothing else. + */ +#define RVG_AMO_WIDTH_MASK GENMASK(14, 13) +#define RVG_AMO_WIDTH_WD BIT(13) + #define RVG_MATCH_AUIPC (RVG_OPCODE_AUIPC) #define RVG_MATCH_JALR (RV_ENCODE_FUNCT3(JALR) | RVG_OPCODE_JALR) #define RVG_MATCH_JAL (RVG_OPCODE_JAL) @@ -200,6 +222,8 @@ #define RVG_MATCH_BGEU (RV_ENCODE_FUNCT3(BGEU) | RVG_OPCODE_BRANCH) #define RVG_MATCH_EBREAK (RV_ENCODE_FUNCT12(EBREAK) | RVG_OPCODE_SYSTEM) #define RVG_MATCH_SRET (RV_ENCODE_FUNCT12(SRET) | RVG_OPCODE_SYSTEM) +#define RVG_MATCH_LR (RV_ENCODE_FUNCT5(LR) | RVG_AMO_WIDTH_WD | RVG_OPCODE_AMO) +#define RVG_MATCH_SC (RV_ENCODE_FUNCT5(SC) | RVG_AMO_WIDTH_WD | RVG_OPCODE_AMO) #define RVC_MATCH_C_BEQZ (RVC_ENCODE_FUNCT3(C_BEQZ) | RVC_OPCODE_C1) #define RVC_MATCH_C_BNEZ (RVC_ENCODE_FUNCT3(C_BNEZ) | RVC_OPCODE_C1) #define RVC_MATCH_C_J (RVC_ENCODE_FUNCT3(C_J) | RVC_OPCODE_C1) @@ -227,6 +251,8 @@ #define RVC_MASK_C_EBREAK 0xffff #define RVG_MASK_EBREAK 0xffffffff #define RVG_MASK_SRET 0xffffffff +#define RVG_MASK_LR (RV_INSN_FUNCT5_MASK | RVG_AMO_WIDTH_MASK | RV_INSN_OPCODE_MASK) +#define RVG_MASK_SC (RV_INSN_FUNCT5_MASK | RVG_AMO_WIDTH_MASK | RV_INSN_OPCODE_MASK) #define __INSN_LENGTH_MASK _UL(0x3) #define __INSN_LENGTH_GE_32 _UL(0x3) @@ -262,6 +288,8 @@ __RISCV_INSN_FUNCS(c_ebreak, RVC_MASK_C_EBREAK, RVC_MATCH_C_EBREAK) __RISCV_INSN_FUNCS(ebreak, RVG_MASK_EBREAK, RVG_MATCH_EBREAK) __RISCV_INSN_FUNCS(sret, RVG_MASK_SRET, RVG_MATCH_SRET) __RISCV_INSN_FUNCS(fence, RVG_MASK_FENCE, RVG_MATCH_FENCE); +__RISCV_INSN_FUNCS(lr, RVG_MASK_LR, RVG_MATCH_LR) +__RISCV_INSN_FUNCS(sc, RVG_MASK_SC, RVG_MATCH_SC) /* special case to catch _any_ system instruction */ static __always_inline bool riscv_insn_is_system(u32 code) diff --git a/arch/riscv/kernel/probes/decode-insn.c b/arch/riscv/kernel/probes/decode-insn.c index 433d9035b888..78a6f2ac6808 100644 --- a/arch/riscv/kernel/probes/decode-insn.c +++ b/arch/riscv/kernel/probes/decode-insn.c @@ -4,11 +4,102 @@ #include <linux/kprobes.h> #include <linux/module.h> #include <linux/kallsyms.h> +#include <linux/bug.h> #include <asm/sections.h> #include "decode-insn.h" #include "simulate-insn.h" +/* + * Can insn sit between the lr and the sc of a constrained LR/SC sequence? + * The ISA allows base integer instructions there, except loads, stores, + * JALR, FENCE, SYSTEM, backward jumps and taken backward branches, plus + * their compressed forms. Backward branches are treated as not allowed: + * the retry branch follows the sc, so one inside the body is not a + * constrained sequence. + */ +static bool __kprobes riscv_insn_is_lrsc_body(u32 insn) +{ + u32 funct3; + + if (GET_INSN_LENGTH(insn) == 2) { + switch (insn & RVC_INSN_OPCODE_MASK) { + case RVC_OPCODE_C0: + /* c.addi4spn; the rest of the quadrant is loads and stores */ + return (insn & RVC_INSN_FUNCT3_MASK) == 0; + case RVC_OPCODE_C1: + /* Arithmetic, c.j, c.jal and branches, bit 12 is the offset sign */ + if (riscv_insn_is_c_j(insn) || riscv_insn_is_c_jal(insn) || + riscv_insn_is_c_beqz(insn) || riscv_insn_is_c_bnez(insn)) + return !(insn & BIT(12)); + return true; + case RVC_OPCODE_C2: + funct3 = (insn & RVC_INSN_FUNCT3_MASK) >> RVC_INSN_FUNCT3_OPOFF; + if (funct3 == 0) /* c.slli */ + return true; + if (funct3 == 4) /* c.mv, c.add, but not c.jr, c.jalr, c.ebreak */ + return !riscv_insn_is_c_jr(insn) && + !riscv_insn_is_c_jalr(insn) && + !riscv_insn_is_c_ebreak(insn); + return false; /* stack loads and stores */ + } + return false; + } + + switch (insn & RV_INSN_OPCODE_MASK) { + case RVG_OPCODE_OP_IMM: + case RVG_OPCODE_OP_IMM_32: + case RVG_OPCODE_LUI: + case RVG_OPCODE_AUIPC: + return true; + case RVG_OPCODE_OP: + case RVG_OPCODE_OP_32: + /* funct7 0 or 0x20 is base I; anything else is M, B or another extension */ + return !((insn & RV_INSN_FUNCT7_MASK) & ~(0x20U << RV_INSN_FUNCT7_OPOFF)); + case RVG_OPCODE_BRANCH: + case RVG_OPCODE_JAL: + /* forward only, bit 31 is the offset sign */ + return !(insn & BIT(31)); + } + return false; +} + +/* + * Does the instruction at the start of buf lie inside a constrained LR/SC + * sequence? Such a sequence holds at most RISCV_LRSC_MAX_BODY instructions + * between the lr and the sc, all from the set above. Walk forward from the + * probed instruction: reaching an sc through permitted instructions only + * means the probe sits in the body; anything else, including an lr that + * opens a new sequence, means it does not. The walk stops at the end of + * buf, so a short buffer only makes the answer more permissive. + */ +bool __kprobes riscv_probe_in_lrsc_body(const u16 *buf, size_t len) +{ + u32 insn; + + /* 2 instructions for LR and SC itself */ + for (size_t n = 0, off = 0; n < RISCV_LRSC_MAX_BODY + 2 && off + 2 <= len; + n++, off += GET_INSN_LENGTH(insn)) { + insn = buf[off / 2]; + if (GET_INSN_LENGTH(insn) == 4) { + if (off + 4 > len) + return false; + insn |= (u32)buf[off / 2 + 1] << 16; + } + if (n == 0) { + if (riscv_insn_is_lr(insn)) + continue; /* 1-st instruction lr is OK */ + else + n++; /* if it's not lr, scan one instruction less */ + } + if (riscv_insn_is_sc(insn)) + return true; + if (!riscv_insn_is_lrsc_body(insn)) + return false; + } + return false; +} + /* Return: * INSN_REJECTED If instruction is one not allowed to kprobe, * INSN_GOOD_NO_SLOT If instruction is supported but doesn't use its slot. diff --git a/arch/riscv/kernel/probes/decode-insn.h b/arch/riscv/kernel/probes/decode-insn.h index 42269a7d676d..0ea952cc7595 100644 --- a/arch/riscv/kernel/probes/decode-insn.h +++ b/arch/riscv/kernel/probes/decode-insn.h @@ -15,4 +15,11 @@ enum probe_insn { enum probe_insn __kprobes riscv_probe_decode_insn(probe_opcode_t *addr, struct arch_probe_insn *asi); +/* A constrained LR/SC sequence has at most this many instructions between lr and sc */ +#define RISCV_LRSC_MAX_BODY 16 +/* Bytes from the probed instruction (can be lr) through the farthest possible sc */ +#define RISCV_LRSC_SCAN_BYTES ((RISCV_LRSC_MAX_BODY + 2) * 4) + +bool __kprobes riscv_probe_in_lrsc_body(const u16 *buf, size_t len); + #endif /* _RISCV_KERNEL_KPROBES_DECODE_INSN_H */ diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c index 63ee5066def5..b8df722ac19f 100644 --- a/arch/riscv/kernel/probes/kprobes.c +++ b/arch/riscv/kernel/probes/kprobes.c @@ -49,35 +49,49 @@ static void __kprobes arch_simulate_insn(struct kprobe *p, struct pt_regs *regs) post_kprobe_handler(p, kcb, regs); } -static bool __kprobes arch_check_kprobe(unsigned long addr) +/* + * Walk the instructions from the enclosing symbol up to addr to check that + * addr is an instruction boundary. Then reject a probe inside an LR/SC + * sequence: the breakpoint trap drops the reservation, so the SC would fail + * and the loop would hit the probe again on every retry. The sequence is + * recognised by a forward scan with the constrained-sequence rules, as for + * uprobes, so an lr that is not followed by an sc (as in __cmpwait()) does + * not taint the rest of the function. + */ +static int __kprobes arch_check_kprobe(unsigned long addr) { unsigned long tmp, offset; /* start iterating at the closest preceding symbol */ if (!kallsyms_lookup_size_offset(addr, NULL, &offset)) - return false; + return -EILSEQ; tmp = addr - offset; while (tmp <= addr) { if (tmp == addr) - return true; + if (riscv_probe_in_lrsc_body((u16 *)addr, RISCV_LRSC_SCAN_BYTES)) + return -EINVAL; + else + return 0; tmp += GET_INSN_LENGTH(*(u16 *)tmp); } - return false; + return -EILSEQ; } int __kprobes arch_prepare_kprobe(struct kprobe *p) { u16 *insn = (u16 *)p->addr; + int ret; if ((unsigned long)insn & 0x1) return -EILSEQ; - if (!arch_check_kprobe((unsigned long)p->addr)) - return -EILSEQ; + ret = arch_check_kprobe((unsigned long)p->addr); + if (ret) + return ret; /* copy instruction */ p->opcode = (kprobe_opcode_t)(*insn++); diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c index eb177d0ce8ab..d97a10262166 100644 --- a/arch/riscv/kernel/probes/uprobes.c +++ b/arch/riscv/kernel/probes/uprobes.c @@ -28,6 +28,34 @@ unsigned long uprobe_get_swbp_addr(struct pt_regs *regs) return instruction_pointer(regs); } +/* + * Is the instruction at addr inside a constrained LR/SC sequence? Only the + * probed instruction was copied, so read the text that follows it from the + * task. The caller holds mmap_lock, as uprobe_write_opcode() requires too. + * Text that cannot be read ends the scan early, which accepts the probe. + */ +static bool uprobe_in_lrsc_body(struct mm_struct *mm, unsigned long addr) +{ + u16 buf[RISCV_LRSC_SCAN_BYTES / sizeof(u16)]; + struct vm_area_struct *vma; + struct page *page; + size_t got = 0; + int len; + + while (got < sizeof(buf)) { + len = min_t(size_t, sizeof(buf) - got, + PAGE_SIZE - offset_in_page(addr + got)); + page = get_user_page_vma_remote(mm, addr + got, FOLL_FORCE, &vma); + if (IS_ERR(page)) + break; + uprobe_copy_from_page(page, addr + got, (void *)buf + got, len); + put_page(page); + got += len; + } + + return riscv_probe_in_lrsc_body(buf, got); +} + int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, unsigned long addr) { @@ -53,6 +81,9 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, return -EINVAL; } + if (uprobe_in_lrsc_body(mm, addr)) + return -EINVAL; + return 0; } -- 2.43.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 2/3] riscv: probes: reject probes inside an LR/SC sequence @ 2026-11-15 12:37 ` Vladimir Kondratiev 0 siblings, 0 replies; 9+ messages in thread From: Vladimir Kondratiev @ 2026-11-15 12:37 UTC (permalink / raw) To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Guo Ren, Pekka Enberg, Patrick Stählin Cc: linux-riscv, linux-kernel, linux-trace-kernel, Palmer Dabbelt, Vladimir Kondratiev A kprobe or uprobe replaces an instruction with a breakpoint. The trap clears the hart's reservation, so if the breakpoint sits on the lr, on the sc, or on any instruction between them, the sc fails, the loop branches back to the lr and the probe fires again. The sequence never completes. This has always been possible on RISC-V, because cmpxchg and the conditional atomics are LR/SC loops. arm64 has the same problem and rejects probes inside an exclusive-load/store-exclusive region. On the other hand, if lr is not paired with sc, breakpoit at lr or after it is OK. lr/wrs sequence is an example - breakpoint can be set anywhere For the instructions between LR and SC, use the rules for a constrained LR/SC sequence: at most 16 instructions between lr and sc, all base integer instructions other than loads, stores, JALR, FENCE, SYSTEM and backward jumps or branches, or their compressed forms. Scan forward from the probed instruction. Reaching an sc through permitted instructions only means the probe sits inside a sequence; any other instruction, an lr, the end of the window or the end of the readable text means it does not. Only a sequence that follows these rules is guaranteed to make progress, so a probe inside any other one is already on unsupported ground. Scanning forward rather than tracking state from the start of the function also keeps an lr that has no sc, such as the one in __cmpwait() before wrs.nto, from affecting probes later in the same function. kprobes scan the kernel text up to the end of the enclosing symbol. uprobes have only the probed instruction copied, so they read the text that follows it from the task, which the caller's mmap_lock allows. Fixes: c22b0bcb1dd0 ("riscv: Add kprobes supported") Fixes: 74784081aac8 ("riscv: Add uprobes supported") Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> --- arch/riscv/include/asm/insn.h | 28 +++++++++++ arch/riscv/kernel/probes/decode-insn.c | 91 ++++++++++++++++++++++++++++++++++ arch/riscv/kernel/probes/decode-insn.h | 7 +++ arch/riscv/kernel/probes/kprobes.c | 26 +++++++--- arch/riscv/kernel/probes/uprobes.c | 31 ++++++++++++ 5 files changed, 177 insertions(+), 6 deletions(-) diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h index c3005573e8c9..f5de81753bec 100644 --- a/arch/riscv/include/asm/insn.h +++ b/arch/riscv/include/asm/insn.h @@ -10,11 +10,16 @@ #define RV_INSN_FUNCT3_MASK GENMASK(14, 12) #define RV_INSN_FUNCT3_OPOFF 12 +#define RV_INSN_FUNCT5_MASK GENMASK(31, 27) +#define RV_INSN_FUNCT5_OPOFF 27 +#define RV_INSN_FUNCT7_MASK GENMASK(31, 25) +#define RV_INSN_FUNCT7_OPOFF 25 #define RV_INSN_OPCODE_MASK GENMASK(6, 0) #define RV_INSN_OPCODE_OPOFF 0 #define RV_INSN_FUNCT12_OPOFF 20 #define RV_ENCODE_FUNCT3(f_) (RVG_FUNCT3_##f_ << RV_INSN_FUNCT3_OPOFF) +#define RV_ENCODE_FUNCT5(f_) (RVG_FUNCT5_##f_ << RV_INSN_FUNCT5_OPOFF) #define RV_ENCODE_FUNCT12(f_) (RVG_FUNCT12_##f_ << RV_INSN_FUNCT12_OPOFF) /* The bit field of immediate value in I-type instruction */ @@ -136,7 +141,13 @@ /* parts of opcode for RVG*/ #define RVG_OPCODE_FENCE 0x0f +#define RVG_OPCODE_OP_IMM 0x13 #define RVG_OPCODE_AUIPC 0x17 +#define RVG_OPCODE_OP_IMM_32 0x1b +#define RVG_OPCODE_AMO 0x2f +#define RVG_OPCODE_OP 0x33 +#define RVG_OPCODE_LUI 0x37 +#define RVG_OPCODE_OP_32 0x3b #define RVG_OPCODE_BRANCH 0x63 #define RVG_OPCODE_JALR 0x67 #define RVG_OPCODE_JAL 0x6f @@ -188,6 +199,17 @@ #define RVG_FUNCT12_EBREAK 0x1 #define RVG_FUNCT12_SRET 0x102 +/* parts of funct5 code for A extension */ +#define RVG_FUNCT5_LR 0x02 +#define RVG_FUNCT5_SC 0x03 + +/* + * The AMO funct3 field holds the access width: 010 for .w, 011 for .d. + * Matching bits 14:13 alone accepts both and nothing else. + */ +#define RVG_AMO_WIDTH_MASK GENMASK(14, 13) +#define RVG_AMO_WIDTH_WD BIT(13) + #define RVG_MATCH_AUIPC (RVG_OPCODE_AUIPC) #define RVG_MATCH_JALR (RV_ENCODE_FUNCT3(JALR) | RVG_OPCODE_JALR) #define RVG_MATCH_JAL (RVG_OPCODE_JAL) @@ -200,6 +222,8 @@ #define RVG_MATCH_BGEU (RV_ENCODE_FUNCT3(BGEU) | RVG_OPCODE_BRANCH) #define RVG_MATCH_EBREAK (RV_ENCODE_FUNCT12(EBREAK) | RVG_OPCODE_SYSTEM) #define RVG_MATCH_SRET (RV_ENCODE_FUNCT12(SRET) | RVG_OPCODE_SYSTEM) +#define RVG_MATCH_LR (RV_ENCODE_FUNCT5(LR) | RVG_AMO_WIDTH_WD | RVG_OPCODE_AMO) +#define RVG_MATCH_SC (RV_ENCODE_FUNCT5(SC) | RVG_AMO_WIDTH_WD | RVG_OPCODE_AMO) #define RVC_MATCH_C_BEQZ (RVC_ENCODE_FUNCT3(C_BEQZ) | RVC_OPCODE_C1) #define RVC_MATCH_C_BNEZ (RVC_ENCODE_FUNCT3(C_BNEZ) | RVC_OPCODE_C1) #define RVC_MATCH_C_J (RVC_ENCODE_FUNCT3(C_J) | RVC_OPCODE_C1) @@ -227,6 +251,8 @@ #define RVC_MASK_C_EBREAK 0xffff #define RVG_MASK_EBREAK 0xffffffff #define RVG_MASK_SRET 0xffffffff +#define RVG_MASK_LR (RV_INSN_FUNCT5_MASK | RVG_AMO_WIDTH_MASK | RV_INSN_OPCODE_MASK) +#define RVG_MASK_SC (RV_INSN_FUNCT5_MASK | RVG_AMO_WIDTH_MASK | RV_INSN_OPCODE_MASK) #define __INSN_LENGTH_MASK _UL(0x3) #define __INSN_LENGTH_GE_32 _UL(0x3) @@ -262,6 +288,8 @@ __RISCV_INSN_FUNCS(c_ebreak, RVC_MASK_C_EBREAK, RVC_MATCH_C_EBREAK) __RISCV_INSN_FUNCS(ebreak, RVG_MASK_EBREAK, RVG_MATCH_EBREAK) __RISCV_INSN_FUNCS(sret, RVG_MASK_SRET, RVG_MATCH_SRET) __RISCV_INSN_FUNCS(fence, RVG_MASK_FENCE, RVG_MATCH_FENCE); +__RISCV_INSN_FUNCS(lr, RVG_MASK_LR, RVG_MATCH_LR) +__RISCV_INSN_FUNCS(sc, RVG_MASK_SC, RVG_MATCH_SC) /* special case to catch _any_ system instruction */ static __always_inline bool riscv_insn_is_system(u32 code) diff --git a/arch/riscv/kernel/probes/decode-insn.c b/arch/riscv/kernel/probes/decode-insn.c index 433d9035b888..78a6f2ac6808 100644 --- a/arch/riscv/kernel/probes/decode-insn.c +++ b/arch/riscv/kernel/probes/decode-insn.c @@ -4,11 +4,102 @@ #include <linux/kprobes.h> #include <linux/module.h> #include <linux/kallsyms.h> +#include <linux/bug.h> #include <asm/sections.h> #include "decode-insn.h" #include "simulate-insn.h" +/* + * Can insn sit between the lr and the sc of a constrained LR/SC sequence? + * The ISA allows base integer instructions there, except loads, stores, + * JALR, FENCE, SYSTEM, backward jumps and taken backward branches, plus + * their compressed forms. Backward branches are treated as not allowed: + * the retry branch follows the sc, so one inside the body is not a + * constrained sequence. + */ +static bool __kprobes riscv_insn_is_lrsc_body(u32 insn) +{ + u32 funct3; + + if (GET_INSN_LENGTH(insn) == 2) { + switch (insn & RVC_INSN_OPCODE_MASK) { + case RVC_OPCODE_C0: + /* c.addi4spn; the rest of the quadrant is loads and stores */ + return (insn & RVC_INSN_FUNCT3_MASK) == 0; + case RVC_OPCODE_C1: + /* Arithmetic, c.j, c.jal and branches, bit 12 is the offset sign */ + if (riscv_insn_is_c_j(insn) || riscv_insn_is_c_jal(insn) || + riscv_insn_is_c_beqz(insn) || riscv_insn_is_c_bnez(insn)) + return !(insn & BIT(12)); + return true; + case RVC_OPCODE_C2: + funct3 = (insn & RVC_INSN_FUNCT3_MASK) >> RVC_INSN_FUNCT3_OPOFF; + if (funct3 == 0) /* c.slli */ + return true; + if (funct3 == 4) /* c.mv, c.add, but not c.jr, c.jalr, c.ebreak */ + return !riscv_insn_is_c_jr(insn) && + !riscv_insn_is_c_jalr(insn) && + !riscv_insn_is_c_ebreak(insn); + return false; /* stack loads and stores */ + } + return false; + } + + switch (insn & RV_INSN_OPCODE_MASK) { + case RVG_OPCODE_OP_IMM: + case RVG_OPCODE_OP_IMM_32: + case RVG_OPCODE_LUI: + case RVG_OPCODE_AUIPC: + return true; + case RVG_OPCODE_OP: + case RVG_OPCODE_OP_32: + /* funct7 0 or 0x20 is base I; anything else is M, B or another extension */ + return !((insn & RV_INSN_FUNCT7_MASK) & ~(0x20U << RV_INSN_FUNCT7_OPOFF)); + case RVG_OPCODE_BRANCH: + case RVG_OPCODE_JAL: + /* forward only, bit 31 is the offset sign */ + return !(insn & BIT(31)); + } + return false; +} + +/* + * Does the instruction at the start of buf lie inside a constrained LR/SC + * sequence? Such a sequence holds at most RISCV_LRSC_MAX_BODY instructions + * between the lr and the sc, all from the set above. Walk forward from the + * probed instruction: reaching an sc through permitted instructions only + * means the probe sits in the body; anything else, including an lr that + * opens a new sequence, means it does not. The walk stops at the end of + * buf, so a short buffer only makes the answer more permissive. + */ +bool __kprobes riscv_probe_in_lrsc_body(const u16 *buf, size_t len) +{ + u32 insn; + + /* 2 instructions for LR and SC itself */ + for (size_t n = 0, off = 0; n < RISCV_LRSC_MAX_BODY + 2 && off + 2 <= len; + n++, off += GET_INSN_LENGTH(insn)) { + insn = buf[off / 2]; + if (GET_INSN_LENGTH(insn) == 4) { + if (off + 4 > len) + return false; + insn |= (u32)buf[off / 2 + 1] << 16; + } + if (n == 0) { + if (riscv_insn_is_lr(insn)) + continue; /* 1-st instruction lr is OK */ + else + n++; /* if it's not lr, scan one instruction less */ + } + if (riscv_insn_is_sc(insn)) + return true; + if (!riscv_insn_is_lrsc_body(insn)) + return false; + } + return false; +} + /* Return: * INSN_REJECTED If instruction is one not allowed to kprobe, * INSN_GOOD_NO_SLOT If instruction is supported but doesn't use its slot. diff --git a/arch/riscv/kernel/probes/decode-insn.h b/arch/riscv/kernel/probes/decode-insn.h index 42269a7d676d..0ea952cc7595 100644 --- a/arch/riscv/kernel/probes/decode-insn.h +++ b/arch/riscv/kernel/probes/decode-insn.h @@ -15,4 +15,11 @@ enum probe_insn { enum probe_insn __kprobes riscv_probe_decode_insn(probe_opcode_t *addr, struct arch_probe_insn *asi); +/* A constrained LR/SC sequence has at most this many instructions between lr and sc */ +#define RISCV_LRSC_MAX_BODY 16 +/* Bytes from the probed instruction (can be lr) through the farthest possible sc */ +#define RISCV_LRSC_SCAN_BYTES ((RISCV_LRSC_MAX_BODY + 2) * 4) + +bool __kprobes riscv_probe_in_lrsc_body(const u16 *buf, size_t len); + #endif /* _RISCV_KERNEL_KPROBES_DECODE_INSN_H */ diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c index 63ee5066def5..b8df722ac19f 100644 --- a/arch/riscv/kernel/probes/kprobes.c +++ b/arch/riscv/kernel/probes/kprobes.c @@ -49,35 +49,49 @@ static void __kprobes arch_simulate_insn(struct kprobe *p, struct pt_regs *regs) post_kprobe_handler(p, kcb, regs); } -static bool __kprobes arch_check_kprobe(unsigned long addr) +/* + * Walk the instructions from the enclosing symbol up to addr to check that + * addr is an instruction boundary. Then reject a probe inside an LR/SC + * sequence: the breakpoint trap drops the reservation, so the SC would fail + * and the loop would hit the probe again on every retry. The sequence is + * recognised by a forward scan with the constrained-sequence rules, as for + * uprobes, so an lr that is not followed by an sc (as in __cmpwait()) does + * not taint the rest of the function. + */ +static int __kprobes arch_check_kprobe(unsigned long addr) { unsigned long tmp, offset; /* start iterating at the closest preceding symbol */ if (!kallsyms_lookup_size_offset(addr, NULL, &offset)) - return false; + return -EILSEQ; tmp = addr - offset; while (tmp <= addr) { if (tmp == addr) - return true; + if (riscv_probe_in_lrsc_body((u16 *)addr, RISCV_LRSC_SCAN_BYTES)) + return -EINVAL; + else + return 0; tmp += GET_INSN_LENGTH(*(u16 *)tmp); } - return false; + return -EILSEQ; } int __kprobes arch_prepare_kprobe(struct kprobe *p) { u16 *insn = (u16 *)p->addr; + int ret; if ((unsigned long)insn & 0x1) return -EILSEQ; - if (!arch_check_kprobe((unsigned long)p->addr)) - return -EILSEQ; + ret = arch_check_kprobe((unsigned long)p->addr); + if (ret) + return ret; /* copy instruction */ p->opcode = (kprobe_opcode_t)(*insn++); diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c index eb177d0ce8ab..d97a10262166 100644 --- a/arch/riscv/kernel/probes/uprobes.c +++ b/arch/riscv/kernel/probes/uprobes.c @@ -28,6 +28,34 @@ unsigned long uprobe_get_swbp_addr(struct pt_regs *regs) return instruction_pointer(regs); } +/* + * Is the instruction at addr inside a constrained LR/SC sequence? Only the + * probed instruction was copied, so read the text that follows it from the + * task. The caller holds mmap_lock, as uprobe_write_opcode() requires too. + * Text that cannot be read ends the scan early, which accepts the probe. + */ +static bool uprobe_in_lrsc_body(struct mm_struct *mm, unsigned long addr) +{ + u16 buf[RISCV_LRSC_SCAN_BYTES / sizeof(u16)]; + struct vm_area_struct *vma; + struct page *page; + size_t got = 0; + int len; + + while (got < sizeof(buf)) { + len = min_t(size_t, sizeof(buf) - got, + PAGE_SIZE - offset_in_page(addr + got)); + page = get_user_page_vma_remote(mm, addr + got, FOLL_FORCE, &vma); + if (IS_ERR(page)) + break; + uprobe_copy_from_page(page, addr + got, (void *)buf + got, len); + put_page(page); + got += len; + } + + return riscv_probe_in_lrsc_body(buf, got); +} + int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, unsigned long addr) { @@ -53,6 +81,9 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, return -EINVAL; } + if (uprobe_in_lrsc_body(mm, addr)) + return -EINVAL; + return 0; } -- 2.43.0 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4 2/3] riscv: probes: reject probes inside an LR/SC sequence 2026-11-15 12:37 ` Vladimir Kondratiev (?) @ 2026-11-15 13:01 ` sashiko-bot -1 siblings, 0 replies; 9+ messages in thread From: sashiko-bot @ 2026-11-15 13:01 UTC (permalink / raw) To: Vladimir Kondratiev; +Cc: linux-trace-kernel Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [Medium] riscv: probes: unbounded symbol scan and 0x0000 body match in LR/SC check - [Medium] riscv: probes: uprobe_in_lrsc_body() accepts probe when first page read fails -- commit adb0f0de869819463b6310d8616d49420ae15ca8 Author: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> riscv: probes: reject probes inside an LR/SC sequence This commit rejects kprobes and uprobes placed inside a constrained LR/SC atomic sequence by scanning forward up to 16 allowed body instructions for a matching SC instruction. > diff --git a/arch/riscv/kernel/probes/decode-insn.c b/arch/riscv/kernel/probes/decode-insn.c > index 433d9035b8882..78a6f2ac6808c 100644 > --- a/arch/riscv/kernel/probes/decode-insn.c > +++ b/arch/riscv/kernel/probes/decode-insn.c > @@ -4,11 +4,102 @@ > #include <linux/kprobes.h> > #include <linux/module.h> > #include <linux/kallsyms.h> > +#include <linux/bug.h> > #include <asm/sections.h> > > #include "decode-insn.h" > #include "simulate-insn.h" > > +/* > + * Can insn sit between the lr and the sc of a constrained LR/SC sequence? > + * The ISA allows base integer instructions there, except loads, stores, > + * JALR, FENCE, SYSTEM, backward jumps and taken backward branches, plus > + * their compressed forms. Backward branches are treated as not allowed: > + * the retry branch follows the sc, so one inside the body is not a > + * constrained sequence. > + */ > +static bool __kprobes riscv_insn_is_lrsc_body(u32 insn) > +{ > + u32 funct3; > + > + if (GET_INSN_LENGTH(insn) == 2) { > + switch (insn & RVC_INSN_OPCODE_MASK) { > + case RVC_OPCODE_C0: > + /* c.addi4spn; the rest of the quadrant is loads and stores */ > + return (insn & RVC_INSN_FUNCT3_MASK) == 0; [ ... ] > diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c > index 63ee5066def5c..b8df722ac19f7 100644 > --- a/arch/riscv/kernel/probes/kprobes.c > +++ b/arch/riscv/kernel/probes/kprobes.c > @@ -49,35 +49,49 @@ static void __kprobes arch_simulate_insn(struct kprobe *p, struct pt_regs *regs) > post_kprobe_handler(p, kcb, regs); > } > > -static bool __kprobes arch_check_kprobe(unsigned long addr) > +/* > + * Walk the instructions from the enclosing symbol up to addr to check that > + * addr is an instruction boundary. Then reject a probe inside an LR/SC > + * sequence: the breakpoint trap drops the reservation, so the SC would fail > + * and the loop would hit the probe again on every retry. The sequence is > + * recognised by a forward scan with the constrained-sequence rules, as for > + * uprobes, so an lr that is not followed by an sc (as in __cmpwait()) does > + * not taint the rest of the function. > + */ > +static int __kprobes arch_check_kprobe(unsigned long addr) > { > unsigned long tmp, offset; > > /* start iterating at the closest preceding symbol */ > if (!kallsyms_lookup_size_offset(addr, NULL, &offset)) > - return false; > + return -EILSEQ; > > tmp = addr - offset; > > while (tmp <= addr) { > if (tmp == addr) > - return true; > + if (riscv_probe_in_lrsc_body((u16 *)addr, RISCV_LRSC_SCAN_BYTES)) > + return -EINVAL; > + else > + return 0; [Severity: Medium] Can arch_check_kprobe() scan past the end of the enclosing symbol and into an unmapped page? The commit message states that kprobes scan up to the end of the enclosing symbol, but arch_check_kprobe() passes NULL for symbolsize to kallsyms_lookup_size_offset() and always scans RISCV_LRSC_SCAN_BYTES (72 bytes) from addr. In addition, the RVC_OPCODE_C0 check in riscv_insn_is_lrsc_body() only tests funct3 == 0 without requiring a non-zero immediate in bits 12:5, so 0x0000 (illegal instruction and zero padding at the end of a module text section) is treated as a valid c.addi4spn body instruction. Probing near the end of a function can therefore scan across symbol boundaries or through trailing zero padding into an unmapped module vmalloc guard page, causing an unhandled kernel page fault while holding kprobe_mutex and text_mutex. Clamping the scan length to symbolsize - offset and rejecting nzuimm == 0 for c.addi4spn would prevent scanning past the symbol or across zero padding. [ ... ] > diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c > index eb177d0ce8ab0..d97a102621665 100644 > --- a/arch/riscv/kernel/probes/uprobes.c > +++ b/arch/riscv/kernel/probes/uprobes.c > @@ -28,6 +28,34 @@ unsigned long uprobe_get_swbp_addr(struct pt_regs *regs) > return instruction_pointer(regs); > } > > +/* > + * Is the instruction at addr inside a constrained LR/SC sequence? Only the > + * probed instruction was copied, so read the text that follows it from the > + * task. The caller holds mmap_lock, as uprobe_write_opcode() requires too. > + * Text that cannot be read ends the scan early, which accepts the probe. > + */ > +static bool uprobe_in_lrsc_body(struct mm_struct *mm, unsigned long addr) > +{ > + u16 buf[RISCV_LRSC_SCAN_BYTES / sizeof(u16)]; > + struct vm_area_struct *vma; > + struct page *page; > + size_t got = 0; > + int len; > + > + while (got < sizeof(buf)) { > + len = min_t(size_t, sizeof(buf) - got, > + PAGE_SIZE - offset_in_page(addr + got)); > + page = get_user_page_vma_remote(mm, addr + got, FOLL_FORCE, &vma); > + if (IS_ERR(page)) > + break; > + uprobe_copy_from_page(page, addr + got, (void *)buf + got, len); > + put_page(page); > + got += len; > + } > + > + return riscv_probe_in_lrsc_body(buf, got); > +} > + > int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, > unsigned long addr) > { > @@ -53,6 +81,9 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, > return -EINVAL; > } > > + if (uprobe_in_lrsc_body(mm, addr)) > + return -EINVAL; > + > return 0; > } [Severity: Medium] Does uprobe_in_lrsc_body() accept a probe on an sc instruction or inside an LR/SC sequence if get_user_page_vma_remote() fails on the first page? When the first get_user_page_vma_remote() call fails (for example due to a pending fatal signal or -ENOMEM), the loop breaks with got == 0 and riscv_probe_in_lrsc_body(buf, 0) returns false without even checking the probed instruction already copied in auprobe->insn. arch_uprobe_analyze_insn() then returns 0, so prepare_uprobe() sets UPROBE_COPY_INSN on the uprobe permanently; subsequent mappings or tasks skip prepare_uprobe() and install the breakpoint inside the LR/SC sequence, livelocking the target task when hit. -- Sashiko AI review · https://sashiko.dev/#/patchset/20261115-probes-in-lrsc-v4-0-48b9b036bd1d@mobileye.com?part=2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 3/3] riscv: kprobes: add KUnit test for LR/SC sequence rejection 2026-11-15 12:37 ` Vladimir Kondratiev @ 2026-11-15 12:37 ` Vladimir Kondratiev -1 siblings, 0 replies; 9+ messages in thread From: Vladimir Kondratiev @ 2026-11-15 12:37 UTC (permalink / raw) To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Guo Ren, Pekka Enberg, Patrick Stählin Cc: linux-riscv, linux-kernel, linux-trace-kernel, Palmer Dabbelt, Vladimir Kondratiev, Xiaofeng Yuan Extend the kprobes KUnit test with a function containing an LR/SC sequence and assert that register_kprobe() accepts a probe placed right before or right after the sequence, and rejects a probe placed on the LR, on an instruction inside the sequence, or on the SC. Add never-executed function that imitates s valid LR/SC section, one with short and one with maximum valis size body, and section with LR and no SC, mimicking __cmpwait where LR is followed by the conditional branch. Suggested-by: Xiaofeng Yuan <yuanxiaofeng@eswincomputing.com> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> --- arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S | 58 ++++++++++++++++++++++ arch/riscv/kernel/tests/kprobes/test-kprobes.c | 32 ++++++++++++ arch/riscv/kernel/tests/kprobes/test-kprobes.h | 6 +++ 3 files changed, 96 insertions(+) diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S b/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S index 15e50934bbd7..ca5b9223f885 100644 --- a/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S +++ b/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S @@ -198,6 +198,44 @@ SYM_FUNC_END(test_kprobes_c_jal) #endif /* CONFIG_RISCV_ISA_C */ +SYM_FUNC_START(test_kprobes_lrsc) +test_kprobes_lrsc_addr0: + nop + /* valid LR/SC sequence */ +1: +test_kprobes_lrsc_addr1: /* expect: rejected */ + lr.w t0, 0(a0) +test_kprobes_lrsc_addr2: /* expect: rejected */ + addi t0, t0, 1 +test_kprobes_lrsc_addr3: /* expect: rejected */ + sc.w t0, t0, 0(a0) +test_kprobes_lrsc_addr4: + bnez t0, 1b + + /* not a valid LR/SC sequence - no SC */ +test_kprobes_lrsc_addr5: + lr.w t0, 0(a0) +test_kprobes_lrsc_addr6: + and t0, t0, a1 + xor t0, t0, a2 + bnez t0, 1f +test_kprobes_lrsc_addr7: + nop +1: + nop + + /* valid LR/SC sequence */ +test_kprobes_lrsc_addr8: /* expect: rejected */ + lr.w t0, 0(a0) +test_kprobes_lrsc_addr9: /* expect: rejected */ + .rept 16 + nop + .endr + sc.w t0, t0, 0(a0) + nop + ret +SYM_FUNC_END(test_kprobes_lrsc) + .section .rodata SYM_DATA_START(test_kprobes_addresses) RISCV_PTR test_kprobes_add_addr1 @@ -253,3 +291,23 @@ SYM_DATA_START(test_kprobes_functions) #endif /* CONFIG_RISCV_ISA_C */ RISCV_PTR 0 SYM_DATA_END(test_kprobes_functions) + +.section .rodata +SYM_DATA_START(test_kprobes_lrsc_allowed_addresses) + RISCV_PTR test_kprobes_lrsc_addr0 + RISCV_PTR test_kprobes_lrsc_addr4 + RISCV_PTR test_kprobes_lrsc_addr5 + RISCV_PTR test_kprobes_lrsc_addr6 + RISCV_PTR test_kprobes_lrsc_addr7 + RISCV_PTR 0 +SYM_DATA_END(test_kprobes_lrsc_allowed_addresses) + +.section .rodata +SYM_DATA_START(test_kprobes_lrsc_rejected_addresses) + RISCV_PTR test_kprobes_lrsc_addr1 + RISCV_PTR test_kprobes_lrsc_addr2 + RISCV_PTR test_kprobes_lrsc_addr3 + RISCV_PTR test_kprobes_lrsc_addr8 + RISCV_PTR test_kprobes_lrsc_addr9 + RISCV_PTR 0 +SYM_DATA_END(test_kprobes_lrsc_rejected_addresses) diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes.c b/arch/riscv/kernel/tests/kprobes/test-kprobes.c index 027424a3ff7b..ff9d5bb2b998 100644 --- a/arch/riscv/kernel/tests/kprobes/test-kprobes.c +++ b/arch/riscv/kernel/tests/kprobes/test-kprobes.c @@ -43,8 +43,40 @@ static void test_kprobe_riscv(struct kunit *test) kfree(kp); } +static void test_kprobe_riscv_lrsc(struct kunit *test) +{ + /* probes outside the LR/SC sequence must be accepted */ + for (int i = 0; test_kprobes_lrsc_allowed_addresses[i]; i++) { + struct kprobe kp = {}; + int ret; + + kp.pre_handler = kprobe_dummy_handler; + kp.addr = test_kprobes_lrsc_allowed_addresses[i]; + + ret = register_kprobe(&kp); + KUNIT_EXPECT_EQ_MSG(test, ret, 0, "location %pS rejected", kp.addr); + if (!ret) + unregister_kprobe(&kp); + } + + /* probes on or inside the LR/SC sequence must be rejected */ + for (int i = 0; test_kprobes_lrsc_rejected_addresses[i]; i++) { + struct kprobe kp = {}; + int ret; + + kp.pre_handler = kprobe_dummy_handler; + kp.addr = test_kprobes_lrsc_rejected_addresses[i]; + + ret = register_kprobe(&kp); + KUNIT_EXPECT_LT_MSG(test, ret, 0, "location %pS accepted", kp.addr); + if (!ret) + unregister_kprobe(&kp); + } +} + static struct kunit_case kprobes_testcases[] = { KUNIT_CASE(test_kprobe_riscv), + KUNIT_CASE(test_kprobe_riscv_lrsc), {} }; diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes.h b/arch/riscv/kernel/tests/kprobes/test-kprobes.h index 537f44aa9d3f..93393e77a2cf 100644 --- a/arch/riscv/kernel/tests/kprobes/test-kprobes.h +++ b/arch/riscv/kernel/tests/kprobes/test-kprobes.h @@ -19,6 +19,12 @@ extern void *test_kprobes_addresses[]; /* array of functions that return KPROBE_TEST_MAGIC */ extern long (*test_kprobes_functions[])(void); +/* probe points that must be accepted, terminated by NULL */ +extern void *test_kprobes_lrsc_allowed_addresses[]; + +/* probe points that must be rejected, terminated by NULL */ +extern void *test_kprobes_lrsc_rejected_addresses[]; + #endif /* __ASSEMBLER__ */ #endif /* TEST_KPROBES_H */ -- 2.43.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 3/3] riscv: kprobes: add KUnit test for LR/SC sequence rejection @ 2026-11-15 12:37 ` Vladimir Kondratiev 0 siblings, 0 replies; 9+ messages in thread From: Vladimir Kondratiev @ 2026-11-15 12:37 UTC (permalink / raw) To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Guo Ren, Pekka Enberg, Patrick Stählin Cc: linux-riscv, linux-kernel, linux-trace-kernel, Palmer Dabbelt, Vladimir Kondratiev, Xiaofeng Yuan Extend the kprobes KUnit test with a function containing an LR/SC sequence and assert that register_kprobe() accepts a probe placed right before or right after the sequence, and rejects a probe placed on the LR, on an instruction inside the sequence, or on the SC. Add never-executed function that imitates s valid LR/SC section, one with short and one with maximum valis size body, and section with LR and no SC, mimicking __cmpwait where LR is followed by the conditional branch. Suggested-by: Xiaofeng Yuan <yuanxiaofeng@eswincomputing.com> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> --- arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S | 58 ++++++++++++++++++++++ arch/riscv/kernel/tests/kprobes/test-kprobes.c | 32 ++++++++++++ arch/riscv/kernel/tests/kprobes/test-kprobes.h | 6 +++ 3 files changed, 96 insertions(+) diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S b/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S index 15e50934bbd7..ca5b9223f885 100644 --- a/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S +++ b/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S @@ -198,6 +198,44 @@ SYM_FUNC_END(test_kprobes_c_jal) #endif /* CONFIG_RISCV_ISA_C */ +SYM_FUNC_START(test_kprobes_lrsc) +test_kprobes_lrsc_addr0: + nop + /* valid LR/SC sequence */ +1: +test_kprobes_lrsc_addr1: /* expect: rejected */ + lr.w t0, 0(a0) +test_kprobes_lrsc_addr2: /* expect: rejected */ + addi t0, t0, 1 +test_kprobes_lrsc_addr3: /* expect: rejected */ + sc.w t0, t0, 0(a0) +test_kprobes_lrsc_addr4: + bnez t0, 1b + + /* not a valid LR/SC sequence - no SC */ +test_kprobes_lrsc_addr5: + lr.w t0, 0(a0) +test_kprobes_lrsc_addr6: + and t0, t0, a1 + xor t0, t0, a2 + bnez t0, 1f +test_kprobes_lrsc_addr7: + nop +1: + nop + + /* valid LR/SC sequence */ +test_kprobes_lrsc_addr8: /* expect: rejected */ + lr.w t0, 0(a0) +test_kprobes_lrsc_addr9: /* expect: rejected */ + .rept 16 + nop + .endr + sc.w t0, t0, 0(a0) + nop + ret +SYM_FUNC_END(test_kprobes_lrsc) + .section .rodata SYM_DATA_START(test_kprobes_addresses) RISCV_PTR test_kprobes_add_addr1 @@ -253,3 +291,23 @@ SYM_DATA_START(test_kprobes_functions) #endif /* CONFIG_RISCV_ISA_C */ RISCV_PTR 0 SYM_DATA_END(test_kprobes_functions) + +.section .rodata +SYM_DATA_START(test_kprobes_lrsc_allowed_addresses) + RISCV_PTR test_kprobes_lrsc_addr0 + RISCV_PTR test_kprobes_lrsc_addr4 + RISCV_PTR test_kprobes_lrsc_addr5 + RISCV_PTR test_kprobes_lrsc_addr6 + RISCV_PTR test_kprobes_lrsc_addr7 + RISCV_PTR 0 +SYM_DATA_END(test_kprobes_lrsc_allowed_addresses) + +.section .rodata +SYM_DATA_START(test_kprobes_lrsc_rejected_addresses) + RISCV_PTR test_kprobes_lrsc_addr1 + RISCV_PTR test_kprobes_lrsc_addr2 + RISCV_PTR test_kprobes_lrsc_addr3 + RISCV_PTR test_kprobes_lrsc_addr8 + RISCV_PTR test_kprobes_lrsc_addr9 + RISCV_PTR 0 +SYM_DATA_END(test_kprobes_lrsc_rejected_addresses) diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes.c b/arch/riscv/kernel/tests/kprobes/test-kprobes.c index 027424a3ff7b..ff9d5bb2b998 100644 --- a/arch/riscv/kernel/tests/kprobes/test-kprobes.c +++ b/arch/riscv/kernel/tests/kprobes/test-kprobes.c @@ -43,8 +43,40 @@ static void test_kprobe_riscv(struct kunit *test) kfree(kp); } +static void test_kprobe_riscv_lrsc(struct kunit *test) +{ + /* probes outside the LR/SC sequence must be accepted */ + for (int i = 0; test_kprobes_lrsc_allowed_addresses[i]; i++) { + struct kprobe kp = {}; + int ret; + + kp.pre_handler = kprobe_dummy_handler; + kp.addr = test_kprobes_lrsc_allowed_addresses[i]; + + ret = register_kprobe(&kp); + KUNIT_EXPECT_EQ_MSG(test, ret, 0, "location %pS rejected", kp.addr); + if (!ret) + unregister_kprobe(&kp); + } + + /* probes on or inside the LR/SC sequence must be rejected */ + for (int i = 0; test_kprobes_lrsc_rejected_addresses[i]; i++) { + struct kprobe kp = {}; + int ret; + + kp.pre_handler = kprobe_dummy_handler; + kp.addr = test_kprobes_lrsc_rejected_addresses[i]; + + ret = register_kprobe(&kp); + KUNIT_EXPECT_LT_MSG(test, ret, 0, "location %pS accepted", kp.addr); + if (!ret) + unregister_kprobe(&kp); + } +} + static struct kunit_case kprobes_testcases[] = { KUNIT_CASE(test_kprobe_riscv), + KUNIT_CASE(test_kprobe_riscv_lrsc), {} }; diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes.h b/arch/riscv/kernel/tests/kprobes/test-kprobes.h index 537f44aa9d3f..93393e77a2cf 100644 --- a/arch/riscv/kernel/tests/kprobes/test-kprobes.h +++ b/arch/riscv/kernel/tests/kprobes/test-kprobes.h @@ -19,6 +19,12 @@ extern void *test_kprobes_addresses[]; /* array of functions that return KPROBE_TEST_MAGIC */ extern long (*test_kprobes_functions[])(void); +/* probe points that must be accepted, terminated by NULL */ +extern void *test_kprobes_lrsc_allowed_addresses[]; + +/* probe points that must be rejected, terminated by NULL */ +extern void *test_kprobes_lrsc_rejected_addresses[]; + #endif /* __ASSEMBLER__ */ #endif /* TEST_KPROBES_H */ -- 2.43.0 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-11-15 13:01 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-11-15 12:37 [PATCH v4 0/3] riscv: probes: reject probes inside an LR/SC sequence Vladimir Kondratiev 2026-11-15 12:37 ` Vladimir Kondratiev 2026-11-15 12:37 ` [PATCH v4 1/3] riscv: kprobes: fix out-of-bounds read in riscv_probe_decode_insn Vladimir Kondratiev 2026-11-15 12:37 ` Vladimir Kondratiev 2026-11-15 12:37 ` [PATCH v4 2/3] riscv: probes: reject probes inside an LR/SC sequence Vladimir Kondratiev 2026-11-15 12:37 ` Vladimir Kondratiev 2026-11-15 13:01 ` sashiko-bot 2026-11-15 12:37 ` [PATCH v4 3/3] riscv: kprobes: add KUnit test for LR/SC sequence rejection Vladimir Kondratiev 2026-11-15 12:37 ` Vladimir Kondratiev
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.