Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Critical] Integer truncation of `nr_pages_to_update` allows a massive buffer resize to bypass memory allocation checks, directly triggering an out-of-bounds page removal loop causing a critical kernel Use-After-Free (UAF). - [High] Multiplication overflow in `subbuf_size * nr_pages` evaluates as an `unsigned int` when allocating huge persistent ring buffers, destroying trace data isolation. --
--- Patch [9]: [PATCH v4 9/9] ring-buffer: Make nr_pages unsigned int --- commit cb1365ebdbbed7daa3a685ae1d6be7a4167052dd Author: Vincent Donnefort <[email protected]> Date: Wed Aug 12 15:33:10 2026 +0000 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. Make ring_buffer_per_cpu::nr_pages unsigned and align all the users to it. As a side effect, this makes ring_buffer_per_cpu slightly smaller. > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c > index 990a904cefe85..2fccb950e593c 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; [Severity: Critical] Could this type change lead to an out-of-bounds page removal regression? In ring_buffer_resize(), size is an unsigned long and can be quite large, resulting in an unsigned int nr_pages near the maximum 32-bit value. When calculating nr_pages_to_update: ring_buffer_resize() { ... cpu_buffer->nr_pages_to_update = nr_pages - cpu_buffer->nr_pages; if (cpu_buffer->nr_pages_to_update <= 0) continue; ... } If nr_pages is very large (e.g. 4294967286), subtracting the current nr_pages (e.g. 10) results in an unsigned value that wraps into a small negative number when stored in the 32-bit int nr_pages_to_update (e.g. -20). This would bypass the allocation checks and trigger rb_remove_pages() to remove more pages than exist in the buffer, causing it to loop past the end of the list and free pages that are still in use. [ ... ] > @@ -2773,7 +2773,7 @@ static struct trace_buffer *alloc_buffer(unsigned long > size, unsigned flags, > struct ring_buffer_remote *remote) > { > struct trace_buffer *buffer __free(kfree) = NULL; > - long nr_pages; > + unsigned int nr_pages; > int subbuf_size; > int bsize; > int cpu; [Severity: High] Will this type change cause a multiplication overflow when allocating large persistent ring buffers? Later in alloc_buffer(), the pointer is advanced for the next CPU's buffer: alloc_buffer() { ... ptr += subbuf_size * nr_pages; ... } Because subbuf_size is an int and nr_pages is now an unsigned int, the multiplication is evaluated as a 32-bit unsigned integer. If the buffer is larger than 4GB per CPU, this will overflow before being added to ptr, placing the next CPU's buffer inside the previous CPU's buffer. Previously, when nr_pages was a long, this math was safely performed as 64-bit on 64-bit systems. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
