diff mbox series

[v5,24/34] fprobe: Use ftrace_regs in fprobe entry handler

Message ID 170290538307.220107.14964448383069008953.stgit@devnote2 (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-22 fail Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-31 fail Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-32 fail Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-38 fail Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-39 fail Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-40 fail Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-13 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-15 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 fail Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 fail Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-18 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / veristat

Commit Message

Masami Hiramatsu (Google) Dec. 18, 2023, 1:16 p.m. UTC 
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

This allows fprobes to be available with CONFIG_DYNAMIC_FTRACE_WITH_ARGS
instead of CONFIG_DYNAMIC_FTRACE_WITH_REGS, then we can enable fprobe
on arm64.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Acked-by: Florent Revest <revest@chromium.org>
---
 Changes from previous series: NOTHING, just forward ported.
---
 include/linux/fprobe.h          |    2 +-
 kernel/trace/Kconfig            |    3 ++-
 kernel/trace/bpf_trace.c        |   10 +++++++---
 kernel/trace/fprobe.c           |    4 ++--
 kernel/trace/trace_fprobe.c     |    6 +++++-
 lib/test_fprobe.c               |    4 ++--
 samples/fprobe/fprobe_example.c |    2 +-
 7 files changed, 20 insertions(+), 11 deletions(-)

Comments

Jiri Olsa Dec. 19, 2023, 1:23 p.m. UTC | #1 
On Mon, Dec 18, 2023 at 10:16:23PM +0900, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> This allows fprobes to be available with CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> instead of CONFIG_DYNAMIC_FTRACE_WITH_REGS, then we can enable fprobe
> on arm64.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Acked-by: Florent Revest <revest@chromium.org>

this change breaks kprobe multi bpf tests (crash below), which are
partially fixed by [1] later on, but I think we have to keep
bisecting crash free

it looks like the rethook will get wrong pointer.. I'm still trying
to digest the whole thing, so I might have some updates later ;-)

jirka


[1] fprobe: Rewrite fprobe on function-graph tracer
---
Dec 19 13:50:04 qemu kernel: BUG: kernel NULL pointer dereference, address: 0000000000000098
Dec 19 13:50:04 qemu kernel: #PF: supervisor read access in kernel mode
Dec 19 13:50:04 qemu kernel: #PF: error_code(0x0000) - not-present page
Dec 19 13:50:04 qemu kernel: PGD 10955f067 P4D 10955f067 PUD 103113067 PMD 0 
Dec 19 13:50:04 qemu kernel: Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC KASAN NOPTI
Dec 19 13:50:04 qemu kernel: CPU: 1 PID: 747 Comm: test_progs Tainted: G    B      OE      6.7.0-rc3+ #194 85bc8297edbc7f21acfc743dabbd52cac073a6bf
Dec 19 13:50:04 qemu kernel: Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014
Dec 19 13:50:04 qemu kernel: RIP: 0010:arch_rethook_prepare+0x18/0x60
Dec 19 13:50:04 qemu kernel: Code: 1f 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 41 55 41 54 55 48 89 f5 53 48 89 fb 48 8d be 98 00 00 00 e8 68 8f 59 >
Dec 19 13:50:04 qemu kernel: RSP: 0018:ffff888125f97a88 EFLAGS: 00010286
Dec 19 13:50:04 qemu kernel: RAX: 0000000000000001 RBX: ffff88818a231410 RCX: ffffffff812190b6
Dec 19 13:50:04 qemu kernel: RDX: fffffbfff0c42e95 RSI: 0000000000000008 RDI: ffffffff862174a0
Dec 19 13:50:04 qemu kernel: RBP: 0000000000000000 R08: 0000000000000001 R09: fffffbfff0c42e94
Dec 19 13:50:04 qemu kernel: R10: ffffffff862174a7 R11: 0000000000000000 R12: ffff88818a231420
Dec 19 13:50:04 qemu kernel: R13: ffffffff8283ee8e R14: ffff88818a231410 R15: fffffffffffffff7
Dec 19 13:50:04 qemu kernel: FS:  00007ff8a16cfd00(0000) GS:ffff88842c600000(0000) knlGS:0000000000000000
Dec 19 13:50:05 qemu kernel: CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Dec 19 13:50:05 qemu kernel: CR2: 0000000000000098 CR3: 000000010633c005 CR4: 0000000000770ef0
Dec 19 13:50:05 qemu kernel: PKRU: 55555554
Dec 19 13:50:05 qemu kernel: Call Trace:
Dec 19 13:50:05 qemu kernel:  <TASK>
Dec 19 13:50:05 qemu kernel:  ? __die+0x1f/0x70
Dec 19 13:50:05 qemu kernel:  ? page_fault_oops+0x215/0x620
Dec 19 13:50:05 qemu kernel:  ? rcu_is_watching+0x34/0x60
Dec 19 13:50:05 qemu kernel:  ? __pfx_page_fault_oops+0x10/0x10
Dec 19 13:50:05 qemu kernel:  ? asm_sysvec_apic_timer_interrupt+0x16/0x20
Dec 19 13:50:05 qemu kernel:  ? do_user_addr_fault+0x4b3/0x910
Dec 19 13:50:05 qemu kernel:  ? exc_page_fault+0x77/0x130
Dec 19 13:50:05 qemu kernel:  ? asm_exc_page_fault+0x22/0x30
Dec 19 13:50:05 qemu kernel:  ? bpf_prog_test_run_tracing+0x1ce/0x2d0
Dec 19 13:50:05 qemu kernel:  ? add_taint+0x26/0x90
Dec 19 13:50:05 qemu kernel:  ? arch_rethook_prepare+0x18/0x60
Dec 19 13:50:05 qemu kernel:  ? arch_rethook_prepare+0x18/0x60
Dec 19 13:50:05 qemu kernel:  ? bpf_prog_test_run_tracing+0x1ce/0x2d0
Dec 19 13:50:05 qemu kernel:  rethook_hook+0x1e/0x50
Dec 19 13:50:05 qemu kernel:  ? __pfx_bpf_fentry_test1+0x10/0x10
Dec 19 13:50:05 qemu kernel:  ? bpf_prog_test_run_tracing+0x1ce/0x2d0
Dec 19 13:50:05 qemu kernel:  fprobe_handler+0x1ca/0x350
Dec 19 13:50:05 qemu kernel:  ? __pfx_bpf_fentry_test1+0x10/0x10
Dec 19 13:50:05 qemu kernel:  arch_ftrace_ops_list_func+0x143/0x2e0
Dec 19 13:50:05 qemu kernel:  ? bpf_prog_test_run_tracing+0x1ce/0x2d0
Dec 19 13:50:05 qemu kernel:  ftrace_call+0x5/0x44
Dec 19 13:50:05 qemu kernel:  ? __pfx_lock_release+0x10/0x10
Dec 19 13:50:05 qemu kernel:  ? rcu_is_watching+0x34/0x60
Dec 19 13:50:05 qemu kernel:  ? bpf_prog_test_run_tracing+0xcd/0x2d0
Dec 19 13:50:05 qemu kernel:  ? bpf_fentry_test1+0x5/0x10
Dec 19 13:50:05 qemu kernel:  ? rcu_is_watching+0x34/0x60
Dec 19 13:50:05 qemu kernel:  bpf_fentry_test1+0x5/0x10
Dec 19 13:50:05 qemu kernel:  bpf_prog_test_run_tracing+0x1ce/0x2d0
Dec 19 13:50:05 qemu kernel:  ? __pfx_lock_release+0x10/0x10
Dec 19 13:50:05 qemu kernel:  ? __pfx_bpf_prog_test_run_tracing+0x10/0x10
Dec 19 13:50:05 qemu kernel:  ? __pfx_lock_release+0x10/0x10
Dec 19 13:50:05 qemu kernel:  ? __fget_light+0xdf/0x100
Dec 19 13:50:05 qemu kernel:  ? __bpf_prog_get+0x107/0x150
Dec 19 13:50:05 qemu kernel:  __sys_bpf+0x552/0x2ef0
Dec 19 13:50:05 qemu kernel:  ? rcu_is_watching+0x34/0x60
Dec 19 13:50:05 qemu kernel:  ? __pfx___sys_bpf+0x10/0x10
Dec 19 13:50:05 qemu kernel:  ? __pfx_lock_release+0x10/0x10
Dec 19 13:50:05 qemu kernel:  ? vfs_write+0x1fa/0x740
Dec 19 13:50:05 qemu kernel:  ? rcu_is_watching+0x34/0x60
Dec 19 13:50:05 qemu kernel:  ? rcu_is_watching+0x34/0x60
Dec 19 13:50:05 qemu kernel:  ? lockdep_hardirqs_on_prepare+0xe/0x250
Dec 19 13:50:05 qemu kernel:  ? seqcount_lockdep_reader_access.constprop.0+0x105/0x120
Dec 19 13:50:05 qemu kernel:  ? seqcount_lockdep_reader_access.constprop.0+0xb2/0x120
Dec 19 13:50:05 qemu kernel:  __x64_sys_bpf+0x44/0x60
Dec 19 13:50:05 qemu kernel:  do_syscall_64+0x3f/0xf0
Dec 19 13:50:05 qemu kernel:  entry_SYSCALL_64_after_hwframe+0x6e/0x76
Dec 19 13:50:05 qemu kernel: RIP: 0033:0x7ff8a1897b4d
Dec 19 13:50:05 qemu kernel: Code: c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f >
Dec 19 13:50:05 qemu kernel: RSP: 002b:00007fff34f7d158 EFLAGS: 00000206 ORIG_RAX: 0000000000000141
Dec 19 13:50:05 qemu kernel: RAX: ffffffffffffffda RBX: 00007ff8a19aa000 RCX: 00007ff8a1897b4d
Dec 19 13:50:05 qemu kernel: RDX: 0000000000000050 RSI: 00007fff34f7d190 RDI: 000000000000000a
Dec 19 13:50:05 qemu kernel: RBP: 00007fff34f7d170 R08: 0000000000000000 R09: 00007fff34f7d190
Dec 19 13:50:05 qemu kernel: R10: 0000000000000064 R11: 0000000000000206 R12: 0000000000000004
Dec 19 13:50:05 qemu kernel: R13: 0000000000000000 R14: 00007ff8a19df000 R15: 0000000000e56db0
Dec 19 13:50:05 qemu kernel:  </TASK>
Dec 19 13:50:05 qemu kernel: Modules linked in: bpf_testmod(OE) intel_rapl_msr intel_rapl_common crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_inte>
Dec 19 13:50:05 qemu kernel: CR2: 0000000000000098
Dec 19 13:50:05 qemu kernel: ---[ end trace 0000000000000000 ]---
Jiri Olsa Dec. 19, 2023, 1:23 p.m. UTC | #2 
On Mon, Dec 18, 2023 at 10:16:23PM +0900, Masami Hiramatsu (Google) wrote:

SNIP

> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 84e8a0f6e4e0..d3f8745d8ead 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2503,7 +2503,7 @@ static int __init bpf_event_init(void)
>  fs_initcall(bpf_event_init);
>  #endif /* CONFIG_MODULES */
>  
> -#ifdef CONFIG_FPROBE
> +#if defined(CONFIG_FPROBE) && defined(CONFIG_DYNAMIC_FTRACE_WITH_REGS)
>  struct bpf_kprobe_multi_link {
>  	struct bpf_link link;
>  	struct fprobe fp;
> @@ -2733,10 +2733,14 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
>  
>  static int
>  kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
> -			  unsigned long ret_ip, struct pt_regs *regs,
> +			  unsigned long ret_ip, struct ftrace_regs *fregs,
>  			  void *data)
>  {
>  	struct bpf_kprobe_multi_link *link;
> +	struct pt_regs *regs = ftrace_get_regs(fregs);
> +
> +	if (!regs)
> +		return 0;
>  
>  	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
>  	kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
> @@ -3008,7 +3012,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
>  	kvfree(cookies);
>  	return err;
>  }
> -#else /* !CONFIG_FPROBE */
> +#else /* !CONFIG_FPROBE || !CONFIG_DYNAMIC_FTRACE_WITH_REGS */
>  int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
>  {
>  	return -EOPNOTSUPP;
> diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> index 6cd2a4e3afb8..f12569494d8a 100644
> --- a/kernel/trace/fprobe.c
> +++ b/kernel/trace/fprobe.c
> @@ -46,7 +46,7 @@ static inline void __fprobe_handler(unsigned long ip, unsigned long parent_ip,
>  	}
>  
>  	if (fp->entry_handler)
> -		ret = fp->entry_handler(fp, ip, parent_ip, ftrace_get_regs(fregs), entry_data);
> +		ret = fp->entry_handler(fp, ip, parent_ip, fregs, entry_data);
>  
>  	/* If entry_handler returns !0, nmissed is not counted. */
>  	if (rh) {
> @@ -182,7 +182,7 @@ static void fprobe_init(struct fprobe *fp)
>  		fp->ops.func = fprobe_kprobe_handler;
>  	else
>  		fp->ops.func = fprobe_handler;
> -	fp->ops.flags |= FTRACE_OPS_FL_SAVE_REGS;
> +	fp->ops.flags |= FTRACE_OPS_FL_SAVE_ARGS;

so with this change you move to ftrace_caller trampoline,
but we need ftrace_regs_caller right?

otherwise the (!regs) check in kprobe_multi_link_handler
will be allways true IIUC

jirka

>  }
>  
>  static int fprobe_init_rethook(struct fprobe *fp, int num)
> diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
> index 7d2ddbcfa377..ef6b36fd05ae 100644
> --- a/kernel/trace/trace_fprobe.c
> +++ b/kernel/trace/trace_fprobe.c
> @@ -320,12 +320,16 @@ NOKPROBE_SYMBOL(fexit_perf_func);
>  #endif	/* CONFIG_PERF_EVENTS */
>  
>  static int fentry_dispatcher(struct fprobe *fp, unsigned long entry_ip,
> -			     unsigned long ret_ip, struct pt_regs *regs,
> +			     unsigned long ret_ip, struct ftrace_regs *fregs,
>  			     void *entry_data)
>  {
>  	struct trace_fprobe *tf = container_of(fp, struct trace_fprobe, fp);
> +	struct pt_regs *regs = ftrace_get_regs(fregs);
>  	int ret = 0;
>  
> +	if (!regs)
> +		return 0;
> +
>  	if (trace_probe_test_flag(&tf->tp, TP_FLAG_TRACE))
>  		fentry_trace_func(tf, entry_ip, regs);
>  #ifdef CONFIG_PERF_EVENTS
> diff --git a/lib/test_fprobe.c b/lib/test_fprobe.c
> index 24de0e5ff859..ff607babba18 100644
> --- a/lib/test_fprobe.c
> +++ b/lib/test_fprobe.c
> @@ -40,7 +40,7 @@ static noinline u32 fprobe_selftest_nest_target(u32 value, u32 (*nest)(u32))
>  
>  static notrace int fp_entry_handler(struct fprobe *fp, unsigned long ip,
>  				    unsigned long ret_ip,
> -				    struct pt_regs *regs, void *data)
> +				    struct ftrace_regs *fregs, void *data)
>  {
>  	KUNIT_EXPECT_FALSE(current_test, preemptible());
>  	/* This can be called on the fprobe_selftest_target and the fprobe_selftest_target2 */
> @@ -81,7 +81,7 @@ static notrace void fp_exit_handler(struct fprobe *fp, unsigned long ip,
>  
>  static notrace int nest_entry_handler(struct fprobe *fp, unsigned long ip,
>  				      unsigned long ret_ip,
> -				      struct pt_regs *regs, void *data)
> +				      struct ftrace_regs *fregs, void *data)
>  {
>  	KUNIT_EXPECT_FALSE(current_test, preemptible());
>  	return 0;
> diff --git a/samples/fprobe/fprobe_example.c b/samples/fprobe/fprobe_example.c
> index 64e715e7ed11..1545a1aac616 100644
> --- a/samples/fprobe/fprobe_example.c
> +++ b/samples/fprobe/fprobe_example.c
> @@ -50,7 +50,7 @@ static void show_backtrace(void)
>  
>  static int sample_entry_handler(struct fprobe *fp, unsigned long ip,
>  				unsigned long ret_ip,
> -				struct pt_regs *regs, void *data)
> +				struct ftrace_regs *fregs, void *data)
>  {
>  	if (use_trace)
>  		/*
>
Masami Hiramatsu (Google) Dec. 19, 2023, 10:51 p.m. UTC | #3 
On Tue, 19 Dec 2023 14:23:48 +0100
Jiri Olsa <olsajiri@gmail.com> wrote:

> On Mon, Dec 18, 2023 at 10:16:23PM +0900, Masami Hiramatsu (Google) wrote:
> 
> SNIP
> 
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 84e8a0f6e4e0..d3f8745d8ead 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2503,7 +2503,7 @@ static int __init bpf_event_init(void)
> >  fs_initcall(bpf_event_init);
> >  #endif /* CONFIG_MODULES */
> >  
> > -#ifdef CONFIG_FPROBE
> > +#if defined(CONFIG_FPROBE) && defined(CONFIG_DYNAMIC_FTRACE_WITH_REGS)
> >  struct bpf_kprobe_multi_link {
> >  	struct bpf_link link;
> >  	struct fprobe fp;
> > @@ -2733,10 +2733,14 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
> >  
> >  static int
> >  kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
> > -			  unsigned long ret_ip, struct pt_regs *regs,
> > +			  unsigned long ret_ip, struct ftrace_regs *fregs,
> >  			  void *data)
> >  {
> >  	struct bpf_kprobe_multi_link *link;
> > +	struct pt_regs *regs = ftrace_get_regs(fregs);
> > +
> > +	if (!regs)
> > +		return 0;
> >  
> >  	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
> >  	kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
> > @@ -3008,7 +3012,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> >  	kvfree(cookies);
> >  	return err;
> >  }
> > -#else /* !CONFIG_FPROBE */
> > +#else /* !CONFIG_FPROBE || !CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> >  int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> >  {
> >  	return -EOPNOTSUPP;
> > diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> > index 6cd2a4e3afb8..f12569494d8a 100644
> > --- a/kernel/trace/fprobe.c
> > +++ b/kernel/trace/fprobe.c
> > @@ -46,7 +46,7 @@ static inline void __fprobe_handler(unsigned long ip, unsigned long parent_ip,
> >  	}
> >  
> >  	if (fp->entry_handler)
> > -		ret = fp->entry_handler(fp, ip, parent_ip, ftrace_get_regs(fregs), entry_data);
> > +		ret = fp->entry_handler(fp, ip, parent_ip, fregs, entry_data);
> >  
> >  	/* If entry_handler returns !0, nmissed is not counted. */
> >  	if (rh) {
> > @@ -182,7 +182,7 @@ static void fprobe_init(struct fprobe *fp)
> >  		fp->ops.func = fprobe_kprobe_handler;
> >  	else
> >  		fp->ops.func = fprobe_handler;
> > -	fp->ops.flags |= FTRACE_OPS_FL_SAVE_REGS;
> > +	fp->ops.flags |= FTRACE_OPS_FL_SAVE_ARGS;
> 
> so with this change you move to ftrace_caller trampoline,
> but we need ftrace_regs_caller right?

Yes, that's right.

> 
> otherwise the (!regs) check in kprobe_multi_link_handler
> will be allways true IIUC

Ah, OK. So until we move to fgraph [28/34], keep this flag SAVE_REGS
then kprobe_multi test will pass.

OK, let me keep it so.

Thank you!

> 
> jirka
> 
> >  }
> >  
> >  static int fprobe_init_rethook(struct fprobe *fp, int num)
> > diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
> > index 7d2ddbcfa377..ef6b36fd05ae 100644
> > --- a/kernel/trace/trace_fprobe.c
> > +++ b/kernel/trace/trace_fprobe.c
> > @@ -320,12 +320,16 @@ NOKPROBE_SYMBOL(fexit_perf_func);
> >  #endif	/* CONFIG_PERF_EVENTS */
> >  
> >  static int fentry_dispatcher(struct fprobe *fp, unsigned long entry_ip,
> > -			     unsigned long ret_ip, struct pt_regs *regs,
> > +			     unsigned long ret_ip, struct ftrace_regs *fregs,
> >  			     void *entry_data)
> >  {
> >  	struct trace_fprobe *tf = container_of(fp, struct trace_fprobe, fp);
> > +	struct pt_regs *regs = ftrace_get_regs(fregs);
> >  	int ret = 0;
> >  
> > +	if (!regs)
> > +		return 0;
> > +
> >  	if (trace_probe_test_flag(&tf->tp, TP_FLAG_TRACE))
> >  		fentry_trace_func(tf, entry_ip, regs);
> >  #ifdef CONFIG_PERF_EVENTS
> > diff --git a/lib/test_fprobe.c b/lib/test_fprobe.c
> > index 24de0e5ff859..ff607babba18 100644
> > --- a/lib/test_fprobe.c
> > +++ b/lib/test_fprobe.c
> > @@ -40,7 +40,7 @@ static noinline u32 fprobe_selftest_nest_target(u32 value, u32 (*nest)(u32))
> >  
> >  static notrace int fp_entry_handler(struct fprobe *fp, unsigned long ip,
> >  				    unsigned long ret_ip,
> > -				    struct pt_regs *regs, void *data)
> > +				    struct ftrace_regs *fregs, void *data)
> >  {
> >  	KUNIT_EXPECT_FALSE(current_test, preemptible());
> >  	/* This can be called on the fprobe_selftest_target and the fprobe_selftest_target2 */
> > @@ -81,7 +81,7 @@ static notrace void fp_exit_handler(struct fprobe *fp, unsigned long ip,
> >  
> >  static notrace int nest_entry_handler(struct fprobe *fp, unsigned long ip,
> >  				      unsigned long ret_ip,
> > -				      struct pt_regs *regs, void *data)
> > +				      struct ftrace_regs *fregs, void *data)
> >  {
> >  	KUNIT_EXPECT_FALSE(current_test, preemptible());
> >  	return 0;
> > diff --git a/samples/fprobe/fprobe_example.c b/samples/fprobe/fprobe_example.c
> > index 64e715e7ed11..1545a1aac616 100644
> > --- a/samples/fprobe/fprobe_example.c
> > +++ b/samples/fprobe/fprobe_example.c
> > @@ -50,7 +50,7 @@ static void show_backtrace(void)
> >  
> >  static int sample_entry_handler(struct fprobe *fp, unsigned long ip,
> >  				unsigned long ret_ip,
> > -				struct pt_regs *regs, void *data)
> > +				struct ftrace_regs *fregs, void *data)
> >  {
> >  	if (use_trace)
> >  		/*
> >
diff mbox series

Patch

diff --git a/include/linux/fprobe.h b/include/linux/fprobe.h
index 3e03758151f4..36c0595f7b93 100644
--- a/include/linux/fprobe.h
+++ b/include/linux/fprobe.h
@@ -35,7 +35,7 @@  struct fprobe {
 	int			nr_maxactive;
 
 	int (*entry_handler)(struct fprobe *fp, unsigned long entry_ip,
-			     unsigned long ret_ip, struct pt_regs *regs,
+			     unsigned long ret_ip, struct ftrace_regs *regs,
 			     void *entry_data);
 	void (*exit_handler)(struct fprobe *fp, unsigned long entry_ip,
 			     unsigned long ret_ip, struct pt_regs *regs,
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 308b3bec01b1..805d72ab77c6 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -290,7 +290,7 @@  config DYNAMIC_FTRACE_WITH_ARGS
 config FPROBE
 	bool "Kernel Function Probe (fprobe)"
 	depends on FUNCTION_TRACER
-	depends on DYNAMIC_FTRACE_WITH_REGS
+	depends on DYNAMIC_FTRACE_WITH_REGS || DYNAMIC_FTRACE_WITH_ARGS
 	depends on HAVE_RETHOOK
 	select RETHOOK
 	default n
@@ -675,6 +675,7 @@  config FPROBE_EVENTS
 	select TRACING
 	select PROBE_EVENTS
 	select DYNAMIC_EVENTS
+	depends on DYNAMIC_FTRACE_WITH_REGS
 	default y
 	help
 	  This allows user to add tracing events on the function entry and
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 84e8a0f6e4e0..d3f8745d8ead 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2503,7 +2503,7 @@  static int __init bpf_event_init(void)
 fs_initcall(bpf_event_init);
 #endif /* CONFIG_MODULES */
 
-#ifdef CONFIG_FPROBE
+#if defined(CONFIG_FPROBE) && defined(CONFIG_DYNAMIC_FTRACE_WITH_REGS)
 struct bpf_kprobe_multi_link {
 	struct bpf_link link;
 	struct fprobe fp;
@@ -2733,10 +2733,14 @@  kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
 
 static int
 kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
-			  unsigned long ret_ip, struct pt_regs *regs,
+			  unsigned long ret_ip, struct ftrace_regs *fregs,
 			  void *data)
 {
 	struct bpf_kprobe_multi_link *link;
+	struct pt_regs *regs = ftrace_get_regs(fregs);
+
+	if (!regs)
+		return 0;
 
 	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
 	kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
@@ -3008,7 +3012,7 @@  int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 	kvfree(cookies);
 	return err;
 }
-#else /* !CONFIG_FPROBE */
+#else /* !CONFIG_FPROBE || !CONFIG_DYNAMIC_FTRACE_WITH_REGS */
 int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
 {
 	return -EOPNOTSUPP;
diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
index 6cd2a4e3afb8..f12569494d8a 100644
--- a/kernel/trace/fprobe.c
+++ b/kernel/trace/fprobe.c
@@ -46,7 +46,7 @@  static inline void __fprobe_handler(unsigned long ip, unsigned long parent_ip,
 	}
 
 	if (fp->entry_handler)
-		ret = fp->entry_handler(fp, ip, parent_ip, ftrace_get_regs(fregs), entry_data);
+		ret = fp->entry_handler(fp, ip, parent_ip, fregs, entry_data);
 
 	/* If entry_handler returns !0, nmissed is not counted. */
 	if (rh) {
@@ -182,7 +182,7 @@  static void fprobe_init(struct fprobe *fp)
 		fp->ops.func = fprobe_kprobe_handler;
 	else
 		fp->ops.func = fprobe_handler;
-	fp->ops.flags |= FTRACE_OPS_FL_SAVE_REGS;
+	fp->ops.flags |= FTRACE_OPS_FL_SAVE_ARGS;
 }
 
 static int fprobe_init_rethook(struct fprobe *fp, int num)
diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
index 7d2ddbcfa377..ef6b36fd05ae 100644
--- a/kernel/trace/trace_fprobe.c
+++ b/kernel/trace/trace_fprobe.c
@@ -320,12 +320,16 @@  NOKPROBE_SYMBOL(fexit_perf_func);
 #endif	/* CONFIG_PERF_EVENTS */
 
 static int fentry_dispatcher(struct fprobe *fp, unsigned long entry_ip,
-			     unsigned long ret_ip, struct pt_regs *regs,
+			     unsigned long ret_ip, struct ftrace_regs *fregs,
 			     void *entry_data)
 {
 	struct trace_fprobe *tf = container_of(fp, struct trace_fprobe, fp);
+	struct pt_regs *regs = ftrace_get_regs(fregs);
 	int ret = 0;
 
+	if (!regs)
+		return 0;
+
 	if (trace_probe_test_flag(&tf->tp, TP_FLAG_TRACE))
 		fentry_trace_func(tf, entry_ip, regs);
 #ifdef CONFIG_PERF_EVENTS
diff --git a/lib/test_fprobe.c b/lib/test_fprobe.c
index 24de0e5ff859..ff607babba18 100644
--- a/lib/test_fprobe.c
+++ b/lib/test_fprobe.c
@@ -40,7 +40,7 @@  static noinline u32 fprobe_selftest_nest_target(u32 value, u32 (*nest)(u32))
 
 static notrace int fp_entry_handler(struct fprobe *fp, unsigned long ip,
 				    unsigned long ret_ip,
-				    struct pt_regs *regs, void *data)
+				    struct ftrace_regs *fregs, void *data)
 {
 	KUNIT_EXPECT_FALSE(current_test, preemptible());
 	/* This can be called on the fprobe_selftest_target and the fprobe_selftest_target2 */
@@ -81,7 +81,7 @@  static notrace void fp_exit_handler(struct fprobe *fp, unsigned long ip,
 
 static notrace int nest_entry_handler(struct fprobe *fp, unsigned long ip,
 				      unsigned long ret_ip,
-				      struct pt_regs *regs, void *data)
+				      struct ftrace_regs *fregs, void *data)
 {
 	KUNIT_EXPECT_FALSE(current_test, preemptible());
 	return 0;
diff --git a/samples/fprobe/fprobe_example.c b/samples/fprobe/fprobe_example.c
index 64e715e7ed11..1545a1aac616 100644
--- a/samples/fprobe/fprobe_example.c
+++ b/samples/fprobe/fprobe_example.c
@@ -50,7 +50,7 @@  static void show_backtrace(void)
 
 static int sample_entry_handler(struct fprobe *fp, unsigned long ip,
 				unsigned long ret_ip,
-				struct pt_regs *regs, void *data)
+				struct ftrace_regs *fregs, void *data)
 {
 	if (use_trace)
 		/*