Re: [PATCH v7 1/4] tracing: add __print_sym() to replace __print_symbolic()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] tracing: show_print_fmt() string parsing breaks if value contains ternary operator
--

commit deeb2ac9d2766875d27bd9a2d5d898875d56cc2a
Author: Johannes Berg <johannes.berg@intel.com>
tracing: add __print_sym() to replace __print_symbolic()

This commit introduces __print_sym() to replace the old __print_symbolic(),
making it more memory efficient and allowing dynamic updates. It relies on a
single dynamic list of items processed at the time userspace reads the
print_fmt, avoiding init-time fixups.

> diff --git a/include/trace/stages/stage7_class_define.h b/include/trace/stages/stage7_class_define.h
> --- a/include/trace/stages/stage7_class_define.h
> +++ b/include/trace/stages/stage7_class_define.h
> @@ -45,6 +45,9 @@
>  #define __event_in_softirq()	(REC->common_flags & 0x10)
>  #define __event_in_irq()	(REC->common_flags & 0x18)
>  
> +#undef __print_sym
> +#define __print_sym(value, symbol_id)	__print_sym(value:symbol_id)

[Severity: Medium]
Does this delimiter choice in the __print_sym() macro in
include/trace/stages/stage7_class_define.h cause issues for arbitrary C
expressions?

This macro transforms the arguments into a string format using a delimiter (:)
that is unsafe for arbitrary C expressions if parsed left-to-right. If a
kernel developer uses an expression containing a colon (such as a ternary
operator like a ? b : c) as the value argument, it seems the string
representation will contain multiple colons.

> +
>  /*
>   * The below is not executed in the kernel. It is only what is
>   * displayed in the print format for userspace to parse.

[ ... ]

> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -2109,6 +2109,102 @@ static void *f_next(struct seq_file *m, void *v, loff_t *pos)
[ ... ]
> +static void show_print_fmt(struct seq_file *m, struct trace_event_call *call)
> +{
[ ... ]
> +		if (in_print_sym && *ptr == ':') {
> +			const char *name;
> +
> +			ptr++;
> +			name = ptr;
> +			/* skip the name */
> +			while (*ptr && *ptr != ')')
> +				ptr++;

[Severity: Medium]
Will this parser in show_print_fmt() in kernel/trace/trace_events.c break
if the value expression contains a ternary operator?

It appears this logic naively splits at the first colon, destroying valid C
expressions like ternary operators that natively contain a colon. If a
tracepoint passes a ternary operator to __print_sym(), the parser incorrectly
assumes the first ':' encountered uniquely separates the value expression from
the symbol ID.

This would cause tracing tools to fail parsing the exported print_fmt, breaking
event decoding for userspace.

Could a backward search for ':' from the closing parenthesis safely isolate
the symbol ID?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922110955.363677-6-johannes@sipsolutions.net?part=1