Re: [PATCH v5 10/10] ring-buffer: Make nr_pages unsigned int
From: sashiko-bot@kernel.org
Date: 2026-08-13 13:55:27
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Critical] Integer overflow in ring_buffer_resize() allows an attacker to bypass allocation checks and trigger a massive out-of-bounds ring buffer shrink, leading to a Use-After-Free. - [Medium] 32-bit shift overflow in __rb_map_vma() truncates the mapped page count for huge ring buffers, causing mmap to fail. --
--- Patch [10]: [PATCH v5 10/10] ring-buffer: Make nr_pages unsigned int ---commit 25e7b6bfdf3741f228ceb9dc41c8bbf6e253f525
Author: Vincent Donnefort [off-list ref]
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.
quoted hunk ↗ jump to hunk
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c index 79459c99ce9a0..d9dcd6d4e7cc4 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;
[ ... ]
quoted hunk ↗ jump to hunk
@@ -3264,7 +3263,7 @@ int ring_buffer_resize(struct trace_buffer *buffer, unsigned long size, int cpu_id) { struct ring_buffer_per_cpu *cpu_buffer; - unsigned long nr_pages; + unsigned int nr_pages;
[Severity: Critical]
If a huge value is written to buffer_size_kb, could the unsigned
difference between nr_pages and cpu_buffer->nr_pages overflow when
assigned to the now 32-bit signed cpu_buffer->nr_pages_to_update?
Later in this function, we do:
cpu_buffer->nr_pages_to_update = nr_pages - cpu_buffer->nr_pages;
if (cpu_buffer->nr_pages_to_update <= 0)
continue;
If the difference overflows into a negative integer, wouldn't it skip this
allocation limits check but still schedule the worker thread (since it is
non-zero)?
The worker thread calls rb_update_pages():
success = rb_remove_pages(cpu_buffer, -cpu_buffer->nr_pages_to_update);
This would pass a large positive count to rb_remove_pages(). As it wildly
wraps around the small circular list:
for (nr_removed = 0; nr_removed < nr_pages; nr_removed++) {
to_remove = rb_list_head(to_remove)->next;
head_bit |= (unsigned long)to_remove & RB_PAGE_HEAD;
}
Could this unlink and double-free actively used pages, leaving ring buffer
structures pointing to freed memory and causing a use-after-free?
int cpu, err; /*
[ ... ]
quoted hunk ↗ jump to hunk
@@ -7708,8 +7708,8 @@ static int __rb_inc_dec_mapped(struct ring_buffer_per_cpu *cpu_buffer, static int __rb_map_vma(struct ring_buffer_per_cpu *cpu_buffer, struct vm_area_struct *vma) { - unsigned long nr_subbufs, nr_pages, nr_vma_pages, pgoff = vma->vm_pgoff; - unsigned int subbuf_pages, subbuf_order; + unsigned long nr_pages, nr_vma_pages, pgoff = vma->vm_pgoff; + unsigned int nr_subbufs, subbuf_pages, subbuf_order;
[Severity: Medium]
Since nr_subbufs is now an unsigned int, could the bitwise shift operation
later in this function silently truncate for exceptionally large ring buffers?
__rb_map_vma() later computes:
nr_pages = ((nr_subbufs + 1) << subbuf_order) - 1;
Because the operands are now 32-bit, the shift is computed entirely in
32-bit math before assignment to the unsigned long nr_pages.
If a buffer is large enough to overflow 32-bits, will this cause bounds
checks to fail and mmap to return -EINVAL?
struct page **pages __free(kfree) = NULL; int p = 0, s = 0; int err;
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260813131152.3589632-1-vdonnefort@google.com?part=10