| Message ID | 20230827083949.204927-1-bjorn@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | riscv: Only consider swbp/ss handlers for correct privileged mode | expand |
Hi Björn, kernel test robot noticed the following build warnings: [auto build test WARNING on 7d2f353b2682dcfe5f9bc71e5b61d5b61770d98e] url: https://github.com/intel-lab-lkp/linux/commits/Bj-rn-T-pel/riscv-Only-consider-swbp-ss-handlers-for-correct-privileged-mode/20230827-164313 base: 7d2f353b2682dcfe5f9bc71e5b61d5b61770d98e patch link: https://lore.kernel.org/r/20230827083949.204927-1-bjorn%40kernel.org patch subject: [PATCH] riscv: Only consider swbp/ss handlers for correct privileged mode config: riscv-randconfig-001-20230827 (https://download.01.org/0day-ci/archive/20230827/202308271841.HlnnHFL7-lkp@intel.com/config) compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a) reproduce: (https://download.01.org/0day-ci/archive/20230827/202308271841.HlnnHFL7-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202308271841.HlnnHFL7-lkp@intel.com/ All warnings (new ones prefixed by >>): In file included from kernel/sched/core.c:49: In file included from include/linux/kprobes.h:32: >> arch/riscv/include/asm/kprobes.h:44:62: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions] 44 | static inline bool kprobe_breakpoint_handler(struct pt_regs *) | ^ arch/riscv/include/asm/kprobes.h:49:63: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions] 49 | static inline bool kprobe_single_step_handler(struct pt_regs *) | ^ kernel/sched/core.c:3695:20: warning: unused function 'rq_has_pinned_tasks' [-Wunused-function] 3695 | static inline bool rq_has_pinned_tasks(struct rq *rq) | ^ kernel/sched/core.c:5821:20: warning: unused function 'sched_tick_start' [-Wunused-function] 5821 | static inline void sched_tick_start(int cpu) { } | ^ kernel/sched/core.c:5822:20: warning: unused function 'sched_tick_stop' [-Wunused-function] 5822 | static inline void sched_tick_stop(int cpu) { } | ^ kernel/sched/core.c:6522:20: warning: unused function 'sched_core_cpu_starting' [-Wunused-function] 6522 | static inline void sched_core_cpu_starting(unsigned int cpu) {} | ^ kernel/sched/core.c:6523:20: warning: unused function 'sched_core_cpu_deactivate' [-Wunused-function] 6523 | static inline void sched_core_cpu_deactivate(unsigned int cpu) {} | ^ kernel/sched/core.c:6524:20: warning: unused function 'sched_core_cpu_dying' [-Wunused-function] 6524 | static inline void sched_core_cpu_dying(unsigned int cpu) {} | ^ 8 warnings generated. vim +44 arch/riscv/include/asm/kprobes.h 38 39 void arch_remove_kprobe(struct kprobe *p); 40 int kprobe_fault_handler(struct pt_regs *regs, unsigned int trapnr); 41 bool kprobe_breakpoint_handler(struct pt_regs *regs); 42 bool kprobe_single_step_handler(struct pt_regs *regs); 43 #else > 44 static inline bool kprobe_breakpoint_handler(struct pt_regs *) 45 { 46 return false; 47 } 48
On Sun, Aug 27, 2023 at 4:39 AM Björn Töpel <bjorn@kernel.org> wrote: > > From: Björn Töpel <bjorn@rivosinc.com> > > RISC-V software breakpoint trap handlers are used for {k,u}probes. > > When trapping from kernelmode, only the kernelmode handlers should be > considered. Vice versa, only usermode handlers for usermode > traps. This is not the case on RISC-V, which can trigger a bug if a > userspace process uses uprobes, and a WARN() is triggered from > kernelmode (which is implemented via {c.,}ebreak). > > The kernel will trap on the kernelmode {c.,}ebreak, look for uprobes > handlers, realize incorrectly that uprobes need to be handled, and > exit the trap handler early. The trap returns to re-executing the > {c.,}ebreak, and enter an infinite trap-loop. Thx for the fixup. Reviewed-by: Guo Ren <guoren@kernel.org> > > The issue was found running the BPF selftest [1]. > > Fix this issue by only considering the swbp/ss handlers for > kernel/usermode respectively. Also, move CONFIG ifdeffery from traps.c > to the asm/{k,u}probes.h headers. > > Note that linux/uprobes.h only include asm/uprobes.h if CONFIG_UPROBES > is defined, which is why asm/uprobes.h needs to be unconditionally > included in traps.c > > Link: https://lore.kernel.org/linux-riscv/87v8d19aun.fsf@all.your.base.are.belong.to.us/ # [1] > Fixes: 74784081aac8 ("riscv: Add uprobes supported") > Signed-off-by: Björn Töpel <bjorn@rivosinc.com> > --- > arch/riscv/include/asm/kprobes.h | 11 ++++++++++- > arch/riscv/include/asm/uprobes.h | 13 ++++++++++++- > arch/riscv/kernel/traps.c | 28 ++++++++++++++++++---------- > 3 files changed, 40 insertions(+), 12 deletions(-) > > diff --git a/arch/riscv/include/asm/kprobes.h b/arch/riscv/include/asm/kprobes.h > index e7882ccb0fd4..89fbd90f16a2 100644 > --- a/arch/riscv/include/asm/kprobes.h > +++ b/arch/riscv/include/asm/kprobes.h > @@ -40,6 +40,15 @@ void arch_remove_kprobe(struct kprobe *p); > int kprobe_fault_handler(struct pt_regs *regs, unsigned int trapnr); > bool kprobe_breakpoint_handler(struct pt_regs *regs); > bool kprobe_single_step_handler(struct pt_regs *regs); > - > +#else > +static inline bool kprobe_breakpoint_handler(struct pt_regs *) > +{ > + return false; > +} > + > +static inline bool kprobe_single_step_handler(struct pt_regs *) > +{ > + return false; > +} > #endif /* CONFIG_KPROBES */ > #endif /* _ASM_RISCV_KPROBES_H */ > diff --git a/arch/riscv/include/asm/uprobes.h b/arch/riscv/include/asm/uprobes.h > index f2183e00fdd2..3fc7deda9190 100644 > --- a/arch/riscv/include/asm/uprobes.h > +++ b/arch/riscv/include/asm/uprobes.h > @@ -34,7 +34,18 @@ struct arch_uprobe { > bool simulate; > }; > > +#ifdef CONFIG_UPROBES > bool uprobe_breakpoint_handler(struct pt_regs *regs); > bool uprobe_single_step_handler(struct pt_regs *regs); > - > +#else > +static inline bool uprobe_breakpoint_handler(struct pt_regs *regs) > +{ > + return false; > +} > + > +static inline bool uprobe_single_step_handler(struct pt_regs *regs) > +{ > + return false; > +} > +#endif /* CONFIG_UPROBES */ > #endif /* _ASM_RISCV_UPROBES_H */ > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > index f798c853bede..cd6f10c73a16 100644 > --- a/arch/riscv/kernel/traps.c > +++ b/arch/riscv/kernel/traps.c > @@ -13,6 +13,8 @@ > #include <linux/kdebug.h> > #include <linux/uaccess.h> > #include <linux/kprobes.h> > +#include <linux/uprobes.h> > +#include <asm/uprobes.h> > #include <linux/mm.h> > #include <linux/module.h> > #include <linux/irq.h> > @@ -246,22 +248,28 @@ static inline unsigned long get_break_insn_length(unsigned long pc) > return GET_INSN_LENGTH(insn); > } > > +static bool probe_single_step_handler(struct pt_regs *regs) > +{ > + bool user = user_mode(regs); > + > + return user ? uprobe_single_step_handler(regs) : kprobe_single_step_handler(regs); > +} > + > +static bool probe_breakpoint_handler(struct pt_regs *regs) > +{ > + bool user = user_mode(regs); > + > + return user ? uprobe_breakpoint_handler(regs) : kprobe_breakpoint_handler(regs); > +} > + > void handle_break(struct pt_regs *regs) > { > -#ifdef CONFIG_KPROBES > - if (kprobe_single_step_handler(regs)) > + if (probe_single_step_handler(regs)) > return; > > - if (kprobe_breakpoint_handler(regs)) > - return; > -#endif > -#ifdef CONFIG_UPROBES > - if (uprobe_single_step_handler(regs)) > + if (probe_breakpoint_handler(regs)) > return; > > - if (uprobe_breakpoint_handler(regs)) > - return; > -#endif > current->thread.bad_cause = regs->cause; > > if (user_mode(regs)) > > base-commit: 7d2f353b2682dcfe5f9bc71e5b61d5b61770d98e > -- > 2.39.2 >
diff --git a/arch/riscv/include/asm/kprobes.h b/arch/riscv/include/asm/kprobes.h index e7882ccb0fd4..89fbd90f16a2 100644 --- a/arch/riscv/include/asm/kprobes.h +++ b/arch/riscv/include/asm/kprobes.h @@ -40,6 +40,15 @@ void arch_remove_kprobe(struct kprobe *p); int kprobe_fault_handler(struct pt_regs *regs, unsigned int trapnr); bool kprobe_breakpoint_handler(struct pt_regs *regs); bool kprobe_single_step_handler(struct pt_regs *regs); - +#else +static inline bool kprobe_breakpoint_handler(struct pt_regs *) +{ + return false; +} + +static inline bool kprobe_single_step_handler(struct pt_regs *) +{ + return false; +} #endif /* CONFIG_KPROBES */ #endif /* _ASM_RISCV_KPROBES_H */ diff --git a/arch/riscv/include/asm/uprobes.h b/arch/riscv/include/asm/uprobes.h index f2183e00fdd2..3fc7deda9190 100644 --- a/arch/riscv/include/asm/uprobes.h +++ b/arch/riscv/include/asm/uprobes.h @@ -34,7 +34,18 @@ struct arch_uprobe { bool simulate; }; +#ifdef CONFIG_UPROBES bool uprobe_breakpoint_handler(struct pt_regs *regs); bool uprobe_single_step_handler(struct pt_regs *regs); - +#else +static inline bool uprobe_breakpoint_handler(struct pt_regs *regs) +{ + return false; +} + +static inline bool uprobe_single_step_handler(struct pt_regs *regs) +{ + return false; +} +#endif /* CONFIG_UPROBES */ #endif /* _ASM_RISCV_UPROBES_H */ diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index f798c853bede..cd6f10c73a16 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -13,6 +13,8 @@ #include <linux/kdebug.h> #include <linux/uaccess.h> #include <linux/kprobes.h> +#include <linux/uprobes.h> +#include <asm/uprobes.h> #include <linux/mm.h> #include <linux/module.h> #include <linux/irq.h> @@ -246,22 +248,28 @@ static inline unsigned long get_break_insn_length(unsigned long pc) return GET_INSN_LENGTH(insn); } +static bool probe_single_step_handler(struct pt_regs *regs) +{ + bool user = user_mode(regs); + + return user ? uprobe_single_step_handler(regs) : kprobe_single_step_handler(regs); +} + +static bool probe_breakpoint_handler(struct pt_regs *regs) +{ + bool user = user_mode(regs); + + return user ? uprobe_breakpoint_handler(regs) : kprobe_breakpoint_handler(regs); +} + void handle_break(struct pt_regs *regs) { -#ifdef CONFIG_KPROBES - if (kprobe_single_step_handler(regs)) + if (probe_single_step_handler(regs)) return; - if (kprobe_breakpoint_handler(regs)) - return; -#endif -#ifdef CONFIG_UPROBES - if (uprobe_single_step_handler(regs)) + if (probe_breakpoint_handler(regs)) return; - if (uprobe_breakpoint_handler(regs)) - return; -#endif current->thread.bad_cause = regs->cause; if (user_mode(regs))