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 <[email protected]> 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/[email protected]?part=1
