Björn Töpel <bjorn@xxxxxxxxxx> writes:
> I'm chasing a workqueue hang on RISC-V/qemu (TCG), using the bpf
> selftests on bpf-next 9e3b47abeb8f.
>
> I'm able to reproduce the hang by multiple runs of:
> | ./test_progs -a link_api -a linked_list
> I'm currently investigating that.
+Guo for uprobe
This was an interesting bug. The hang is an ebreak (RISC-V breakpoint),
that puts the kernel into an infinite loop.
To reproduce, simply run the BPF selftest:
./test_progs -v -a link_api -a linked_list
First the link_api test is being run, which exercises the uprobe
functionality. The link_api test completes, and test_progs will still
have the uprobe active/enabled. Next the linked_list test triggered a
WARN_ON (which is implemented via ebreak as well).
Now, handle_break() is entered, and the uprobe_breakpoint_handler()
returns true exiting the handle_break(), which returns to the WARN
ebreak, and we have merry-go-round.
Lucky for the RISC-V folks, the BPF memory handler had a WARN that
surfaced the bug! ;-)
This patch fixes the issue, but it's probably a prettier variant:
--8<--
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index f798c853bede..1198cb879d2f 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -248,23 +248,29 @@ static inline unsigned long get_break_insn_length(unsigned long pc)
void handle_break(struct pt_regs *regs)
{
+ bool user = user_mode(regs);
+
#ifdef CONFIG_KPROBES
- if (kprobe_single_step_handler(regs))
- return;
+ if (!user) {
+ if (kprobe_single_step_handler(regs))
+ return;
- if (kprobe_breakpoint_handler(regs))
- return;
+ if (kprobe_breakpoint_handler(regs))
+ return;
+ }
#endif
#ifdef CONFIG_UPROBES
- if (uprobe_single_step_handler(regs))
- return;
+ if (user) {
+ if (uprobe_single_step_handler(regs))
+ return;
- if (uprobe_breakpoint_handler(regs))
- return;
+ if (uprobe_breakpoint_handler(regs))
+ return;
+ }
#endif
current->thread.bad_cause = regs->cause;
- if (user_mode(regs))
+ if (user)
force_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->epc);
#ifdef CONFIG_KGDB
else if (notify_die(DIE_TRAP, "EBREAK", regs, 0, regs->cause, SIGTRAP)
--8<--
I'll cook a cleaner/proper patch for this, unless the uprobes folks has
a better solution.
Björn
