Skip to contentSkip to conversation

riscv: Add k/uprobe supported

PATCH Superseded [email protected] 3 participants 13 messages Jul 9, 2020 – Jul 21, 2020

Subsystems (5)

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

v2 cover riscv: Add k/uprobe supported

From: Guo Ren <[email protected]>

The patchset includes kprobe/uprobe support and some related fixups. Patrick provides HAVE_REGS_AND_STACK_ACCESS_API support and some kprobe's code. The framework of k/uprobe is from csky but also refers to other arches'. kprobes on ftrace is also supported in the patchset.

There is no single step exception in riscv ISA, only single-step facility for jtag. See riscv-Privileged spec:

Interrupt Exception Code-Description
1 0 Reserved
1 1 Supervisor software interrupt
1 2–4 Reserved
1 5 Supervisor timer interrupt
1 6–8 Reserved
1 9 Supervisor external interrupt
1 10–15 Reserved
1 ≥16 Available for platform use
0 0 Instruction address misaligned
0 1 Instruction access fault
0 2 Illegal instruction
0 3 Breakpoint
0 4 Load address misaligned
0 5 Load access fault
0 6 Store/AMO address misaligned
0 7 Store/AMO access fault
0 8 Environment call from U-mode
0 9 Environment call from S-mode
0 10–11 Reserved
0 12 Instruction page fault
0 13 Load page fault
0 14 Reserved
0 15 Store/AMO page fault
0 16–23 Reserved
0 24–31 Available for custom use
0 32–47 Reserved
0 48–63 Available for custom use
0 ≥64 Reserved

No single step!

Other arches use hardware single-step exception for k/uprobe, eg:
- powerpc: regs->msr |= MSR_SINGLESTEP
- arm/arm64: PSTATE.D for enabling software step exceptions
- s390: Set PER control regs, turns on single step for the given address
- x86: regs->flags |= X86_EFLAGS_TF
- csky: of course use hw single step :)

All the above arches use a hardware single-step exception mechanism to execute the instruction that was replaced with a probe breakpoint. So utilize ebreak to simulate.

Some pc related instructions couldn't be executed out of line and some system/fence instructions couldn't be a trace site at all. So we give out a reject list and simulate list in decode-insn.c.

You could use uprobe to test simulate code like this:

echo 'p:enter_current_state_one /hello:0x6e4 a0=%a0 a1=%a1' >> /sys/kernel/debug/tracing/uprobe_events echo 1 > /sys/kernel/debug/tracing/events/uprobes/enable
/hello
^C
cat /sys/kernel/debug/tracing/trace
tracer: nop

entries-in-buffer/entries-written: 1/1 #P:1

                              _-----=> irqs-off
                             / _----=> need-resched
                            | / _---=> hardirq/softirq
                            || / _--=> preempt-depth
                            ||| /     delay
           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
              | |       |   ||||       |         |
          hello-94    [000] d...    55.404242: enter_current_state_one: (0x106e4) a0=0x1 a1=0x3fffa8ada8

Be care /hello:0x6e4 is the file offset in elf and it relate to 0x106e4 in memory and hello is your target elf program.

Try kprobe like this:

echo 'p:myprobe _do_fork dfd=%a0 filename=%a1 flags=%a2 mode=+4($stack)' > /sys/kernel/debug/tracing/kprobe_events echo 'r:myretprobe _do_fork $retval' >> /sys/kernel/debug/tracing/kprobe_event

echo 1 >/sys/kernel/debug/tracing/events/kprobes/enable cat /sys/kernel/debug/tracing/trace
tracer: nop

entries-in-buffer/entries-written: 2/2 #P:1

                              _-----=> irqs-off
                             / _----=> need-resched
                            | / _---=> hardirq/softirq
                            || / _--=> preempt-depth
                            ||| /     delay
           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
              | |       |   ||||       |         |
             sh-92    [000] .n..   131.804230: myprobe: (_do_fork+0x0/0x2e6) dfd=0xffffffe03929fdf8 filename=0x0 flags=0x101000 mode=0x1200000ffffffe0
             sh-92    [000] d...   131.806607: myretprobe: (__do_sys_clone+0x70/0x82 <- _do_fork) arg1=0x5f

cat /sys/kernel/debug/tracing/trace

Changes v2:
- Add Reviewed-by, Tested-by, Acked-by, thx for all of you
- Add kprobes on ftrace feature

Guo Ren (5):
riscv: Fixup __vdso_gettimeofday broke dynamic ftrace
riscv: Fixup compile error BUILD_BUG_ON failed
riscv: Add kprobes supported
riscv: Add uprobes supported
riscv: Add KPROBES_ON_FTRACE supported

Patrick Stählin (1):
RISC-V: Implement ptrace regs and stack API

 arch/riscv/Kconfig                            |   7 +
 arch/riscv/include/asm/kprobes.h              |  40 +++
 arch/riscv/include/asm/probes.h               |  24 ++
 arch/riscv/include/asm/processor.h            |   1 +
 arch/riscv/include/asm/ptrace.h               |  29 ++
 arch/riscv/include/asm/thread_info.h          |   4 +-
 arch/riscv/include/asm/uprobes.h              |  40 +++
 arch/riscv/kernel/Makefile                    |   1 +
 arch/riscv/kernel/patch.c                     |   8 +-
 arch/riscv/kernel/probes/Makefile             |   6 +
 arch/riscv/kernel/probes/decode-insn.c        |  48 +++
 arch/riscv/kernel/probes/decode-insn.h        |  18 +
 arch/riscv/kernel/probes/ftrace.c             |  53 +++
 arch/riscv/kernel/probes/kprobes.c            | 471 ++++++++++++++++++++++++++
 arch/riscv/kernel/probes/kprobes_trampoline.S |  93 +++++
 arch/riscv/kernel/probes/simulate-insn.c      |  85 +++++
 arch/riscv/kernel/probes/simulate-insn.h      |  47 +++
 arch/riscv/kernel/probes/uprobes.c            | 186 ++++++++++
 arch/riscv/kernel/ptrace.c                    |  99 ++++++
 arch/riscv/kernel/signal.c                    |   3 +
 arch/riscv/kernel/traps.c                     |  19 ++
 arch/riscv/kernel/vdso/Makefile               |   3 +
 arch/riscv/mm/fault.c                         |  11 +
 23 files changed, 1293 insertions(+), 3 deletions(-)
 create mode 100644 arch/riscv/include/asm/probes.h
 create mode 100644 arch/riscv/include/asm/uprobes.h
 create mode 100644 arch/riscv/kernel/probes/Makefile
 create mode 100644 arch/riscv/kernel/probes/decode-insn.c
 create mode 100644 arch/riscv/kernel/probes/decode-insn.h
 create mode 100644 arch/riscv/kernel/probes/ftrace.c
 create mode 100644 arch/riscv/kernel/probes/kprobes.c
 create mode 100644 arch/riscv/kernel/probes/kprobes_trampoline.S
 create mode 100644 arch/riscv/kernel/probes/simulate-insn.c
 create mode 100644 arch/riscv/kernel/probes/simulate-insn.h
 create mode 100644 arch/riscv/kernel/probes/uprobes.c
Signature · 1 line
2.7.4
author maintainer
#1 lore raw

PATCH v2 1/6 riscv: Fixup __vdso_gettimeofday broke dynamic ftrace

From: Guo Ren <[email protected]>

For linux-5.8-rc1, enable ftrace of riscv will cause boot panic:

[    2.388980] Run /sbin/init as init process
[    2.529938] init[39]: unhandled signal 4 code 0x1 at 0x0000003ff449e000
[    2.531078] CPU: 0 PID: 39 Comm: init Not tainted 5.8.0-rc1-dirty #13
[    2.532719] epc: 0000003ff449e000 ra : 0000003ff449e954 sp : 0000003fffedb900
[    2.534005]  gp : 00000000000e8528 tp : 0000003ff449d800 t0 : 000000000000001e
[    2.534965]  t1 : 000000000000000a t2 : 0000003fffedb89e s0 : 0000003fffedb920
[    2.536279]  s1 : 0000003fffedb940 a0 : 0000003ff43d4b2c a1 : 0000000000000000
[    2.537334]  a2 : 0000000000000001 a3 : 0000000000000000 a4 : fffffffffbad8000
[    2.538466]  a5 : 0000003ff449e93a a6 : 0000000000000000 a7 : 0000000000000000
[    2.539511]  s2 : 0000000000000000 s3 : 0000003ff448412c s4 : 0000000000000010
[    2.541260]  s5 : 0000000000000016 s6 : 00000000000d0a30 s7 : 0000003fffedba70
[    2.542152]  s8 : 0000000000000000 s9 : 0000000000000000 s10: 0000003fffedb960
[    2.543335]  s11: 0000000000000000 t3 : 0000000000000000 t4 : 0000003fffedb8a0
[    2.544471]  t5 : 0000000000000000 t6 : 0000000000000000
[    2.545730] status: 0000000000004020 badaddr: 00000000464c457f cause: 0000000000000002
[    2.549867] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000004
[    2.551267] CPU: 0 PID: 1 Comm: init Not tainted 5.8.0-rc1-dirty #13
[    2.552061] Call Trace:
[    2.552626] [<ffffffe00020374a>] walk_stackframe+0x0/0xc4
[    2.553486] [<ffffffe0002039f4>] show_stack+0x40/0x4c
[    2.553995] [<ffffffe00054a6ae>] dump_stack+0x7a/0x98
[    2.554615] [<ffffffe00020b9b8>] panic+0x114/0x2f4
[    2.555395] [<ffffffe00020ebd6>] do_exit+0x89c/0x8c2
[    2.555949] [<ffffffe00020f930>] do_group_exit+0x3a/0x90
[    2.556715] [<ffffffe000219e08>] get_signal+0xe2/0x6e6
[    2.557388] [<ffffffe000202d72>] do_notify_resume+0x6a/0x37a
[    2.558089] [<ffffffe000201c16>] ret_from_exception+0x0/0xc

"ra:0x3ff449e954" is the return address of "call _mcount" in the prologue of __vdso_gettimeofday(). Without proper relocate, pc jmp to 0x0000003ff449e000 (vdso map base) with a illegal instruction trap.

The solution comes from arch/arm64/kernel/vdso/Makefile:

CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) -Os $(CC_FLAGS_SCS)

- CC_FLAGS_SCS is ShadowCallStack feature in Clang and only implemented for arm64, no use for riscv.

The bug comes from the following commit:

ad5d1122b82f ("riscv: use vDSO common flow to reduce the latency of the time-related functions")

Cc Vincent Chen <[email protected]>
Cc Atish Patra <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Alan Kao <[email protected]>
Cc Greentime Hu <[email protected]>
---
 arch/riscv/kernel/vdso/Makefile | 3 +++
 1 file changed, 3 insertions(+)
arch/riscv/kernel/vdso/Makefile +3 -0
index 38ba55b..3079935 100644
@@ -27,6 +27,9 @@ obj-vdso := $(addprefix $(obj)/, $(obj-vdso))
 obj-y += vdso.o vdso-syms.o
 CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
 
+# Disable -pg to prevent insert call site
+CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) -Os
+
 # Disable gcov profiling for VDSO code
 GCOV_PROFILE := n
 
Signature · 1 line
2.7.4
#2 lore raw

Re: [PATCH v2 1/6] riscv: Fixup __vdso_gettimeofday broke dynamic ftrace

Quoted 64 lines · On Wed, 08 Jul 2020 19:19:09 PDT (-0700), [email protected] wrote:

From: Guo Ren <[email protected]>

For linux-5.8-rc1, enable ftrace of riscv will cause boot panic:

[    2.388980] Run /sbin/init as init process
[    2.529938] init[39]: unhandled signal 4 code 0x1 at 0x0000003ff449e000
[    2.531078] CPU: 0 PID: 39 Comm: init Not tainted 5.8.0-rc1-dirty #13
[    2.532719] epc: 0000003ff449e000 ra : 0000003ff449e954 sp : 0000003fffedb900
[    2.534005]  gp : 00000000000e8528 tp : 0000003ff449d800 t0 : 000000000000001e
[    2.534965]  t1 : 000000000000000a t2 : 0000003fffedb89e s0 : 0000003fffedb920
[    2.536279]  s1 : 0000003fffedb940 a0 : 0000003ff43d4b2c a1 : 0000000000000000
[    2.537334]  a2 : 0000000000000001 a3 : 0000000000000000 a4 : fffffffffbad8000
[    2.538466]  a5 : 0000003ff449e93a a6 : 0000000000000000 a7 : 0000000000000000
[    2.539511]  s2 : 0000000000000000 s3 : 0000003ff448412c s4 : 0000000000000010
[    2.541260]  s5 : 0000000000000016 s6 : 00000000000d0a30 s7 : 0000003fffedba70
[    2.542152]  s8 : 0000000000000000 s9 : 0000000000000000 s10: 0000003fffedb960
[    2.543335]  s11: 0000000000000000 t3 : 0000000000000000 t4 : 0000003fffedb8a0
[    2.544471]  t5 : 0000000000000000 t6 : 0000000000000000
[    2.545730] status: 0000000000004020 badaddr: 00000000464c457f cause: 0000000000000002
[    2.549867] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000004
[    2.551267] CPU: 0 PID: 1 Comm: init Not tainted 5.8.0-rc1-dirty #13
[    2.552061] Call Trace:
[    2.552626] [<ffffffe00020374a>] walk_stackframe+0x0/0xc4
[    2.553486] [<ffffffe0002039f4>] show_stack+0x40/0x4c
[    2.553995] [<ffffffe00054a6ae>] dump_stack+0x7a/0x98
[    2.554615] [<ffffffe00020b9b8>] panic+0x114/0x2f4
[    2.555395] [<ffffffe00020ebd6>] do_exit+0x89c/0x8c2
[    2.555949] [<ffffffe00020f930>] do_group_exit+0x3a/0x90
[    2.556715] [<ffffffe000219e08>] get_signal+0xe2/0x6e6
[    2.557388] [<ffffffe000202d72>] do_notify_resume+0x6a/0x37a
[    2.558089] [<ffffffe000201c16>] ret_from_exception+0x0/0xc

"ra:0x3ff449e954" is the return address of "call _mcount" in the prologue of __vdso_gettimeofday(). Without proper relocate, pc jmp to 0x0000003ff449e000 (vdso map base) with a illegal instruction trap.

The solution comes from arch/arm64/kernel/vdso/Makefile:

CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) -Os $(CC_FLAGS_SCS)

- CC_FLAGS_SCS is ShadowCallStack feature in Clang and only implemented for arm64, no use for riscv.

The bug comes from the following commit:

ad5d1122b82f ("riscv: use vDSO common flow to reduce the latency of the time-related functions")

Cc Vincent Chen <[email protected]>
Cc Atish Patra <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Alan Kao <[email protected]>
Cc Greentime Hu <[email protected]>
---
 arch/riscv/kernel/vdso/Makefile | 3 +++
 1 file changed, 3 insertions(+)
arch/riscv/kernel/vdso/Makefile +0 -0
index 38ba55b..3079935 100644

arch/riscv/kernel/vdso/Makefile obj-vdso := $(addprefix $(obj)/, $(obj-vdso)) · line 34

Quoted 8 lines
arch/riscv/kernel/vdso/Makefile +3 -0
@@ -27,6 +27,9 @@ obj-vdso := $(addprefix $(obj)/, $(obj-vdso))
 obj-y += vdso.o vdso-syms.o
 CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
 
+# Disable -pg to prevent insert call site
+CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) -Os
+
 # Disable gcov profiling for VDSO code
 GCOV_PROFILE := n

As far as I can tell this is already in Linus' tree. LMK if I'm missing something.

author maintainer
#3 lore raw

PATCH v2 2/6 RISC-V: Implement ptrace regs and stack API

In reply to [email protected]

From: Patrick Stählin <[email protected]>

Needed for kprobes support. Copied and adapted from arm64 code.

Guo Ren fixup pt_regs type for linux-5.8-rc1.

---
 arch/riscv/Kconfig              |  1 +
 arch/riscv/include/asm/ptrace.h | 29 ++++++++++++
 arch/riscv/kernel/ptrace.c      | 99 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 129 insertions(+)
arch/riscv/Kconfig +1 -0
index 128192e..58d6f66 100644
@@ -76,6 +76,7 @@ config RISCV
 	select SPARSE_IRQ
 	select SYSCTL_EXCEPTION_TRACE
 	select THREAD_INFO_IN_TASK
+	select HAVE_REGS_AND_STACK_ACCESS_API
 
 config ARCH_MMAP_RND_BITS_MIN
 	default 18 if 64BIT
arch/riscv/include/asm/ptrace.h +29 -0
index ee49f80..23372bb 100644
@@ -8,6 +8,7 @@
 
 #include <uapi/asm/ptrace.h>
 #include <asm/csr.h>
+#include <linux/compiler.h>
 
 #ifndef __ASSEMBLY__
 
@@ -60,6 +61,7 @@ struct pt_regs {
 
 #define user_mode(regs) (((regs)->status & SR_PP) == 0)
 
+#define MAX_REG_OFFSET offsetof(struct pt_regs, orig_a0)
 
 /* Helpers for working with the instruction pointer */
 static inline unsigned long instruction_pointer(struct pt_regs *regs)
@@ -85,6 +87,12 @@ static inline void user_stack_pointer_set(struct pt_regs *regs,
 	regs->sp =  val;
 }
 
+/* Valid only for Kernel mode traps. */
+static inline unsigned long kernel_stack_pointer(struct pt_regs *regs)
+{
+	return regs->sp;
+}
+
 /* Helpers for working with the frame pointer */
 static inline unsigned long frame_pointer(struct pt_regs *regs)
 {
@@ -101,6 +109,27 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
 	return regs->a0;
 }
 
+extern int regs_query_register_offset(const char *name);
+extern unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs,
+					       unsigned int n);
+
+/**
+ * regs_get_register() - get register value from its offset
+ * @regs:	pt_regs from which register value is gotten
+ * @offset:	offset of the register.
+ *
+ * regs_get_register returns the value of a register whose offset from @regs.
+ * The @offset is the offset of the register in struct pt_regs.
+ * If @offset is bigger than MAX_REG_OFFSET, this returns 0.
+ */
+static inline unsigned long regs_get_register(struct pt_regs *regs,
+					      unsigned int offset)
+{
+	if (unlikely(offset > MAX_REG_OFFSET))
+		return 0;
+
+	return *(unsigned long *)((unsigned long)regs + offset);
+}
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_RISCV_PTRACE_H */
arch/riscv/kernel/ptrace.c +99 -0

105 lines not on this page · raw message

Signature · 1 line
2.7.4
author maintainer
#4 lore raw

PATCH v2 3/6 riscv: Fixup compile error BUILD_BUG_ON failed

In reply to [email protected]

From: Guo Ren <[email protected]>

Unfortunately, the current code couldn't be compiled:

CC arch/riscv/kernel/patch.o
In file included from ./include/linux/kernel.h:11,

                 from ./include/linux/list.h:9,
                 from ./include/linux/preempt.h:11,
                 from ./include/linux/spinlock.h:51,
                 from arch/riscv/kernel/patch.c:6:

In function ‘fix_to_virt’,
inlined from ‘patch_map’ at arch/riscv/kernel/patch.c:37:17:
./include/linux/compiler.h:392:38: error: call to ‘__compiletime_assert_205’ declared with attribute error: BUILD_BUG_ON failed: idx >= __end_of_fixed_addresses
_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)

                                      ^
./include/linux/compiler.h:373:4: note: in definition of macro ‘__compiletime_assert’
    prefix ## suffix();    \
    ^~~~~~

./include/linux/compiler.h:392:2: note: in expansion of macro ‘_compiletime_assert’
_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’

 #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                     ^~~~~~~~~~~~~~~~~~

./include/linux/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) ^~~~~~~~~~~~~~~~
./include/asm-generic/fixmap.h:32:2: note: in expansion of macro ‘BUILD_BUG_ON’
BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
^~~~~~~~~~~~

Because fix_to_virt(, idx) needs a const value, not a dynamic variable of reg-a0 or BUILD_BUG_ON failed with "idx >= __end_of_fixed_addresses".

---
Changelog V2:
- Use __always_inline as same as fix_to_virt
- Use const "const unsigned int" for 2th param

---
 arch/riscv/kernel/patch.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
arch/riscv/kernel/patch.c +6 -2
index d4a64df..3179a4e 100644
@@ -20,7 +20,12 @@ struct patch_insn {
 };
 
 #ifdef CONFIG_MMU
-static void *patch_map(void *addr, int fixmap)
+/*
+ * The fix_to_virt(, idx) needs a const value (not a dynamic variable of
+ * reg-a0) or BUILD_BUG_ON failed with "idx >= __end_of_fixed_addresses".
+ * So use '__always_inline' and 'const unsigned int fixmap' here.
+ */
+static __always_inline void *patch_map(void *addr, const unsigned int fixmap)
 {
 	uintptr_t uintaddr = (uintptr_t) addr;
 	struct page *page;
@@ -37,7 +42,6 @@ static void *patch_map(void *addr, int fixmap)
 	return (void *)set_fixmap_offset(fixmap, page_to_phys(page) +
 					 (uintaddr & ~PAGE_MASK));
 }
-NOKPROBE_SYMBOL(patch_map);
 
 static void patch_unmap(int fixmap)
 {
Signature · 1 line
2.7.4
author maintainer
#5 lore raw

PATCH v2 4/6 riscv: Add kprobes supported

In reply to [email protected]

From: Guo Ren <[email protected]>

This patch enables "kprobe & kretprobe" to work with ftrace interface. It utilized software breakpoint as single-step mechanism.

Some instructions which can't be single-step executed must be simulated in kernel execution slot, such as: branch, jal, auipc, la ...

Some instructions should be rejected for probing and we use a blacklist to filter, such as: ecall, ebreak, ...

We use ebreak & c.ebreak to replace origin instruction and the kprobe handler prepares an executable memory slot for out-of-line execution with a copy of the original instruction being probed. In execution slot we add ebreak behind original instruction to simulate a single-setp mechanism.

The patch is based on packi's work [1] and csky's work [2].
- The kprobes_trampoline.S is all from packi's patch
- The single-step mechanism is new designed for riscv without hw single-step trap
- The simulation codes are from csky
- Frankly, all codes refer to other archs' implementation

[1] https://lore.kernel.org/linux-riscv/[email protected]/
[2] https://lore.kernel.org/linux-csky/[email protected]/

---
 arch/riscv/Kconfig                            |   2 +
 arch/riscv/include/asm/kprobes.h              |  40 +++
 arch/riscv/include/asm/probes.h               |  24 ++
 arch/riscv/kernel/Makefile                    |   1 +
 arch/riscv/kernel/probes/Makefile             |   4 +
 arch/riscv/kernel/probes/decode-insn.c        |  48 +++
 arch/riscv/kernel/probes/decode-insn.h        |  18 +
 arch/riscv/kernel/probes/kprobes.c            | 471 ++++++++++++++++++++++++++
 arch/riscv/kernel/probes/kprobes_trampoline.S |  93 +++++
 arch/riscv/kernel/probes/simulate-insn.c      |  85 +++++
 arch/riscv/kernel/probes/simulate-insn.h      |  47 +++
 arch/riscv/kernel/traps.c                     |   9 +
 arch/riscv/mm/fault.c                         |   4 +
 13 files changed, 846 insertions(+)
 create mode 100644 arch/riscv/include/asm/probes.h
 create mode 100644 arch/riscv/kernel/probes/Makefile
 create mode 100644 arch/riscv/kernel/probes/decode-insn.c
 create mode 100644 arch/riscv/kernel/probes/decode-insn.h
 create mode 100644 arch/riscv/kernel/probes/kprobes.c
 create mode 100644 arch/riscv/kernel/probes/kprobes_trampoline.S
 create mode 100644 arch/riscv/kernel/probes/simulate-insn.c
 create mode 100644 arch/riscv/kernel/probes/simulate-insn.h
arch/riscv/Kconfig +2 -0
index 58d6f66..a295f0b 100644
@@ -57,6 +57,8 @@ config RISCV
 	select HAVE_EBPF_JIT if MMU
 	select HAVE_FUTEX_CMPXCHG if FUTEX
 	select HAVE_GENERIC_VDSO if MMU && 64BIT
+	select HAVE_KPROBES
+	select HAVE_KRETPROBES
 	select HAVE_PCI
 	select HAVE_PERF_EVENTS
 	select HAVE_PERF_REGS
arch/riscv/include/asm/kprobes.h +40 -0
index 56a98ea3..4647d38 100644
@@ -11,4 +11,44 @@
 
 #include <asm-generic/kprobes.h>
 
+#ifdef CONFIG_KPROBES
+#include <linux/types.h>
+#include <linux/ptrace.h>
+#include <linux/percpu.h>
+
+#define __ARCH_WANT_KPROBES_INSN_SLOT
+#define MAX_INSN_SIZE			2
+
+#define flush_insn_slot(p)		do { } while (0)
+#define kretprobe_blacklist_size	0
+
+#include <asm/probes.h>
+
+struct prev_kprobe {
+	struct kprobe *kp;
+	unsigned int status;
+};
+
+/* Single step context for kprobe */
+struct kprobe_step_ctx {
+	unsigned long ss_pending;
+	unsigned long match_addr;
+};
+
+/* per-cpu kprobe control block */
+struct kprobe_ctlblk {
+	unsigned int kprobe_status;
+	unsigned long saved_status;
+	struct prev_kprobe prev_kprobe;
+	struct kprobe_step_ctx ss_ctx;
+};
+
+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);
+void kretprobe_trampoline(void);
+void __kprobes *trampoline_probe_handler(struct pt_regs *regs);
+
+#endif /* CONFIG_KPROBES */
 #endif /* _ASM_RISCV_KPROBES_H */
arch/riscv/include/asm/probes.h new file +24 -0
new file mode 100644
index 00000000..a787e6d
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _ASM_RISCV_PROBES_H
+#define _ASM_RISCV_PROBES_H
+
+typedef u32 probe_opcode_t;
+typedef bool (probes_handler_t) (u32 opcode, unsigned long addr, struct pt_regs *);
+
+/* architecture specific copy of original instruction */
+struct arch_probe_insn {
+	probe_opcode_t *insn;
+	probes_handler_t *handler;
+	/* restore address after simulation */
+	unsigned long restore;
+};
+
+#ifdef CONFIG_KPROBES
+typedef u32 kprobe_opcode_t;
+struct arch_specific_insn {
+	struct arch_probe_insn api;
+};
+#endif
+
+#endif /* _ASM_RISCV_PROBES_H */
arch/riscv/kernel/Makefile +1 -0
index b355cf4..c3fff3e 100644
@@ -29,6 +29,7 @@ obj-y	+= riscv_ksyms.o
 obj-y	+= stacktrace.o
 obj-y	+= cacheinfo.o
 obj-y	+= patch.o
+obj-y	+= probes/
 obj-$(CONFIG_MMU) += vdso.o vdso/
 
 obj-$(CONFIG_RISCV_M_MODE)	+= clint.o traps_misaligned.o
arch/riscv/kernel/probes/Makefile new file +4 -0
new file mode 100644
index 00000000..8a39507
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_KPROBES)		+= kprobes.o decode-insn.o simulate-insn.o
+obj-$(CONFIG_KPROBES)		+= kprobes_trampoline.o
+CFLAGS_REMOVE_simulate-insn.o = $(CC_FLAGS_FTRACE)
arch/riscv/kernel/probes/decode-insn.c new file +48 -0

48 lines not on this page · raw message

arch/riscv/kernel/probes/decode-insn.h new file +18 -0
new file mode 100644
index 00000000..42269a7
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef _RISCV_KERNEL_KPROBES_DECODE_INSN_H
+#define _RISCV_KERNEL_KPROBES_DECODE_INSN_H
+
+#include <asm/sections.h>
+#include <asm/kprobes.h>
+
+enum probe_insn {
+	INSN_REJECTED,
+	INSN_GOOD_NO_SLOT,
+	INSN_GOOD,
+};
+
+enum probe_insn __kprobes
+riscv_probe_decode_insn(probe_opcode_t *addr, struct arch_probe_insn *asi);
+
+#endif /* _RISCV_KERNEL_KPROBES_DECODE_INSN_H */
arch/riscv/kernel/probes/kprobes.c new file +471 -0

471 lines not on this page · raw message

arch/riscv/kernel/probes/kprobes_trampoline.S new file +93 -0

93 lines not on this page · raw message

arch/riscv/kernel/probes/simulate-insn.c new file +85 -0

85 lines not on this page · raw message

arch/riscv/kernel/probes/simulate-insn.h new file +47 -0

47 lines not on this page · raw message

arch/riscv/kernel/traps.c +9 -0
index ecec177..ac2e786 100644
@@ -12,6 +12,7 @@
 #include <linux/signal.h>
 #include <linux/kdebug.h>
 #include <linux/uaccess.h>
+#include <linux/kprobes.h>
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/irq.h>
@@ -145,6 +146,14 @@ static inline unsigned long get_break_insn_length(unsigned long pc)
 
 asmlinkage __visible void do_trap_break(struct pt_regs *regs)
 {
+#ifdef CONFIG_KPROBES
+	if (kprobe_single_step_handler(regs))
+		return;
+
+	if (kprobe_breakpoint_handler(regs))
+		return;
+#endif
+
 	if (user_mode(regs))
 		force_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->epc);
 #ifdef CONFIG_KGDB
arch/riscv/mm/fault.c +4 -0
index ae7b7fe..da0c08c 100644
@@ -13,6 +13,7 @@
 #include <linux/perf_event.h>
 #include <linux/signal.h>
 #include <linux/uaccess.h>
+#include <linux/kprobes.h>
 
 #include <asm/pgalloc.h>
 #include <asm/ptrace.h>
@@ -40,6 +41,9 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	tsk = current;
 	mm = tsk->mm;
 
+	if (kprobe_page_fault(regs, cause))
+		return;
+
 	/*
 	 * Fault-in kernel-space virtual memory on-demand.
 	 * The 'reference' page table is init_mm.pgd.
Signature · 1 line
2.7.4
author maintainer
#6 lore raw

PATCH v2 5/6 riscv: Add uprobes supported

In reply to [email protected]

From: Guo Ren <[email protected]>

This patch adds support for uprobes on riscv architecture.

Just like kprobe, it support single-step and simulate instructions.

Cc Patrick Stählin <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Björn Töpel <[email protected]>
---
 arch/riscv/Kconfig                   |   3 +
 arch/riscv/include/asm/processor.h   |   1 +
 arch/riscv/include/asm/thread_info.h |   4 +-
 arch/riscv/include/asm/uprobes.h     |  40 ++++++++
 arch/riscv/kernel/probes/Makefile    |   1 +
 arch/riscv/kernel/probes/uprobes.c   | 186 +++++++++++++++++++++++++++++++++++
 arch/riscv/kernel/signal.c           |   3 +
 arch/riscv/kernel/traps.c            |  10 ++
 arch/riscv/mm/fault.c                |   7 ++
 9 files changed, 254 insertions(+), 1 deletion(-)
 create mode 100644 arch/riscv/include/asm/uprobes.h
 create mode 100644 arch/riscv/kernel/probes/uprobes.c
arch/riscv/Kconfig +3 -0
index a295f0b..f927a91 100644
@@ -146,6 +146,9 @@ config ARCH_WANT_GENERAL_HUGETLB
 config ARCH_SUPPORTS_DEBUG_PAGEALLOC
 	def_bool y
 
+config ARCH_SUPPORTS_UPROBES
+	def_bool y
+
 config SYS_SUPPORTS_HUGETLBFS
 	depends on MMU
 	def_bool y
arch/riscv/include/asm/processor.h +1 -0
index bdddcd5..3a24003 100644
@@ -34,6 +34,7 @@ struct thread_struct {
 	unsigned long sp;	/* Kernel mode stack */
 	unsigned long s[12];	/* s[0]: frame pointer */
 	struct __riscv_d_ext_state fstate;
+	unsigned long bad_cause;
 };
 
 #define INIT_THREAD {					\
arch/riscv/include/asm/thread_info.h +3 -1
index 1dd12a0..b3a7eb6 100644
@@ -76,6 +76,7 @@ struct thread_info {
 #define TIF_SYSCALL_TRACEPOINT  6       /* syscall tracepoint instrumentation */
 #define TIF_SYSCALL_AUDIT	7	/* syscall auditing */
 #define TIF_SECCOMP		8	/* syscall secure computing */
+#define TIF_UPROBE		9	/* uprobe breakpoint or singlestep */
 
 #define _TIF_SYSCALL_TRACE	(1 << TIF_SYSCALL_TRACE)
 #define _TIF_NOTIFY_RESUME	(1 << TIF_NOTIFY_RESUME)
@@ -84,9 +85,10 @@ struct thread_info {
 #define _TIF_SYSCALL_TRACEPOINT	(1 << TIF_SYSCALL_TRACEPOINT)
 #define _TIF_SYSCALL_AUDIT	(1 << TIF_SYSCALL_AUDIT)
 #define _TIF_SECCOMP		(1 << TIF_SECCOMP)
+#define _TIF_UPROBE		(1 << TIF_UPROBE)
 
 #define _TIF_WORK_MASK \
-	(_TIF_NOTIFY_RESUME | _TIF_SIGPENDING | _TIF_NEED_RESCHED)
+	(_TIF_NOTIFY_RESUME | _TIF_SIGPENDING | _TIF_NEED_RESCHED | _TIF_UPROBE)
 
 #define _TIF_SYSCALL_WORK \
 	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_TRACEPOINT | _TIF_SYSCALL_AUDIT | \
arch/riscv/include/asm/uprobes.h new file +40 -0
new file mode 100644
index 00000000..f2183e0
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _ASM_RISCV_UPROBES_H
+#define _ASM_RISCV_UPROBES_H
+
+#include <asm/probes.h>
+#include <asm/patch.h>
+#include <asm/bug.h>
+
+#define MAX_UINSN_BYTES		8
+
+#ifdef CONFIG_RISCV_ISA_C
+#define UPROBE_SWBP_INSN	__BUG_INSN_16
+#define UPROBE_SWBP_INSN_SIZE	2
+#else
+#define UPROBE_SWBP_INSN	__BUG_INSN_32
+#define UPROBE_SWBP_INSN_SIZE	4
+#endif
+#define UPROBE_XOL_SLOT_BYTES	MAX_UINSN_BYTES
+
+typedef u32 uprobe_opcode_t;
+
+struct arch_uprobe_task {
+	unsigned long   saved_cause;
+};
+
+struct arch_uprobe {
+	union {
+		u8 insn[MAX_UINSN_BYTES];
+		u8 ixol[MAX_UINSN_BYTES];
+	};
+	struct arch_probe_insn api;
+	unsigned long insn_size;
+	bool simulate;
+};
+
+bool uprobe_breakpoint_handler(struct pt_regs *regs);
+bool uprobe_single_step_handler(struct pt_regs *regs);
+
+#endif /* _ASM_RISCV_UPROBES_H */
arch/riscv/kernel/probes/Makefile +1 -0
index 8a39507..cb62991 100644
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_KPROBES)		+= kprobes.o decode-insn.o simulate-insn.o
 obj-$(CONFIG_KPROBES)		+= kprobes_trampoline.o
+obj-$(CONFIG_UPROBES)		+= uprobes.o decode-insn.o simulate-insn.o
 CFLAGS_REMOVE_simulate-insn.o = $(CC_FLAGS_FTRACE)
arch/riscv/kernel/probes/uprobes.c new file +186 -0

186 lines not on this page · raw message

arch/riscv/kernel/signal.c +3 -0
index 17ba190..a96db83b 100644
@@ -309,6 +309,9 @@ static void do_signal(struct pt_regs *regs)
 asmlinkage __visible void do_notify_resume(struct pt_regs *regs,
 					   unsigned long thread_info_flags)
 {
+	if (thread_info_flags & _TIF_UPROBE)
+		uprobe_notify_resume(regs);
+
 	/* Handle pending signal delivery */
 	if (thread_info_flags & _TIF_SIGPENDING)
 		do_signal(regs);
arch/riscv/kernel/traps.c +10 -0
index ac2e786..6981276 100644
@@ -76,6 +76,8 @@ void do_trap(struct pt_regs *regs, int signo, int code, unsigned long addr)
 static void do_trap_error(struct pt_regs *regs, int signo, int code,
 	unsigned long addr, const char *str)
 {
+	current->thread.bad_cause = regs->cause;
+
 	if (user_mode(regs)) {
 		do_trap(regs, signo, code, addr);
 	} else {
@@ -153,6 +155,14 @@ asmlinkage __visible void do_trap_break(struct pt_regs *regs)
 	if (kprobe_breakpoint_handler(regs))
 		return;
 #endif
+#ifdef CONFIG_UPROBES
+	if (uprobe_single_step_handler(regs))
+		return;
+
+	if (uprobe_breakpoint_handler(regs))
+		return;
+#endif
+	current->thread.bad_cause = regs->cause;
 
 	if (user_mode(regs))
 		force_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->epc);
arch/riscv/mm/fault.c +7 -0
index da0c08c..ac96d93 100644
@@ -170,11 +170,14 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	mmap_read_unlock(mm);
 	/* User mode accesses just cause a SIGSEGV */
 	if (user_mode(regs)) {
+		tsk->thread.bad_cause = cause;
 		do_trap(regs, SIGSEGV, code, addr);
 		return;
 	}
 
 no_context:
+	tsk->thread.bad_cause = cause;
+
 	/* Are we prepared to handle this kernel fault? */
 	if (fixup_exception(regs))
 		return;
@@ -195,6 +198,8 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	 * (which will retry the fault, or kill us if we got oom-killed).
 	 */
 out_of_memory:
+	tsk->thread.bad_cause = cause;
+
 	mmap_read_unlock(mm);
 	if (!user_mode(regs))
 		goto no_context;
@@ -202,6 +207,8 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	return;
 
 do_sigbus:
+	tsk->thread.bad_cause = cause;
+
 	mmap_read_unlock(mm);
 	/* Kernel mode? Handle exceptions or die */
 	if (!user_mode(regs))
Signature · 1 line
2.7.4
author maintainer
#7 lore raw

PATCH v2 6/6 riscv: Add KPROBES_ON_FTRACE supported

In reply to [email protected]

From: Guo Ren <[email protected]>

This patch adds support for kprobes on ftrace call sites to avoids much of the overhead with regular kprobes. Try it with simple steps:

1. Get _do_fork ftrace call site.
Dump of assembler code for function _do_fork:

   0xffffffe00020af64 <+0>:     addi    sp,sp,-128
   0xffffffe00020af66 <+2>:     sd      s0,112(sp)
   0xffffffe00020af68 <+4>:     sd      ra,120(sp)
   0xffffffe00020af6a <+6>:     addi    s0,sp,128
   0xffffffe00020af6c <+8>:     sd      s1,104(sp)
   0xffffffe00020af6e <+10>:    sd      s2,96(sp)
   0xffffffe00020af70 <+12>:    sd      s3,88(sp)
   0xffffffe00020af72 <+14>:    sd      s4,80(sp)
   0xffffffe00020af74 <+16>:    sd      s5,72(sp)
   0xffffffe00020af76 <+18>:    sd      s6,64(sp)
   0xffffffe00020af78 <+20>:    sd      s7,56(sp)
   0xffffffe00020af7a <+22>:    mv      s4,a0
   0xffffffe00020af7c <+24>:    mv      a0,ra
   0xffffffe00020af7e <+26>:    nop	<<<<<<<< here!
   0xffffffe00020af82 <+30>:    nop
   0xffffffe00020af86 <+34>:    ld      s3,0(s4)

2. Set _do_fork+26 as the kprobe.
echo 'p:myprobe _do_fork+26 dfd=%a0 filename=%a1 flags=%a2 mode=+4($stack)' > /sys/kernel/debug/tracing/kprobe_events echo 1 > /sys/kernel/debug/tracing/events/kprobes/enable
cat /sys/kernel/debug/tracing/trace
tracer: nop

entries-in-buffer/entries-written: 3/3 #P:1

                               _-----=> irqs-off
                              / _----=> need-resched
                             | / _---=> hardirq/softirq
                             || / _--=> preempt-depth
                             ||| /     delay
            TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
               | |       |   ||||       |         |
              sh-87    [000] ....   551.557031: myprobe: (_do_fork+0x1a/0x2e6) dfd=0xffffffe00020af7e filename=0xffffffe00020b34e flags=0xffffffe00101e7c0 mode=0x20af86ffffffe0

cat /sys/kernel/debug/kprobes/list
ffffffe00020af7e k _do_fork+0x1a [FTRACE]

                                       ^^^^^^
Cc Masami Hiramatsu <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Paul Walmsley <[email protected]>
Cc Björn Töpel <[email protected]>
Cc Zong Li <[email protected]>
Cc Pekka Enberg <[email protected]>
---
 arch/riscv/Kconfig                |  1 +
 arch/riscv/kernel/probes/Makefile |  1 +
 arch/riscv/kernel/probes/ftrace.c | 53 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+)
 create mode 100644 arch/riscv/kernel/probes/ftrace.c
arch/riscv/Kconfig +1 -0
index f927a91..91bfc6c 100644
@@ -58,6 +58,7 @@ config RISCV
 	select HAVE_FUTEX_CMPXCHG if FUTEX
 	select HAVE_GENERIC_VDSO if MMU && 64BIT
 	select HAVE_KPROBES
+	select HAVE_KPROBES_ON_FTRACE
 	select HAVE_KRETPROBES
 	select HAVE_PCI
 	select HAVE_PERF_EVENTS
arch/riscv/kernel/probes/Makefile +1 -0
index cb62991..7f0840d 100644
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_KPROBES)		+= kprobes.o decode-insn.o simulate-insn.o
 obj-$(CONFIG_KPROBES)		+= kprobes_trampoline.o
+obj-$(CONFIG_KPROBES_ON_FTRACE)	+= ftrace.o
 obj-$(CONFIG_UPROBES)		+= uprobes.o decode-insn.o simulate-insn.o
 CFLAGS_REMOVE_simulate-insn.o = $(CC_FLAGS_FTRACE)
arch/riscv/kernel/probes/ftrace.c new file +53 -0

53 lines not on this page · raw message

Signature · 1 line
2.7.4
maintainer
#8 lore raw

Re: [PATCH v2 6/6] riscv: Add KPROBES_ON_FTRACE supported

Hi Guo,

Quoted 23 lines · On Thu, 9 Jul 2020 02:19:14 +0000 [email protected] wrote:
arch/riscv/kernel/probes/ftrace.c new file +23 -0
+/* Ftrace callback handler for kprobes -- called under preepmt disabed */
+void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
+			   struct ftrace_ops *ops, struct pt_regs *regs)
+{
+	struct kprobe *p;
+	struct kprobe_ctlblk *kcb;
+
+	p = get_kprobe((kprobe_opcode_t *)ip);
+	if (unlikely(!p) || kprobe_disabled(p))
+		return;
+
+	kcb = get_kprobe_ctlblk();
+	if (kprobe_running()) {
+		kprobes_inc_nmissed_count(p);
+	} else {
+		/*
+		 * The regs->epc hasn't been saved by SAVE_ALL in mcount-dyn.S
+		 * So no need to resume it, just for kprobe handler.
+		 */
+		instruction_pointer_set(regs, ip);
+		__this_cpu_write(current_kprobe, p);
+		kcb->kprobe_status = KPROBE_HIT_ACTIVE;
+		if (!p->pre_handler || !p->pre_handler(p, regs)) {

arch/riscv/kernel/probes/ftrace.c line 37

Quoted 10 lines
arch/riscv/kernel/probes/ftrace.c new file +10 -0
+			/*
+			 * Emulate singlestep (and also recover regs->pc)
+			 * as if there is a nop
+			 */
+			instruction_pointer_set(regs,
+				(unsigned long)p->addr + MCOUNT_INSN_SIZE);
+			if (unlikely(p->post_handler)) {
+				kcb->kprobe_status = KPROBE_HIT_SSDONE;
+				p->post_handler(p, regs, 0);
+			}

Hmm, don't you need restoring the previous instruction pointer here? If you don't support modifying the instruction pointer in the handler, it must not be compatible with kprobes.

Now BPF function override and function error injection depends on this behevior, so could you consider to support it in the "ftrace" implementation at first? (And if it is enabled, you can enable the livepatch on RISCV too)

Thank you,

Quoted 19 lines
arch/riscv/kernel/probes/ftrace.c new file +16 -0
+		}
+
+		/*
+		 * If pre_handler returns !0, it changes regs->pc. We have to
+		 * skip emulating post_handler.
+		 */
+		__this_cpu_write(current_kprobe, NULL);
+	}
+}
+NOKPROBE_SYMBOL(kprobe_ftrace_handler);
+
+int arch_prepare_kprobe_ftrace(struct kprobe *p)
+{
+	p->ainsn.api.insn = NULL;
+	return 0;
+}
Signature · 1 line
2.7.4
Signature · 1 line
Masami Hiramatsu <[email protected]>
author maintainer
#9 lore raw

Re: [PATCH v2 6/6] riscv: Add KPROBES_ON_FTRACE supported

Thx Masami,

Quoted 33 lines · Masami Hiramatsu

Hi Guo,

Quoted 27 lines · On Thu, 9 Jul 2020 02:19:14 +0000 [email protected] wrote:
arch/riscv/kernel/probes/ftrace.c new file +27 -0
+/* Ftrace callback handler for kprobes -- called under preepmt disabed */
+void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
+                        struct ftrace_ops *ops, struct pt_regs *regs)
+{
+     struct kprobe *p;
+     struct kprobe_ctlblk *kcb;
+
+     p = get_kprobe((kprobe_opcode_t *)ip);
+     if (unlikely(!p) || kprobe_disabled(p))
+             return;
+
+     kcb = get_kprobe_ctlblk();
+     if (kprobe_running()) {
+             kprobes_inc_nmissed_count(p);
+     } else {
+             /*
+              * The regs->epc hasn't been saved by SAVE_ALL in mcount-dyn.S
+              * So no need to resume it, just for kprobe handler.
+              */
+             instruction_pointer_set(regs, ip);
+             __this_cpu_write(current_kprobe, p);
+             kcb->kprobe_status = KPROBE_HIT_ACTIVE;
+             if (!p->pre_handler || !p->pre_handler(p, regs)) {
+                     /*
+                      * Emulate singlestep (and also recover regs->pc)
+                      * as if there is a nop
+                      */
Quoted 8 lines

arch/riscv/kernel/probes/ftrace.c line 37

Quoted 6 lines
arch/riscv/kernel/probes/ftrace.c new file +6 -0
+                     instruction_pointer_set(regs,
+                             (unsigned long)p->addr + MCOUNT_INSN_SIZE);
+                     if (unlikely(p->post_handler)) {
+                             kcb->kprobe_status = KPROBE_HIT_SSDONE;
+                             p->post_handler(p, regs, 0);
+                     }

Hmm, don't you need restoring the previous instruction pointer here?

look at riscv mcount-dyn.S SAVE_ALL function, sp frame lay out like this:
-----------------------
| return address |
-----------------------
| frame pointer |
-----------------------
| pt_regs x1-x31|
-----------------------
It's not a complete pt_regs for the handler, so modifing regs->ip is no use.

Quoted 1 line

If you don't support modifying the instruction pointer in the handler,

We can modify ip like this if necessary:
*(unsigned long *)((unsigned long)regs + sizeof(struct pt_regs) + 8) = xxx;

Quoted 1 line

it must not be compatible with kprobes.

Why, can you show related codes? thank you very much.

Quoted 5 lines

Now BPF function override and function error injection depends on this behevior, so could you consider to support it in the "ftrace" implementation at first? (And if it is enabled, you can enable the livepatch on RISCV too)

Great message!

But can you show me codes that bpf and err-jnject using the behavior? Thx

I'll try to fix up it :)

Signature · 4 lines
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/
maintainer
#10 lore raw

Re: [PATCH v2 6/6] riscv: Add KPROBES_ON_FTRACE supported

Quoted 44 lines · On Sat, 11 Jul 2020 09:32:01 +0800 Guo Ren <[email protected]> wrote:

Thx Masami,

Quoted 33 lines · Masami Hiramatsu

Hi Guo,

Quoted 27 lines · On Thu, 9 Jul 2020 02:19:14 +0000 [email protected] wrote:
arch/riscv/kernel/probes/ftrace.c new file +27 -0
+/* Ftrace callback handler for kprobes -- called under preepmt disabed */
+void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
+                        struct ftrace_ops *ops, struct pt_regs *regs)
+{
+     struct kprobe *p;
+     struct kprobe_ctlblk *kcb;
+
+     p = get_kprobe((kprobe_opcode_t *)ip);
+     if (unlikely(!p) || kprobe_disabled(p))
+             return;
+
+     kcb = get_kprobe_ctlblk();
+     if (kprobe_running()) {
+             kprobes_inc_nmissed_count(p);
+     } else {
+             /*
+              * The regs->epc hasn't been saved by SAVE_ALL in mcount-dyn.S
+              * So no need to resume it, just for kprobe handler.
+              */
+             instruction_pointer_set(regs, ip);
+             __this_cpu_write(current_kprobe, p);
+             kcb->kprobe_status = KPROBE_HIT_ACTIVE;
+             if (!p->pre_handler || !p->pre_handler(p, regs)) {
+                     /*
+                      * Emulate singlestep (and also recover regs->pc)
+                      * as if there is a nop
+                      */
Quoted 8 lines

arch/riscv/kernel/probes/ftrace.c line 37

Quoted 6 lines
arch/riscv/kernel/probes/ftrace.c new file +6 -0
+                     instruction_pointer_set(regs,
+                             (unsigned long)p->addr + MCOUNT_INSN_SIZE);
+                     if (unlikely(p->post_handler)) {
+                             kcb->kprobe_status = KPROBE_HIT_SSDONE;
+                             p->post_handler(p, regs, 0);
+                     }

Hmm, don't you need restoring the previous instruction pointer here?

Quoted 9 lines

look at riscv mcount-dyn.S SAVE_ALL function, sp frame lay out like this:
-----------------------
| return address |
-----------------------
| frame pointer |
-----------------------
| pt_regs x1-x31|
-----------------------
It's not a complete pt_regs for the handler, so modifing regs->ip is no use.

Yes, that is same on x86. But ftrace regs_caller on x86-64 modifies the return address on the stack by regs->ip.

See arch/x86/kernel/ftrace_64.S:

-----
SYM_INNER_LABEL(ftrace_regs_call, SYM_L_GLOBAL)
call ftrace_stub

/* Copy flags back to SS, to restore them */ movq EFLAGS(%rsp), %rax
movq %rax, MCOUNT_REG_SIZE(%rsp)

/* Handlers can change the RIP */
movq RIP(%rsp), %rax
movq %rax, MCOUNT_REG_SIZE+8(%rsp)
-----

The last part update the return address on the stack.

Quoted 6 lines
Quoted 1 line

If you don't support modifying the instruction pointer in the handler,

We can modify ip like this if necessary:
*(unsigned long *)((unsigned long)regs + sizeof(struct pt_regs) + 8) = xxx;

Quoted 1 line

it must not be compatible with kprobes.

Why, can you show related codes? thank you very much.

The "Changing Execution Path" section in the Documentation/kprobes.txt said

-----
Since kprobes can probe into a running kernel code, it can change the register set, including instruction pointer.
-----

As you can see, this is the expected behavior on kprobes. Since user will not know the kprobes is working on a breakpoint or ftrace, we have to support this behavior transparently. So, could you enable this feature at first on ftrace@RISCV?

Quoted 9 lines
Quoted 6 lines

Now BPF function override and function error injection depends on this behevior, so could you consider to support it in the "ftrace" implementation at first? (And if it is enabled, you can enable the livepatch on RISCV too)

Great message!

But can you show me codes that bpf and err-jnject using the behavior? Thx

In kernel/fail_function.c

-----
static int fei_kprobe_handler(struct kprobe *kp, struct pt_regs *regs)

{
        struct fei_attr *attr = container_of(kp, struct fei_attr, kp);

        if (should_fail(&fei_fault_attr, 1)) {
                regs_set_return_value(regs, attr->retval);
                override_function_with_return(regs);
                return 1;
        }

        return 0;
}

-----

And follow the code related to CONFIG_BPF_KPROBE_OVERRIDE in kernel/trace/bpf_trace.c. (doing similar thing, but fail_function.c is simpler.)

Thank you,

Signature · 1 line
Masami Hiramatsu <[email protected]>
author maintainer
#11 lore raw

Re: [PATCH v2 6/6] riscv: Add KPROBES_ON_FTRACE supported

Hi Masami,

Quoted 67 lines · Masami Hiramatsu
Quoted 44 lines · On Sat, 11 Jul 2020 09:32:01 +0800 Guo Ren <[email protected]> wrote:

Thx Masami,

Quoted 33 lines · Masami Hiramatsu

Hi Guo,

Quoted 27 lines · On Thu, 9 Jul 2020 02:19:14 +0000 [email protected] wrote:
arch/riscv/kernel/probes/ftrace.c new file +27 -0
+/* Ftrace callback handler for kprobes -- called under preepmt disabed */
+void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
+                        struct ftrace_ops *ops, struct pt_regs *regs)
+{
+     struct kprobe *p;
+     struct kprobe_ctlblk *kcb;
+
+     p = get_kprobe((kprobe_opcode_t *)ip);
+     if (unlikely(!p) || kprobe_disabled(p))
+             return;
+
+     kcb = get_kprobe_ctlblk();
+     if (kprobe_running()) {
+             kprobes_inc_nmissed_count(p);
+     } else {
+             /*
+              * The regs->epc hasn't been saved by SAVE_ALL in mcount-dyn.S
+              * So no need to resume it, just for kprobe handler.
+              */
+             instruction_pointer_set(regs, ip);
+             __this_cpu_write(current_kprobe, p);
+             kcb->kprobe_status = KPROBE_HIT_ACTIVE;
+             if (!p->pre_handler || !p->pre_handler(p, regs)) {
+                     /*
+                      * Emulate singlestep (and also recover regs->pc)
+                      * as if there is a nop
+                      */
Quoted 8 lines

arch/riscv/kernel/probes/ftrace.c line 37

Quoted 6 lines
arch/riscv/kernel/probes/ftrace.c new file +6 -0
+                     instruction_pointer_set(regs,
+                             (unsigned long)p->addr + MCOUNT_INSN_SIZE);
+                     if (unlikely(p->post_handler)) {
+                             kcb->kprobe_status = KPROBE_HIT_SSDONE;
+                             p->post_handler(p, regs, 0);
+                     }

Hmm, don't you need restoring the previous instruction pointer here?

Quoted 9 lines

look at riscv mcount-dyn.S SAVE_ALL function, sp frame lay out like this:
-----------------------
| return address |
-----------------------
| frame pointer |
-----------------------
| pt_regs x1-x31|
-----------------------
It's not a complete pt_regs for the handler, so modifing regs->ip is no use.

Yes, that is same on x86. But ftrace regs_caller on x86-64 modifies the return address on the stack by regs->ip.

See arch/x86/kernel/ftrace_64.S:

-----
SYM_INNER_LABEL(ftrace_regs_call, SYM_L_GLOBAL)
call ftrace_stub

Quoted 8 lines

/* Copy flags back to SS, to restore them */ movq EFLAGS(%rsp), %rax
movq %rax, MCOUNT_REG_SIZE(%rsp)

/* Handlers can change the RIP */
movq RIP(%rsp), %rax
movq %rax, MCOUNT_REG_SIZE+8(%rsp)
-----

Yes, and in save_mcount_regs, it prepare the origin pt_regs RIP.

/* Move RIP to its proper location */
movq MCOUNT_REG_SIZE+\added(%rsp), %rdi
movq %rdi, RIP(%rsp)

I've fixed up riscv's: [1]
[1]: https://lore.kernel.org/linux-riscv/[email protected]/T/#u

Quoted 8 lines

The last part update the return address on the stack.

Quoted 5 lines
Quoted 1 line

If you don't support modifying the instruction pointer in the handler,

We can modify ip like this if necessary:
*(unsigned long *)((unsigned long)regs + sizeof(struct pt_regs) + 8) = xxx;

Quoted 1 line

it must not be compatible with kprobes.

Quoted 8 lines
Quoted 1 line

Why, can you show related codes? thank you very much.

The "Changing Execution Path" section in the Documentation/kprobes.txt said

-----
Since kprobes can probe into a running kernel code, it can change the register set, including instruction pointer.
-----

Got it, thx for correct me.

Quoted 5 lines

As you can see, this is the expected behavior on kprobes. Since user will not know the kprobes is working on a breakpoint or ftrace, we have to support this behavior transparently. So, could you enable this feature at first on ftrace@RISCV?

Ditto

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Now BPF function override and function error injection depends on this behevior, so could you consider to support it in the "ftrace" implementation at first? (And if it is enabled, you can enable the livepatch on RISCV too)

Quoted 5 lines
Quoted 3 lines

Great message!

But can you show me codes that bpf and err-jnject using the behavior? Thx

In kernel/fail_function.c

Nice tip, thx. And I've tested err-jnject: [2]
[2]: https://lore.kernel.org/linux-riscv/[email protected]/T/#md66e4e58f3463c1369e11ab9e5d646343f7d250d

Quoted 14 lines

-----
static int fei_kprobe_handler(struct kprobe *kp, struct pt_regs *regs)

{
        struct fei_attr *attr = container_of(kp, struct fei_attr, kp);

        if (should_fail(&fei_fault_attr, 1)) {
                regs_set_return_value(regs, attr->retval);
                override_function_with_return(regs);
                return 1;
        }

        return 0;
}
Quoted 5 lines

-----

And follow the code related to CONFIG_BPF_KPROBE_OVERRIDE in kernel/trace/bpf_trace.c. (doing similar thing, but fail_function.c is simpler.)

Nice tip, thx

--
Best Regards
Guo Ren

ML: https://lore.kernel.org/linux-csky/

maintainer
#12 lore raw

Re: [PATCH v2 6/6] riscv: Add KPROBES_ON_FTRACE supported

Hi Guo,

Quoted 78 lines · On Tue, 14 Jul 2020 07:47:20 +0800 Guo Ren <[email protected]> wrote:

Hi Masami,

Quoted 67 lines · Masami Hiramatsu
Quoted 44 lines · On Sat, 11 Jul 2020 09:32:01 +0800 Guo Ren <[email protected]> wrote:

Thx Masami,

Quoted 33 lines · Masami Hiramatsu

Hi Guo,

Quoted 27 lines · On Thu, 9 Jul 2020 02:19:14 +0000 [email protected] wrote:
arch/riscv/kernel/probes/ftrace.c new file +27 -0
+/* Ftrace callback handler for kprobes -- called under preepmt disabed */
+void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
+                        struct ftrace_ops *ops, struct pt_regs *regs)
+{
+     struct kprobe *p;
+     struct kprobe_ctlblk *kcb;
+
+     p = get_kprobe((kprobe_opcode_t *)ip);
+     if (unlikely(!p) || kprobe_disabled(p))
+             return;
+
+     kcb = get_kprobe_ctlblk();
+     if (kprobe_running()) {
+             kprobes_inc_nmissed_count(p);
+     } else {
+             /*
+              * The regs->epc hasn't been saved by SAVE_ALL in mcount-dyn.S
+              * So no need to resume it, just for kprobe handler.
+              */
+             instruction_pointer_set(regs, ip);
+             __this_cpu_write(current_kprobe, p);
+             kcb->kprobe_status = KPROBE_HIT_ACTIVE;
+             if (!p->pre_handler || !p->pre_handler(p, regs)) {
+                     /*
+                      * Emulate singlestep (and also recover regs->pc)
+                      * as if there is a nop
+                      */
Quoted 8 lines

arch/riscv/kernel/probes/ftrace.c line 37

Quoted 6 lines
arch/riscv/kernel/probes/ftrace.c new file +6 -0
+                     instruction_pointer_set(regs,
+                             (unsigned long)p->addr + MCOUNT_INSN_SIZE);
+                     if (unlikely(p->post_handler)) {
+                             kcb->kprobe_status = KPROBE_HIT_SSDONE;
+                             p->post_handler(p, regs, 0);
+                     }

Hmm, don't you need restoring the previous instruction pointer here?

Quoted 9 lines

look at riscv mcount-dyn.S SAVE_ALL function, sp frame lay out like this:
-----------------------
| return address |
-----------------------
| frame pointer |
-----------------------
| pt_regs x1-x31|
-----------------------
It's not a complete pt_regs for the handler, so modifing regs->ip is no use.

Yes, that is same on x86. But ftrace regs_caller on x86-64 modifies the return address on the stack by regs->ip.

See arch/x86/kernel/ftrace_64.S:

-----
SYM_INNER_LABEL(ftrace_regs_call, SYM_L_GLOBAL)
call ftrace_stub

Quoted 8 lines

/* Copy flags back to SS, to restore them */ movq EFLAGS(%rsp), %rax
movq %rax, MCOUNT_REG_SIZE(%rsp)

/* Handlers can change the RIP */
movq RIP(%rsp), %rax
movq %rax, MCOUNT_REG_SIZE+8(%rsp)
-----

Quoted 8 lines

Yes, and in save_mcount_regs, it prepare the origin pt_regs RIP.

/* Move RIP to its proper location */
movq MCOUNT_REG_SIZE+\added(%rsp), %rdi
movq %rdi, RIP(%rsp)

I've fixed up riscv's: [1]
[1]: https://lore.kernel.org/linux-riscv/[email protected]/T/#u

OK, looks good :)

Quoted 32 lines
Quoted 8 lines

The last part update the return address on the stack.

Quoted 5 lines
Quoted 1 line

If you don't support modifying the instruction pointer in the handler,

We can modify ip like this if necessary:
*(unsigned long *)((unsigned long)regs + sizeof(struct pt_regs) + 8) = xxx;

Quoted 1 line

it must not be compatible with kprobes.

Quoted 8 lines
Quoted 1 line

Why, can you show related codes? thank you very much.

The "Changing Execution Path" section in the Documentation/kprobes.txt said

-----
Since kprobes can probe into a running kernel code, it can change the register set, including instruction pointer.
-----

Got it, thx for correct me.

Quoted 5 lines

As you can see, this is the expected behavior on kprobes. Since user will not know the kprobes is working on a breakpoint or ftrace, we have to support this behavior transparently. So, could you enable this feature at first on ftrace@RISCV?

Ditto

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Now BPF function override and function error injection depends on this behevior, so could you consider to support it in the "ftrace" implementation at first? (And if it is enabled, you can enable the livepatch on RISCV too)

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

Great message!

But can you show me codes that bpf and err-jnject using the behavior? Thx

In kernel/fail_function.c

Nice tip, thx. And I've tested err-jnject: [2]
[2]: https://lore.kernel.org/linux-riscv/[email protected]/T/#md66e4e58f3463c1369e11ab9e5d646343f7d250d

Good! Thanks for porting it!

Quoted 27 lines
Quoted 15 lines

-----
static int fei_kprobe_handler(struct kprobe *kp, struct pt_regs *regs)

{
        struct fei_attr *attr = container_of(kp, struct fei_attr, kp);

        if (should_fail(&fei_fault_attr, 1)) {
                regs_set_return_value(regs, attr->retval);
                override_function_with_return(regs);
                return 1;
        }

        return 0;
}
Quoted 5 lines

-----

And follow the code related to CONFIG_BPF_KPROBE_OVERRIDE in kernel/trace/bpf_trace.c. (doing similar thing, but fail_function.c is simpler.)

Nice tip, thx

--
Best Regards
Guo Ren

ML: https://lore.kernel.org/linux-csky/

Signature · 1 line
Masami Hiramatsu <[email protected]>