Re: [PATCH v9 4/4] ring-buffer: Prevent truncation of nr_pages / nr_subbufs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Heap buffer overflow in rb_cpu_meta_valid due to unvalidated meta->nr_subbufs
--

commit 1ffddb5e98b6cd5a5142af30014e0d2a25f757f8
Author: Vincent Donnefort <vdonnefort@google.com>

ring-buffer: Prevent truncation of nr_pages / nr_subbufs

This commit updates the variables tracking ring buffer pages and sub-buffers
from 32-bit types to unsigned long or unsigned int to prevent truncation
on very large buffers.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -1854,13 +1853,13 @@ static bool rb_meta_init(struct trace_buffer *buffer, int scratch_size)
>   * must be the same.
>   */
>  static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
> -			      struct trace_buffer *buffer, int nr_pages,
> +			      struct trace_buffer *buffer, unsigned long nr_pages,
>  			      unsigned long *subbuf_mask)
>  {
> -	int subbuf_size = PAGE_SIZE;
> +	unsigned long subbuf_size = PAGE_SIZE;
>  	unsigned long buffers_start;
>  	unsigned long buffers_end;
> -	int i;
> +	unsigned long i;
>  
>  	if (!subbuf_mask)
>  		return false;

[Severity: High]
This is a pre-existing issue, but does this function overflow the
subbuf_mask bitmap?

The size of subbuf_mask is allocated as nr_pages + 1 bits in
rb_range_meta_init(). However, this function unconditionally uses
meta->nr_subbufs from the persistent memory mapped region without
validating it against nr_pages + 1.

If nr_cpu_ids is increased between boots (e.g. by changing the CPU count
or maxcpus= boot parameter), the per-CPU buffer size shrinks, leading to a
smaller nr_pages for the current boot. rb_meta_init() will still accept
the persistent memory as valid because the overall total_size is identical.

Consequently, meta->nr_subbufs from the previous boot will be larger than
the newly allocated subbuf_mask size, which can cause bitmap_clear() and
set_bit() to write out of bounds further down in this function:

    bitmap_clear(subbuf_mask, 0, meta->nr_subbufs);

    for (i = 0; i < meta->nr_subbufs; i++) {
        ...
        set_bit(meta->buffers[i], subbuf_mask);
    }

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