Thread (21 messages) flat view 21 messages, 3 authors, 2d ago

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

From: sashiko-bot@kernel.org
Date: 2026-09-01 16:48:37

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 [off-list ref]

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.
quoted hunk ↗ jump to hunk
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help