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? > 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
