Re: [PATCH v6 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:
- [High] tracing: unaligned access in ftrace_sym_defs section causes kernel panic
--

commit e2d306a70156a518ace125ae399dac289ad0b9ff
Author: Johannes Berg <johannes.berg@intel.com>

tracing: add __print_sym() to replace __print_symbolic()

The way __print_symbolic() works is limited and inefficient in multiple ways.
This commit introduces __print_sym() to replace it, requiring only a single
list of items created by TRACE_DEFINE_SYM_LIST() and doing the fixup when
userspace reads the print_fmt.

> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index b2988aa12f664..e48327fafdfe8 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -276,8 +276,10 @@
>  	. = ALIGN(8);							\
>  	BOUNDED_SECTION(_ftrace_events)					\
>  	BOUNDED_SECTION_BY(_ftrace_eval_map, _ftrace_eval_maps)
> +#define FTRACE_SYM_DEFS()	BOUNDED_SECTION(_ftrace_sym_defs)

[Severity: High]
Does this missing . = ALIGN(8); before BOUNDED_SECTION(_ftrace_sym_defs) in
include/asm-generic/vmlinux.lds.h within FTRACE_SYM_DEFS() cause unaligned
addresses to be captured? Since FTRACE_SYM_DEFS follows TRACEPOINT_STR() which
contains 1-byte aligned strings, the linker might assign an unaligned location
counter to __start_ftrace_sym_defs before adding padding.

>  #else
>  #define FTRACE_EVENTS()
> +#define FTRACE_SYM_DEFS()
>  #endif

[ ... ]

> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 9dbc2441763b1..f14646ba9b22d 100644
> --- 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)
>  		return node;
>  }
>  
> +extern struct trace_sym_def *__start_ftrace_sym_defs[];
> +extern struct trace_sym_def *__stop_ftrace_sym_defs[];
> +
> +/* note: @name is not NUL-terminated */
> +static void show_sym_list(struct seq_file *m, struct trace_event_call *call,
> +			  const char *name, unsigned int name_len)
> +{
> +	struct trace_sym_def **sym_defs;
> +	unsigned int n_sym_defs, i;
> +
> +	if ((call->flags & TRACE_EVENT_FL_DYNAMIC) || !call->module) {
> +		sym_defs = __start_ftrace_sym_defs;
> +		n_sym_defs = __stop_ftrace_sym_defs - __start_ftrace_sym_defs;
> +	} else {

[ ... ]

> +
> +	for (i = 0; i < n_sym_defs; i++) {
> +		unsigned int sym_len;
> +
> +		if (!sym_defs[i])
> +			continue;
> +		if (sym_defs[i]->system != call->class->system)

[Severity: High]
If __start_ftrace_sym_defs was given an unaligned address in the linker script,
could this regression in show_sym_list() in kernel/trace/trace_events.c
dereference padding bytes as partial pointers and cause a kernel panic?
This seems reachable by any user with read access to the tracefs format
file for events using __print_sym.

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