Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] tracing: Use-After-Free on temp_buffer and tracing_buffer_mask when tracer_alloc_buffers fails --
commit 74cf4963596c0d99d36452a0e504036b25685bcc Author: Xiang Gao <[email protected]> tracing: add ring-buffer memory usage statistics in tracefs This commit adds a tracefs file, trace_stats/memory_usage_kb, to report the memory consumed by tracing ring buffers. It calculates the memory usage across the global trace array, instances, and snapshot buffers using a new function, ring_buffer_memory_size(). > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index 8658cad53cb59..5a023018f612e 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c [ ... ] > @@ -5771,6 +5771,62 @@ tracing_total_entries_read(struct file *filp, char > __user *ubuf, > return simple_read_from_buffer(ubuf, cnt, ppos, buf, r); > } > > +struct trace_mem_stats { > + unsigned long buffers; > + unsigned long snapshot; > +}; > + > +static void > +trace_array_buffer_memory(struct trace_array *tr, int cpu, > + unsigned long *buffers, unsigned long *snapshot) > +{ > + if (tr->array_buffer.buffer) > + *buffers += ring_buffer_memory_size(tr->array_buffer.buffer, > cpu); > + > +#ifdef CONFIG_TRACER_SNAPSHOT > + if (tr->snapshot_buffer.buffer) > + *snapshot += > ring_buffer_memory_size(tr->snapshot_buffer.buffer, cpu); > +#endif > +} > + > +static struct trace_mem_stats trace_buffers_memory(void) > +{ > + struct trace_mem_stats stats = {}; > + struct trace_array *tr; > + int cpu; > + > + guard(mutex)(&trace_types_lock); > + > + list_for_each_entry(tr, &ftrace_trace_arrays, list) { > + for_each_tracing_cpu(cpu) [Severity: High] Does this code trigger a use-after-free if tracer_alloc_buffers() fails during early boot? If tracer_alloc_buffers() fails in kernel/trace/trace.c, it frees both tracing_buffer_mask and temp_buffer but leaves their global pointers dangling: kernel/trace/trace.c:tracer_alloc_buffers() { ... out_free_buffer_mask: ... ring_buffer_free(temp_buffer); free_cpumask_var(tracing_buffer_mask); ... } Because trace_stats/memory_usage_kb is created unconditionally by tracer_init_tracefs_work_func(), and its file operations (trace_mem_fops) do not check the tracing_disabled flag, a local user can still read the file. When trace_mem_show() executes trace_buffers_memory(), the for_each_tracing_cpu() macro will dereference the dangling tracing_buffer_mask. > + trace_array_buffer_memory(tr, cpu, &stats.buffers, > + &stats.snapshot); > + } > + > + /* > + * temp_buffer is allocated in tracer_alloc_buffers() and is never > + * attached to a trace array. It temporarily holds event data for > + * triggers when tracing is off. Account for its pages too. > + */ > + if (temp_buffer) { > + for_each_tracing_cpu(cpu) > + stats.buffers += ring_buffer_memory_size(temp_buffer, > cpu); [Severity: High] Does this also cause a use-after-free on temp_buffer in trace_buffers_memory()? If tracer_alloc_buffers() fails, temp_buffer is freed but its pointer is never set to NULL on the error path. This branch will evaluate the dangling pointer as true and pass it to ring_buffer_memory_size(), which will dereference the freed memory. > + } > + > + return stats; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
