Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] trace: Transient NULL pointer deref in tracing_set_tracer() due to non-atomic update --
commit 8e34d0e2d1517b818cf24f9067ddfaa0e4b47cd2 Author: Hao Ge <[email protected]> tracing: Fix crash in print_graph_function_event() This commit attempts to fix a NULL pointer dereference in print_graph_function_event() by ensuring tr->current_trace_flags is never NULL. It provides a fallback to a static empty no_tracer_flags when a selected tracer has no flags defined. v1 Link: https://lore.kernel.org/all/[email protected]/ > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index e4a490d3d08c8..fba9521d25115 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c [ ... ] > @@ -1319,6 +1325,8 @@ static int run_tracer_selftest(struct tracer *type) > > tr->current_trace = type; > tr->current_trace_flags = type->flags ? : type->default_flags; > + if (!tr->current_trace_flags) > + tr->current_trace_flags = &no_tracer_flags; [Severity: High] Does this non-atomic double assignment expose a transient NULL pointer to concurrent lockless readers? A concurrent reader might fetch tr->current_trace_flags during the brief window between the first store (which writes NULL if the tracer lacks flags) and the second store. Could we compute the new flags pointer into a local variable first, and then assign it to tr->current_trace_flags in a single store, perhaps using WRITE_ONCE()? [ ... ] > @@ -5090,6 +5098,8 @@ int tracing_set_tracer(struct trace_array *tr, const > char *buf) > } > > tr->current_trace_flags = t->flags ? : t->tracer->flags; > + if (!tr->current_trace_flags) > + tr->current_trace_flags = &no_tracer_flags; [Severity: High] Can this similarly result in a regression with a transient NULL pointer dereference? While tracing_set_tracer() holds trace_types_lock, readers accessing the trace file (via seq_read() -> s_show() -> print_trace_line() -> print_graph_function_event()) iterate locklessly. If a concurrent reader observes the intermediate NULL value, it will dereference it (e.g., tr->current_trace_flags->val), leading to the exact kernel panic this patch is meant to fix. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
