Thread (15 messages) flat view 15 messages, 3 authors, 15d ago

Re: [PATCH v8 3/3] ring-buffer: Prevent truncation of nr_pages / nr_subbufs

From: sashiko-bot@kernel.org
Date: 2026-08-26 10:02:18

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

Pre-existing issues:
- [High] The kernel fails to validate the untrusted `meta->nr_subbufs` value from persistent memory against the expected `nr_pages + 1`, leading to arbitrary heap out-of-bounds writes and potential arbitrary kernel memory corruption.
--

commit 5ed0ab549782b2ff87e1bd2394abb78b10c4b602
Author: Vincent Donnefort [off-list ref]

ring-buffer: Prevent truncation of nr_pages / nr_subbufs

Although ring_buffer_per_cpu::nr_pages is defined as unsigned long, it
is capped to 32-bits in a few places, limiting the operations possible
on a very large buffer. Use unsigned long where appropriate and
prevent truncation of values using nr_pages (or nr_subbufs).

While at it, subbuf_size must be at least unsigned int.

Note that persistent, remote and user-mapped ring buffers are capping
the number of pages to 31 bits already, making "int" safe in many
places.
quoted hunk ↗ jump to hunk
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 6089fcc67e2b6..1fcf55e2ca81f 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1745,7 +1744,7 @@ static void *rb_range_meta(struct trace_buffer *buffer, int nr_pages, int cpu)
 /* Return the start of subbufs given the meta pointer */
 static void *rb_subbufs_from_meta(struct ring_buffer_cpu_meta *meta)
 {
-	int subbuf_size = meta->subbuf_size;
+	unsigned int subbuf_size = meta->subbuf_size;
 	unsigned long ptr;
 
 	ptr = (unsigned long)meta;
[Severity: High]
This isn't a bug introduced by this patch, but does the next line in this
function perform pointer arithmetic using an unvalidated meta->nr_subbufs?

	ptr = rb_range_align_subbuf(ptr, subbuf_size, meta->nr_subbufs);

Could this yield a corrupted or attacker-controlled subbuf base pointer if
meta->nr_subbufs is read from persistent memory without being checked against
expected limits?

[ ... ]
quoted hunk ↗ jump to hunk
@@ -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 isn't a bug introduced by this patch, but is there a risk of an
out-of-bounds heap write further down in this function?

The subbuf_mask is allocated with nr_pages + 1 bits in rb_range_meta_init(),
but later in rb_cpu_meta_valid() operations use the unvalidated
meta->nr_subbufs value read from persistent memory:

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

	/*
	 * Ensure the meta::buffers array has correct data. The data in each subbufs
	 * are checked later in rb_meta_validate_events().
	 */
	for (i = 0; i < meta->nr_subbufs; i++) {
		if (meta->buffers[i] < 0 ||
		    meta->buffers[i] >= meta->nr_subbufs) {
			pr_info("Ring buffer boot meta [%d] array out of range\n", cpu);
			return false;
		}

		if (test_bit(meta->buffers[i], subbuf_mask)) {
			pr_info("Ring buffer boot meta [%d] array has duplicates\n", cpu);
			return false;
		}

		set_bit(meta->buffers[i], subbuf_mask);
	}

If meta->nr_subbufs is larger than nr_pages + 1, could this write out of
bounds and corrupt kernel memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826094528.3738023-1-vdonnefort@google.com?part=3
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help