Re: [PATCH v10 3/4] ring-buffer: Cap static ring buffer nr_pages
From: sashiko-bot@kernel.org
Date: 2026-09-04 17:04:04
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [High] Integer underflow in size calculation bypasses the max pages limit on 32-bit systems, leading to massive out-of-bounds memory initialization. - [High] Unchecked return value from `ring_buffer_desc()` leading to a NULL pointer dereference in remote buffer allocation. -- commit be7971253ae6dad60dcb1cc2d29fe4019ffb6e16 Author: Vincent Donnefort [off-list ref] ring-buffer: Cap static ring buffer nr_pages This commit introduces a limit on the number of pages for static ring buffers to ensure the page count fits within the bpage::id field. It adds bounds checks during ring buffer creation and mapping to prevent exceeding this maximum limit.
quoted hunk ↗ jump to hunk
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c index 2f61cb510b0ac..6b914abd80e3c 100644 --- a/kernel/trace/ring_buffer.c +++ b/kernel/trace/ring_buffer.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2833,6 +2842,8 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags, size = end - buffers_start; size = size / nr_cpu_ids; + if (size < sizeof(struct ring_buffer_cpu_meta)) + goto fail_free_buffers;
[Severity: High] This is a pre-existing issue, but could this size calculation result in an integer underflow if a user configures a small range size where end is less than buffers_start? On 32-bit systems, the unsigned subtraction would wrap around to a huge positive integer. Because the resulting size is massive, the new size less than sizeof(struct ring_buffer_cpu_meta) check would pass. Since nr_pages would become approximately 262,144 in this wrapped state, it would also bypass the newly added nr_pages > rb_static_max_pages() limit. Could this loop then accumulate pointers wrapping around the 32-bit address space, bypassing the bounds check and leading to massive out-of-bounds memory initialization?
quoted hunk ↗ jump to hunk
/* * The number of sub-buffers (nr_pages) is determined by the * total size allocated minus the meta data size.@@ -2842,6 +2853,10 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags, */ nr_pages = (size - sizeof(struct ring_buffer_cpu_meta)) / (subbuf_size + sizeof(int)); + + if (nr_pages > rb_static_max_pages()) + goto fail_free_buffers; + /* Need at least two pages plus the reader page */ if (nr_pages < 3) goto fail_free_buffers;
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2874,6 +2889,10 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags, /* The writer is remote. This ring-buffer is read-only */ atomic_inc(&buffer->record_disabled); nr_pages = desc->nr_page_va - 1;
[Severity: High] This isn't a bug introduced by this patch, but is it possible for ring_buffer_desc() to return NULL here if the provided remote descriptor lacks an entry for the currently executing CPU? If the descriptor does not have an entry for the executing CPU, unconditionally dereferencing desc->nr_page_va would cause a NULL pointer dereference. Other functions in this file, such as __rb_allocate_pages(), appear to check if desc is NULL before proceeding. Could this lead to a kernel panic if an untrusted hypervisor or client provides an incomplete remote trace descriptor?
+
+ if (nr_pages > rb_static_max_pages())
+ goto fail_free_buffers;
+
if (nr_pages < 2)
goto fail_free_buffers;
} else {[ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/20260904164450.1345852-1-vdonnefort@google.com?part=3