Re: [PATCH v4 9/9] ring-buffer: Make nr_pages unsigned int

[sashiko-bot@kernel.org]

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 <vdonnefort@google.com>
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.

[ ... ]
> @@ -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.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=9