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 <[email protected]>

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

Reply via email to