From b2a866975f6cd5859c746f1da39c8f5736c8def2 Mon Sep 17 00:00:00 2001 From: "Masami Hiramatsu (Google)" Date: Fri, 10 Nov 2023 19:59:03 +0900 Subject: Documentation: tracing: Add a note about argument and retval access Add a note about the argument and return value accecss will be best effort. Depending on the type, it will be passed via stack or a pair of the registers, but $argN and $retval only support the single register access. Link: https://lore.kernel.org/all/169556269377.146934.14829235476649685954.stgit@devnote2/ Suggested-by: Alexei Starovoitov Signed-off-by: Masami Hiramatsu (Google) --- Documentation/trace/fprobetrace.rst | 8 ++++++-- Documentation/trace/kprobetrace.rst | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/Documentation/trace/fprobetrace.rst b/Documentation/trace/fprobetrace.rst index 8e9bebcf0a2e..e35e6b18df40 100644 --- a/Documentation/trace/fprobetrace.rst +++ b/Documentation/trace/fprobetrace.rst @@ -59,8 +59,12 @@ Synopsis of fprobe-events and bitfield are supported. (\*1) This is available only when BTF is enabled. - (\*2) only for the probe on function entry (offs == 0). - (\*3) only for return probe. + (\*2) only for the probe on function entry (offs == 0). Note, this argument access + is best effort, because depending on the argument type, it may be passed on + the stack. But this only support the arguments via registers. + (\*3) only for return probe. Note that this is also best effort. Depending on the + return value type, it might be passed via a pair of registers. But this only + accesses one register. (\*4) this is useful for fetching a field of data structures. (\*5) "u" means user-space dereference. diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst index 8a2dfee38145..bf9cecb69fc9 100644 --- a/Documentation/trace/kprobetrace.rst +++ b/Documentation/trace/kprobetrace.rst @@ -61,8 +61,12 @@ Synopsis of kprobe_events (x8/x16/x32/x64), "char", "string", "ustring", "symbol", "symstr" and bitfield are supported. - (\*1) only for the probe on function entry (offs == 0). - (\*2) only for return probe. + (\*1) only for the probe on function entry (offs == 0). Note, this argument access + is best effort, because depending on the argument type, it may be passed on + the stack. But this only support the arguments via registers. + (\*2) only for return probe. Note that this is also best effort. Depending on the + return value type, it might be passed via a pair of registers. But this only + accesses one register. (\*3) this is useful for fetching a field of data structures. (\*4) "u" means user-space dereference. See :ref:`user_mem_access`. -- cgit v1.2.3 From 3afe73372966d4d8b40922d7229c8ecb1c2ca287 Mon Sep 17 00:00:00 2001 From: "wuqiang.matt" Date: Fri, 10 Nov 2023 19:59:04 +0900 Subject: lib: test_objpool: make global variables static Kernel test robot reported build warnings that structures g_ot_sync_ops, g_ot_async_ops and g_testcases should be static. These definitions are only used in test_objpool.c, so make them static Link: https://lore.kernel.org/all/20231108012248.313574-1-wuqiang.matt@bytedance.com/ Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202311071229.WGrWUjM1-lkp@intel.com/ Signed-off-by: wuqiang.matt Acked-by: Masami Hiramatsu (Google) Signed-off-by: Masami Hiramatsu (Google) --- lib/test_objpool.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/test_objpool.c b/lib/test_objpool.c index a94078402138..bfdb81599832 100644 --- a/lib/test_objpool.c +++ b/lib/test_objpool.c @@ -311,7 +311,7 @@ static void ot_fini_sync(struct ot_context *sop) ot_kfree(sop->test, sop, sizeof(*sop)); } -struct { +static struct { struct ot_context * (*init)(struct ot_test *oc); void (*fini)(struct ot_context *sop); } g_ot_sync_ops[] = { @@ -475,7 +475,7 @@ static struct ot_context *ot_init_async_m0(struct ot_test *test) return sop; } -struct { +static struct { struct ot_context * (*init)(struct ot_test *oc); void (*fini)(struct ot_context *sop); } g_ot_async_ops[] = { @@ -632,7 +632,7 @@ static int ot_start_async(struct ot_test *test) #define NODE_COMPACT sizeof(struct ot_node) #define NODE_VMALLOC (512) -struct ot_test g_testcases[] = { +static struct ot_test g_testcases[] = { /* sync & normal */ {0, 0, NODE_COMPACT, 1000, 0, 1, 0, 0, "sync: percpu objpool"}, -- cgit v1.2.3 From abc28463c81853e4fdf8d009f71b2a3ce62a6f40 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Fri, 10 Nov 2023 19:59:05 +0900 Subject: kprobes: unify kprobes_exceptions_nofify() prototypes Most architectures that support kprobes declare this function in their own asm/kprobes.h header and provide an override, but some are missing the prototype, which causes a warning for the __weak stub implementation: kernel/kprobes.c:1865:12: error: no previous prototype for 'kprobe_exceptions_notify' [-Werror=missing-prototypes] 1865 | int __weak kprobe_exceptions_notify(struct notifier_block *self, Move the prototype into linux/kprobes.h so it is visible to all the definitions. Link: https://lore.kernel.org/all/20231108125843.3806765-4-arnd@kernel.org/ Acked-by: Masami Hiramatsu (Google) Signed-off-by: Arnd Bergmann Signed-off-by: Masami Hiramatsu (Google) --- arch/arc/include/asm/kprobes.h | 3 --- arch/arm/include/asm/kprobes.h | 2 -- arch/arm64/include/asm/kprobes.h | 2 -- arch/mips/include/asm/kprobes.h | 2 -- arch/powerpc/include/asm/kprobes.h | 2 -- arch/s390/include/asm/kprobes.h | 2 -- arch/sh/include/asm/kprobes.h | 2 -- arch/sparc/include/asm/kprobes.h | 2 -- arch/x86/include/asm/kprobes.h | 2 -- include/linux/kprobes.h | 4 ++++ 10 files changed, 4 insertions(+), 19 deletions(-) diff --git a/arch/arc/include/asm/kprobes.h b/arch/arc/include/asm/kprobes.h index de1566e32cb8..68e8301c0df2 100644 --- a/arch/arc/include/asm/kprobes.h +++ b/arch/arc/include/asm/kprobes.h @@ -32,9 +32,6 @@ struct kprobe; void arch_remove_kprobe(struct kprobe *p); -int kprobe_exceptions_notify(struct notifier_block *self, - unsigned long val, void *data); - struct prev_kprobe { struct kprobe *kp; unsigned long status; diff --git a/arch/arm/include/asm/kprobes.h b/arch/arm/include/asm/kprobes.h index e26a278d301a..5b8dbf1b0be4 100644 --- a/arch/arm/include/asm/kprobes.h +++ b/arch/arm/include/asm/kprobes.h @@ -40,8 +40,6 @@ struct kprobe_ctlblk { void arch_remove_kprobe(struct kprobe *); int kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr); -int kprobe_exceptions_notify(struct notifier_block *self, - unsigned long val, void *data); /* optinsn template addresses */ extern __visible kprobe_opcode_t optprobe_template_entry[]; diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h index 05cd82eeca13..be7a3680dadf 100644 --- a/arch/arm64/include/asm/kprobes.h +++ b/arch/arm64/include/asm/kprobes.h @@ -37,8 +37,6 @@ struct kprobe_ctlblk { void arch_remove_kprobe(struct kprobe *); int kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr); -int kprobe_exceptions_notify(struct notifier_block *self, - unsigned long val, void *data); void __kretprobe_trampoline(void); void __kprobes *trampoline_probe_handler(struct pt_regs *regs); diff --git a/arch/mips/include/asm/kprobes.h b/arch/mips/include/asm/kprobes.h index 68b1e5d458cf..bc27d99c9436 100644 --- a/arch/mips/include/asm/kprobes.h +++ b/arch/mips/include/asm/kprobes.h @@ -71,8 +71,6 @@ struct kprobe_ctlblk { struct prev_kprobe prev_kprobe; }; -extern int kprobe_exceptions_notify(struct notifier_block *self, - unsigned long val, void *data); #endif /* CONFIG_KPROBES */ #endif /* _ASM_KPROBES_H */ diff --git a/arch/powerpc/include/asm/kprobes.h b/arch/powerpc/include/asm/kprobes.h index c8e4b4fd4e33..4525a9c68260 100644 --- a/arch/powerpc/include/asm/kprobes.h +++ b/arch/powerpc/include/asm/kprobes.h @@ -84,8 +84,6 @@ struct arch_optimized_insn { kprobe_opcode_t *insn; }; -extern int kprobe_exceptions_notify(struct notifier_block *self, - unsigned long val, void *data); extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr); extern int kprobe_handler(struct pt_regs *regs); extern int kprobe_post_handler(struct pt_regs *regs); diff --git a/arch/s390/include/asm/kprobes.h b/arch/s390/include/asm/kprobes.h index 21b9e5290c04..01f1682a73b7 100644 --- a/arch/s390/include/asm/kprobes.h +++ b/arch/s390/include/asm/kprobes.h @@ -73,8 +73,6 @@ struct kprobe_ctlblk { void arch_remove_kprobe(struct kprobe *p); int kprobe_fault_handler(struct pt_regs *regs, int trapnr); -int kprobe_exceptions_notify(struct notifier_block *self, - unsigned long val, void *data); #define flush_insn_slot(p) do { } while (0) diff --git a/arch/sh/include/asm/kprobes.h b/arch/sh/include/asm/kprobes.h index eeba83e0a7d2..65d4c3316a5b 100644 --- a/arch/sh/include/asm/kprobes.h +++ b/arch/sh/include/asm/kprobes.h @@ -46,8 +46,6 @@ struct kprobe_ctlblk { }; extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr); -extern int kprobe_exceptions_notify(struct notifier_block *self, - unsigned long val, void *data); extern int kprobe_handle_illslot(unsigned long pc); #else diff --git a/arch/sparc/include/asm/kprobes.h b/arch/sparc/include/asm/kprobes.h index 06c2bc767ef7..aec742cd898f 100644 --- a/arch/sparc/include/asm/kprobes.h +++ b/arch/sparc/include/asm/kprobes.h @@ -47,8 +47,6 @@ struct kprobe_ctlblk { struct prev_kprobe prev_kprobe; }; -int kprobe_exceptions_notify(struct notifier_block *self, - unsigned long val, void *data); int kprobe_fault_handler(struct pt_regs *regs, int trapnr); asmlinkage void __kprobes kprobe_trap(unsigned long trap_level, struct pt_regs *regs); diff --git a/arch/x86/include/asm/kprobes.h b/arch/x86/include/asm/kprobes.h index a2e9317aad49..5939694dfb28 100644 --- a/arch/x86/include/asm/kprobes.h +++ b/arch/x86/include/asm/kprobes.h @@ -113,8 +113,6 @@ struct kprobe_ctlblk { }; extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr); -extern int kprobe_exceptions_notify(struct notifier_block *self, - unsigned long val, void *data); extern int kprobe_int3_handler(struct pt_regs *regs); #else diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h index 365eb092e9c4..ab1da3142b06 100644 --- a/include/linux/kprobes.h +++ b/include/linux/kprobes.h @@ -445,6 +445,10 @@ int kprobe_get_kallsym(unsigned int symnum, unsigned long *value, char *type, int arch_kprobe_get_kallsym(unsigned int *symnum, unsigned long *value, char *type, char *sym); + +int kprobe_exceptions_notify(struct notifier_block *self, + unsigned long val, void *data); + #else /* !CONFIG_KPROBES: */ static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr) -- cgit v1.2.3 From ce51e6153f7781bcde0f8bb4c81d6fd85ee422e6 Mon Sep 17 00:00:00 2001 From: "Masami Hiramatsu (Google)" Date: Wed, 8 Nov 2023 21:12:39 +0900 Subject: tracing: fprobe-event: Fix to check tracepoint event and return Fix to check the tracepoint event is not valid with $retval. The commit 08c9306fc2e3 ("tracing/fprobe-event: Assume fprobe is a return event by $retval") introduced automatic return probe conversion with $retval. But since tracepoint event does not support return probe, $retval is not acceptable. Without this fix, ftracetest, tprobe_syntax_errors.tc fails; [22] Tracepoint probe event parser error log check [FAIL] ---- # tail 22-tprobe_syntax_errors.tc-log.mRKroL + ftrace_errlog_check trace_fprobe t kfree ^$retval dynamic_events + printf %s t kfree + wc -c + pos=8 + printf %s t kfree ^$retval + tr -d ^ + command=t kfree $retval + echo Test command: t kfree $retval Test command: t kfree $retval + echo ---- So 't kfree $retval' should fail (tracepoint doesn't support return probe) but passed it. Link: https://lore.kernel.org/all/169944555933.45057.12831706585287704173.stgit@devnote2/ Fixes: 08c9306fc2e3 ("tracing/fprobe-event: Assume fprobe is a return event by $retval") Cc: stable@vger.kernel.org Signed-off-by: Masami Hiramatsu (Google) --- kernel/trace/trace_fprobe.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c index 8bfe23af9c73..7d2ddbcfa377 100644 --- a/kernel/trace/trace_fprobe.c +++ b/kernel/trace/trace_fprobe.c @@ -927,11 +927,12 @@ static int parse_symbol_and_return(int argc, const char *argv[], for (i = 2; i < argc; i++) { tmp = strstr(argv[i], "$retval"); if (tmp && !isalnum(tmp[7]) && tmp[7] != '_') { + if (is_tracepoint) { + trace_probe_log_set_index(i); + trace_probe_log_err(tmp - argv[i], RETVAL_ON_PROBE); + return -EINVAL; + } *is_return = true; - /* - * NOTE: Don't check is_tracepoint here, because it will - * be checked when the argument is parsed. - */ break; } } -- cgit v1.2.3 From f032c53bea6d2057c14553832d846be2f151cfb2 Mon Sep 17 00:00:00 2001 From: Yujie Liu Date: Tue, 31 Oct 2023 12:13:05 +0800 Subject: tracing/kprobes: Fix the order of argument descriptions The order of descriptions should be consistent with the argument list of the function, so "kretprobe" should be the second one. int __kprobe_event_gen_cmd_start(struct dynevent_cmd *cmd, bool kretprobe, const char *name, const char *loc, ...) Link: https://lore.kernel.org/all/20231031041305.3363712-1-yujie.liu@intel.com/ Fixes: 2a588dd1d5d6 ("tracing: Add kprobe event command generation functions") Suggested-by: Mukesh Ojha Signed-off-by: Yujie Liu Reviewed-by: Mukesh Ojha Signed-off-by: Masami Hiramatsu (Google) --- kernel/trace/trace_kprobe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c index a3442db35670..52f8b537dd0a 100644 --- a/kernel/trace/trace_kprobe.c +++ b/kernel/trace/trace_kprobe.c @@ -1020,9 +1020,9 @@ EXPORT_SYMBOL_GPL(kprobe_event_cmd_init); /** * __kprobe_event_gen_cmd_start - Generate a kprobe event command from arg list * @cmd: A pointer to the dynevent_cmd struct representing the new event + * @kretprobe: Is this a return probe? * @name: The name of the kprobe event * @loc: The location of the kprobe event - * @kretprobe: Is this a return probe? * @...: Variable number of arg (pairs), one pair for each field * * NOTE: Users normally won't want to call this function directly, but -- cgit v1.2.3