Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] `ring_buffer_alloc_read_page()` returning a cached page with an 
obsolete order causes `tracing_buffers_read()` to permanently return EOF to 
userspace.
--

--- Patch [3]: [PATCH v5 03/10] ring-buffer: Make cpu_buffer::free_page a 
buffer_data_read_page ---
commit 19f06657067d8dbcf108f6bacc6885d031739166
Author: Vincent Donnefort <[email protected]>

    ring-buffer: Make cpu_buffer::free_page a buffer_data_read_page

This commit changes the free_page cache to store the page order alongside
the page address. This fixes a mismatch when a cached reader page is
discarded after a concurrent ring buffer resize changes the global
subbuf_order.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index ec4f5a0c93e85..b6fa258aafe2c 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
[ ... ]
> @@ -6962,9 +6962,9 @@ ring_buffer_alloc_read_page(struct trace_buffer 
> *buffer, int cpu)
>       local_irq_save(flags);
>       arch_spin_lock(&cpu_buffer->lock);
>  
> -     if (cpu_buffer->free_page) {
> -             bpage->data = cpu_buffer->free_page;
> -             cpu_buffer->free_page = NULL;
> +     if (cpu_buffer->free_page.data) {
> +             *bpage = cpu_buffer->free_page;
> +             cpu_buffer->free_page.data = NULL;

[Severity: High]
Does this struct assignment cause tracing_buffers_read() to permanently
return EOF to userspace during a concurrent resize?

When a resize happens in ring_buffer_subbuf_order_set(),
buffer->subbuf_order is updated first:

        buffer->subbuf_order = order;

        /* Make sure all new buffers are allocated, before deleting... */
        for_each_buffer_cpu(buffer, cpu) {

This opens a large race window before cpu_buffer->free_page.data is
cleared for each CPU.

If a user reads from trace_pipe_raw during this window,
ring_buffer_alloc_read_page() allocates bpage and initially sets
bpage->order to the new buffer->subbuf_order. However, the struct
assignment *bpage = cpu_buffer->free_page copies the old free_page,
overwriting bpage->order with the old order.

Later in tracing_buffers_read(), the caller caches this mismatched page
and sets spare_size to the new size:

                } else {
                        info->spare_cpu = iter->cpu_file;
                        info->spare_size = page_size;
                }

Because the orders don't match, ring_buffer_read_page() rejects the page:

        if (data_page->order != buffer->subbuf_order)
                return -1;

This returns -1, which tracing_buffers_read() treats as EOF. Subsequent
reads see info->spare_size == page_size, reuse the bad page, and
permanently return EOF.

>       }
>  
>       arch_spin_unlock(&cpu_buffer->lock);

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=3

Reply via email to