tracing: probes: Fixes and enhancing error logs

[PATCH 2/8] tracing: tprobe-events: Reject invalid tracepoint name

Posted by Masami Hiramatsu (Google) 9 months, 3 weeks ago

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"),		\

Re: [PATCH 2/8] tracing: tprobe-events: Reject invalid tracepoint name

Posted by Markus Elfring 9 months, 3 weeks ago

…
> +++ 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) {
…
> +			kfree(*symbol);
> +			*symbol = NULL;
> +			return -EINVAL;
> +		}
> +	}
…

May a bit of exception handling code be shared by an additional jump target?
Will another goto chain become helpful here?

Regards,
Markus

Re: [PATCH 2/8] tracing: tprobe-events: Reject invalid tracepoint name

Posted by Steven Rostedt 9 months, 3 weeks ago

On Wed, 26 Feb 2025 15:18:54 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> 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

Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>

-- Steve


> ---
>  kernel/trace/trace_fprobe.c |   13 +++++++++++++
>  kernel/trace/trace_probe.h  |    1 +
>  2 files changed, 14 insertions(+)