The following series fixes some bugs and adding some error messages which are not handled. This also add some selftests which tests the new error messages.
Thank you,
---
Masami Hiramatsu (Google) (8): tracing: tprobe-events: Fix a memory leak when tprobe with $retval tracing: tprobe-events: Reject invalid tracepoint name tracing: fprobe-events: Log error for exceeding the number of entry args tracing: probe-events: Log errro for exceeding the number of arguments tracing: probe-events: Remove unused MAX_ARG_BUF_LEN macro selftests/ftrace: Expand the tprobe event test to check wrong format selftests/ftrace: Add new syntax error test selftests/ftrace: Add dynamic events argument limitation test case
kernel/trace/trace_eprobe.c | 2 + kernel/trace/trace_fprobe.c | 25 +++++++++++- kernel/trace/trace_kprobe.c | 5 ++ kernel/trace/trace_probe.h | 6 ++- kernel/trace/trace_uprobe.c | 9 +++- .../ftrace/test.d/dynevent/add_remove_tprobe.tc | 14 +++++++ .../ftrace/test.d/dynevent/dynevent_limitations.tc | 42 ++++++++++++++++++++ .../ftrace/test.d/dynevent/fprobe_syntax_errors.tc | 1 8 files changed, 98 insertions(+), 6 deletions(-) create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/dynevent_limitations.tc
-- Masami Hiramatsu (Google) mhiramat@kernel.org
From: Masami Hiramatsu (Google) mhiramat@kernel.org
Fix a memory leak when a tprobe is defined with $retval. This combination is not allowed, but the parse_symbol_and_return() does not free the *symbol which should not be used if it returns the error. Thus, it leaks the *symbol memory in that error path.
Fixes: ce51e6153f77 ("tracing: fprobe-event: Fix to check tracepoint event and return") Signed-off-by: Masami Hiramatsu (Google) mhiramat@kernel.org Cc: stable@vger.kernel.org --- kernel/trace/trace_fprobe.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c index b8f3c4ba309b..8826f44f69a4 100644 --- a/kernel/trace/trace_fprobe.c +++ b/kernel/trace/trace_fprobe.c @@ -1056,6 +1056,8 @@ static int parse_symbol_and_return(int argc, const char *argv[], if (is_tracepoint) { trace_probe_log_set_index(i); trace_probe_log_err(tmp - argv[i], RETVAL_ON_PROBE); + kfree(*symbol); + *symbol = NULL; return -EINVAL; } *is_return = true;
From: Masami Hiramatsu (Google) mhiramat@kernel.org
Commit 57a7e6de9e30 ("tracing/fprobe: Support raw tracepoints on future loaded modules") allows user to set a tprobe on non-exist tracepoint but it does not check the tracepoint name is acceptable. So it leads tprobe has a wrong character for events (e.g. with subsystem prefix). In this case, the event is not shown in the events directory.
Reject such invalid tracepoint name.
The tracepoint name must consist of alphabet or digit or '_'.
Fixes: 57a7e6de9e30 ("tracing/fprobe: Support raw tracepoints on future loaded modules") Signed-off-by: Masami Hiramatsu (Google) mhiramat@kernel.org Cc: stable@vger.kernel.org --- kernel/trace/trace_fprobe.c | 13 +++++++++++++ kernel/trace/trace_probe.h | 1 + 2 files changed, 14 insertions(+)
diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c index 8826f44f69a4..85f037dc1462 100644 --- a/kernel/trace/trace_fprobe.c +++ b/kernel/trace/trace_fprobe.c @@ -1049,6 +1049,19 @@ static int parse_symbol_and_return(int argc, const char *argv[], if (*is_return) return 0;
+ if (is_tracepoint) { + tmp = *symbol; + while (*tmp && (isalnum(*tmp) || *tmp == '_')) + tmp++; + if (*tmp) { + /* find a wrong character. */ + trace_probe_log_err(tmp - *symbol, BAD_TP_NAME); + kfree(*symbol); + *symbol = NULL; + return -EINVAL; + } + } + /* If there is $retval, this should be a return fprobe. */ for (i = 2; i < argc; i++) { tmp = strstr(argv[i], "$retval"); diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h index 5803e6a41570..fba3ede87054 100644 --- a/kernel/trace/trace_probe.h +++ b/kernel/trace/trace_probe.h @@ -481,6 +481,7 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call, C(NON_UNIQ_SYMBOL, "The symbol is not unique"), \ C(BAD_RETPROBE, "Retprobe address must be an function entry"), \ C(NO_TRACEPOINT, "Tracepoint is not found"), \ + C(BAD_TP_NAME, "Invalid character in tracepoint name"),\ C(BAD_ADDR_SUFFIX, "Invalid probed address suffix"), \ C(NO_GROUP_NAME, "Group name is not specified"), \ C(GROUP_TOO_LONG, "Group name is too long"), \
From: Masami Hiramatsu (Google) mhiramat@kernel.org
Add error message when the number of entry argument exceeds the maximum size of entry data. This is currently checked when registering fprobe, but in this case no error message is shown in the error_log file.
Fixes: 25f00e40ce79 ("tracing/probes: Support $argN in return probe (kprobe and fprobe)") Signed-off-by: Masami Hiramatsu (Google) mhiramat@kernel.org --- kernel/trace/trace_fprobe.c | 5 +++++ kernel/trace/trace_probe.h | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c index 85f037dc1462..e27305d31fc5 100644 --- a/kernel/trace/trace_fprobe.c +++ b/kernel/trace/trace_fprobe.c @@ -1230,6 +1230,11 @@ static int trace_fprobe_create_internal(int argc, const char *argv[], if (is_return && tf->tp.entry_arg) { tf->fp.entry_handler = trace_fprobe_entry_handler; tf->fp.entry_data_size = traceprobe_get_entry_data_size(&tf->tp); + if (ALIGN(tf->fp.entry_data_size, sizeof(long)) > MAX_FPROBE_DATA_SIZE) { + trace_probe_log_set_index(2); + trace_probe_log_err(0, TOO_MANY_EARGS); + return -E2BIG; + } }
ret = traceprobe_set_print_fmt(&tf->tp, diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h index fba3ede87054..c47ca002347a 100644 --- a/kernel/trace/trace_probe.h +++ b/kernel/trace/trace_probe.h @@ -545,7 +545,8 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call, C(NO_BTF_FIELD, "This field is not found."), \ C(BAD_BTF_TID, "Failed to get BTF type info."),\ C(BAD_TYPE4STR, "This type does not fit for string."),\ - C(NEED_STRING_TYPE, "$comm and immediate-string only accepts string type"), + C(NEED_STRING_TYPE, "$comm and immediate-string only accepts string type"),\ + C(TOO_MANY_EARGS, "Too many entry arguments specified"),
#undef C #define C(a, b) TP_ERR_##a
From: Masami Hiramatsu (Google) mhiramat@kernel.org
Add error message when the number of arguments exceeeds the limitation.
Signed-off-by: Masami Hiramatsu (Google) mhiramat@kernel.org --- kernel/trace/trace_eprobe.c | 2 ++ kernel/trace/trace_fprobe.c | 5 ++++- kernel/trace/trace_kprobe.c | 5 ++++- kernel/trace/trace_probe.h | 1 + kernel/trace/trace_uprobe.c | 9 +++++++-- 5 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c index 82fd637cfc19..af9fa0632b57 100644 --- a/kernel/trace/trace_eprobe.c +++ b/kernel/trace/trace_eprobe.c @@ -913,6 +913,8 @@ static int __trace_eprobe_create(int argc, const char *argv[]) }
if (argc - 2 > MAX_TRACE_ARGS) { + trace_probe_log_set_index(2); + trace_probe_log_err(0, TOO_MANY_ARGS); ret = -E2BIG; goto error; } diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c index e27305d31fc5..372936163c21 100644 --- a/kernel/trace/trace_fprobe.c +++ b/kernel/trace/trace_fprobe.c @@ -1201,8 +1201,11 @@ static int trace_fprobe_create_internal(int argc, const char *argv[], argc = new_argc; argv = new_argv; } - if (argc > MAX_TRACE_ARGS) + if (argc > MAX_TRACE_ARGS) { + trace_probe_log_set_index(2); + trace_probe_log_err(0, TOO_MANY_ARGS); return -E2BIG; + }
ret = traceprobe_expand_dentry_args(argc, argv, &dbuf); if (ret) diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c index d8d5f18a141a..8287b175667f 100644 --- a/kernel/trace/trace_kprobe.c +++ b/kernel/trace/trace_kprobe.c @@ -1007,8 +1007,11 @@ static int trace_kprobe_create_internal(int argc, const char *argv[], argc = new_argc; argv = new_argv; } - if (argc > MAX_TRACE_ARGS) + if (argc > MAX_TRACE_ARGS) { + trace_probe_log_set_index(2); + trace_probe_log_err(0, TOO_MANY_ARGS); return -E2BIG; + }
ret = traceprobe_expand_dentry_args(argc, argv, &dbuf); if (ret) diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h index c47ca002347a..6075afc8ea67 100644 --- a/kernel/trace/trace_probe.h +++ b/kernel/trace/trace_probe.h @@ -546,6 +546,7 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call, C(BAD_BTF_TID, "Failed to get BTF type info."),\ C(BAD_TYPE4STR, "This type does not fit for string."),\ C(NEED_STRING_TYPE, "$comm and immediate-string only accepts string type"),\ + C(TOO_MANY_ARGS, "Too many arguments are specified"), \ C(TOO_MANY_EARGS, "Too many entry arguments specified"),
#undef C diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index ccc762fbb69c..3386439ec9f6 100644 --- a/kernel/trace/trace_uprobe.c +++ b/kernel/trace/trace_uprobe.c @@ -562,8 +562,14 @@ static int __trace_uprobe_create(int argc, const char **argv)
if (argc < 2) return -ECANCELED; - if (argc - 2 > MAX_TRACE_ARGS) + + trace_probe_log_init("trace_uprobe", argc, argv); + + if (argc - 2 > MAX_TRACE_ARGS) { + trace_probe_log_set_index(2); + trace_probe_log_err(0, TOO_MANY_ARGS); return -E2BIG; + }
if (argv[0][1] == ':') event = &argv[0][2]; @@ -582,7 +588,6 @@ static int __trace_uprobe_create(int argc, const char **argv) return -ECANCELED; }
- trace_probe_log_init("trace_uprobe", argc, argv); trace_probe_log_set_index(1); /* filename is the 2nd argument */
*arg++ = '\0';
From: Masami Hiramatsu (Google) mhiramat@kernel.org
Commit 18b1e870a496 ("tracing/probes: Add $arg* meta argument for all function args") introduced MAX_ARG_BUF_LEN but it is not used. Remove it.
Fixes: 18b1e870a496 ("tracing/probes: Add $arg* meta argument for all function args") Signed-off-by: Masami Hiramatsu (Google) mhiramat@kernel.org --- kernel/trace/trace_probe.h | 1 - 1 file changed, 1 deletion(-)
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h index 6075afc8ea67..854e5668f5ee 100644 --- a/kernel/trace/trace_probe.h +++ b/kernel/trace/trace_probe.h @@ -36,7 +36,6 @@ #define MAX_BTF_ARGS_LEN 128 #define MAX_DENTRY_ARGS_LEN 256 #define MAX_STRING_SIZE PATH_MAX -#define MAX_ARG_BUF_LEN (MAX_TRACE_ARGS * MAX_ARG_NAME_LEN)
/* Reserved field names */ #define FIELD_STRING_IP "__probe_ip"
From: Masami Hiramatsu (Google) mhiramat@kernel.org
Expand the tprobe event test case to check wrong tracepoint format.
Signed-off-by: Masami Hiramatsu (Google) mhiramat@kernel.org --- .../ftrace/test.d/dynevent/add_remove_tprobe.tc | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_tprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_tprobe.tc index 155792eaeee5..f271c4238b72 100644 --- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_tprobe.tc +++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_tprobe.tc @@ -6,6 +6,7 @@ echo 0 > events/enable echo > dynamic_events
+SUBSYSTEM=kmem TRACEPOINT1=kmem_cache_alloc TRACEPOINT2=kmem_cache_free
@@ -24,4 +25,17 @@ grep -q myevent1 dynamic_events
echo > dynamic_events
+# auto naming check +echo "t $TRACEPOINT1" >> dynamic_events + +test -d events/tracepoints/$TRACEPOINT1 + +echo > dynamic_events + +# SUBSYSTEM is not supported +echo "t $SUBSYSTEM/$TRACEPOINT1" >> dynamic_events && exit_fail ||: +echo "t $SUBSYSTEM:$TRACEPOINT1" >> dynamic_events && exit_fail ||: +echo "t:myevent3 $SUBSYSTEM/$TRACEPOINT1" >> dynamic_events && exit_fail ||: +echo "t:myevent3 $SUBSYSTEM:$TRACEPOINT1" >> dynamic_events && exit_fail ||: + clear_trace
From: Masami Hiramatsu (Google) mhiramat@kernel.org
Add BAD_TP_NAME syntax error message check.
Signed-off-by: Masami Hiramatsu (Google) mhiramat@kernel.org --- .../ftrace/test.d/dynevent/fprobe_syntax_errors.tc | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc index c9425a34fae3..fee479295e2f 100644 --- a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc +++ b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc @@ -27,6 +27,7 @@ check_error 'f:^foo.1/bar vfs_read' # BAD_GROUP_NAME check_error 'f:^ vfs_read' # NO_EVENT_NAME check_error 'f:foo/^12345678901234567890123456789012345678901234567890123456789012345 vfs_read' # EVENT_TOO_LONG check_error 'f:foo/^bar.1 vfs_read' # BAD_EVENT_NAME +check_error 't kmem^/kfree' # BAD_TP_NAME
check_error 'f vfs_read ^$stack10000' # BAD_STACK_NUM
From: Masami Hiramatsu (Google) mhiramat@kernel.org
Add argument limitation test case for dynamic events. This is a boudary check for the maximum number of the probe event arguments.
Signed-off-by: Masami Hiramatsu (Google) mhiramat@kernel.org --- .../ftrace/test.d/dynevent/dynevent_limitations.tc | 42 ++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/dynevent_limitations.tc
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/dynevent_limitations.tc b/tools/testing/selftests/ftrace/test.d/dynevent/dynevent_limitations.tc new file mode 100644 index 000000000000..6b94b678741a --- /dev/null +++ b/tools/testing/selftests/ftrace/test.d/dynevent/dynevent_limitations.tc @@ -0,0 +1,42 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# description: Checking dynamic events limitations +# requires: dynamic_events "imm-value":README + +# Max arguments limitation +MAX_ARGS=128 +EXCEED_ARGS=$((MAX_ARGS + 1)) + +check_max_args() { # event_header + TEST_STRING=$1 + # Acceptable + for i in `seq 1 $MAX_ARGS`; do + TEST_STRING="$TEST_STRING \$i" + done + echo "$TEST_STRING" >> dynamic_events + echo > dynamic_events + # Error + TEST_STRING="$TEST_STRING \$EXCEED_ARGS" + ! echo "$TEST_STRING" >> dynamic_events + return 0 +} + +# Kprobe max args limitation +if grep -q "kprobe_events" README; then + check_max_args "p vfs_read" +fi + +# Fprobe max args limitation +if grep -q "f[:[<group>/][<event>]] <func-name>[%return] [<args>]" README; then + check_max_args "f vfs_read" +fi + +# Tprobe max args limitation +if grep -q "t[:[<group>/][<event>]] <tracepoint> [<args>]" README; then + check_max_args "t kfree" +fi + +# Uprobe max args limitation +if grep -q "uprobe_events" README; then + check_max_args "p /bin/sh:10" +fi
linux-kselftest-mirror@lists.linaro.org
-
Masami Hiramatsu (Google)