On Wed, Aug 12, 2026 at 03:47:11PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
> to consider:
> - [Critical] Integer truncation of `nr_pages_to_update` allows a massive
> buffer resize to bypass memory allocation checks, directly triggering an
> out-of-bounds page removal loop causing a critical kernel Use-After-Free
> (UAF).
> - [High] Multiplication overflow in `subbuf_size * nr_pages` evaluates as an
> `unsigned int` when allocating huge persistent ring buffers, destroying trace
> data isolation.
> --
>
> --- Patch [9]: [PATCH v4 9/9] ring-buffer: Make nr_pages unsigned int ---
> commit cb1365ebdbbed7daa3a685ae1d6be7a4167052dd
> Author: Vincent Donnefort <[email protected]>
> Date: Wed Aug 12 15:33:10 2026 +0000
>
> ring-buffer: Make nr_pages unsigned int
>
> nr_pages is an int or unsigned int almost everywhere already. Also, all
> the meta-data ring_buffer_desc, ring_buffer_cpu_meta and
> trace_buffer_meta allowing to share information about the ring buffer
> are already capping this value to 32-bits.
>
> Make ring_buffer_per_cpu::nr_pages unsigned and align all the users to
> it. As a side effect, this makes ring_buffer_per_cpu slightly smaller.
>
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index 990a904cefe85..2fccb950e593c 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> [ ... ]
> > @@ -561,7 +561,7 @@ struct ring_buffer_per_cpu {
> > struct ring_buffer_remote *remote;
> >
> > /* ring buffer pages to update, > 0 to add, < 0 to remove */
> > - long nr_pages_to_update;
> > + int nr_pages_to_update;
> > struct list_head new_pages; /* new pages to add */
> > struct work_struct update_pages_work;
> > struct completion update_done;
>
> [Severity: Critical]
> Could this type change lead to an out-of-bounds page removal regression?
>
> In ring_buffer_resize(), size is an unsigned long and can be quite large,
> resulting in an unsigned int nr_pages near the maximum 32-bit value. When
> calculating nr_pages_to_update:
>
> ring_buffer_resize() {
> ...
> cpu_buffer->nr_pages_to_update = nr_pages - cpu_buffer->nr_pages;
> if (cpu_buffer->nr_pages_to_update <= 0)
> continue;
> ...
> }
>
> If nr_pages is very large (e.g. 4294967286), subtracting the current nr_pages
> (e.g. 10) results in an unsigned value that wraps into a small negative number
> when stored in the 32-bit int nr_pages_to_update (e.g. -20).
>
> This would bypass the allocation checks and trigger rb_remove_pages() to
> remove more pages than exist in the buffer, causing it to loop past the end
> of the list and free pages that are still in use.
That would mean 15 TiB for a single CPU right?
>
> [ ... ]
> > @@ -2773,7 +2773,7 @@ static struct trace_buffer *alloc_buffer(unsigned
> > long size, unsigned flags,
> > struct ring_buffer_remote *remote)
> > {
> > struct trace_buffer *buffer __free(kfree) = NULL;
> > - long nr_pages;
> > + unsigned int nr_pages;
> > int subbuf_size;
> > int bsize;
> > int cpu;
>
> [Severity: High]
> Will this type change cause a multiplication overflow when allocating large
> persistent ring buffers?
>
> Later in alloc_buffer(), the pointer is advanced for the next CPU's buffer:
>
> alloc_buffer() {
> ...
> ptr += subbuf_size * nr_pages;
> ...
> }
>
> Because subbuf_size is an int and nr_pages is now an unsigned int, the
> multiplication is evaluated as a 32-bit unsigned integer.
>
> If the buffer is larger than 4GB per CPU, this will overflow before being
> added to ptr, placing the next CPU's buffer inside the previous CPU's buffer.
>
> Previously, when nr_pages was a long, this math was safely performed as 64-bit
> on 64-bit systems.
This one is worth improving though!
--
Vincent
>
> --
> Sashiko AI review ยท
> https://sashiko.dev/#/patchset/[email protected]?part=9
