On Fri, Aug 14, 2026 at 04:01:30PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> to consider:
> - [High] The local variable `spare_size` is left uninitialized via `goto
> read;` in `tracing_buffers_read()`, resulting in a kernel memory leak to
> userspace.
> --
>
> --- Patch [1]: [PATCH v6 1/2] tracing: Fix subbuf resize races with
> trace_pipe_raw readers ---
> commit bdc760fe7b1aa6eb8c8a8b8c38e9fd02d25051b6
> Author: Vincent Donnefort <[email protected]>
>
> tracing: Fix subbuf resize races with trace_pipe_raw readers
>
> Concurrent subbuffer resizes may crash trace_pipe_raw readers or leak
> uninitialized memory to userspace due to stale size values. This commit
> modifies ring_buffer_alloc_read_page() to handle resizing and introduces
> ring_buffer_read_page_size().
>
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 395238b2b715c..355278083b3fb 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> [ ... ]
> > @@ -7091,36 +7091,21 @@ ssize_t tracing_buffers_read(struct file *filp,
> > char __user *ubuf,
> > if (iter->snapshot && tracer_uses_snapshot(iter->tr->current_trace))
> > return -EBUSY;
> >
> > - page_size = ring_buffer_subbuf_size_get(iter->array_buffer->buffer);
> > -
> > - /* Make sure the spare matches the current sub buffer size */
> > - if (info->spare) {
> > - if (page_size != info->spare_size) {
> > - ring_buffer_free_read_page(iter->array_buffer->buffer,
> > - info->spare_cpu,
> > info->spare);
> > - info->spare = NULL;
> > - }
> > - }
> > +again:
> > + /* Do we have previous read data to read? */
> > + if (info->spare && (info->read <
> > ring_buffer_read_page_size(info->spare)))
> > + goto read;
> >
> > - if (!info->spare) {
> > - info->spare =
> > ring_buffer_alloc_read_page(iter->array_buffer->buffer,
> > - iter->cpu_file);
> > - if (IS_ERR(info->spare)) {
> > - ret = PTR_ERR(info->spare);
> > - info->spare = NULL;
> > - } else {
> > - info->spare_cpu = iter->cpu_file;
> > - info->spare_size = page_size;
> > - }
> > - }
> > - if (!info->spare)
> > + /* Make sure the read page order is aligned with the current buffer
> > subbuf order */
> > + ret = ring_buffer_alloc_read_page(iter->array_buffer->buffer,
> > iter->cpu_file,
> > + &info->spare);
> > + if (ret < 0)
> > return ret;
> >
> > - /* Do we have previous read data to read? */
> > - if (info->read < page_size)
> > - goto read;
> > + spare_size = ring_buffer_read_page_size(info->spare);
> > + info->read = spare_size;
> > + info->spare_cpu = iter->cpu_file;
> >
> > - again:
> > trace_access_lock(iter->cpu_file);
> > ret = ring_buffer_read_page(iter->array_buffer->buffer,
> > info->spare,
> [ ... ]
> > @@ -7146,8 +7133,9 @@ ssize_t tracing_buffers_read(struct file *filp, char
> > __user *ubuf,
> > }
> >
> > info->read = 0;
> > +
> > read:
> > - size = page_size - info->read;
> > + size = spare_size - info->read;
>
> [Severity: High]
> Can this regression leak uninitialized stack memory to userspace?
>
> If a userspace process does a partial read (consuming fewer bytes than a full
> subbuffer page), the next read syscall will take the goto read; branch at the
> top of tracing_buffers_read().
>
> When that branch is taken, spare_size is completely uninitialized stack
> memory.
>
> Could size end up being a large garbage value (or underflow if spare_size <
> info->read), causing copy_to_user() below to read beyond the bounds of
> info->spare and leak arbitrary kernel memory?
sigh... I hope I will eventually get this right!
again:
- /* Do we have previous read data to read? */
- if (info->spare && (info->read <
ring_buffer_read_page_size(info->spare)))
- goto read;
+ if (info->spare) {
+ spare_size = ring_buffer_read_page_size(info->spare);
+ /* Do we have previous read data to read? */
+ if (info->read < spare_size)
+ goto read;
+ }
This should do.
>
> > if (size > count)
> > size = count;
> > trace_data = ring_buffer_read_page_data(info->spare);
> > ret = copy_to_user(ubuf, trace_data + info->read, size);
>
> --
> Sashiko AI review ยท
> https://sashiko.dev/#/patchset/[email protected]?part=1
--
Vincent
