Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Integer underflow in alloc_buffer() for mapped ring buffers leads to memory corruption on 32-bit systems. - [High] Out-of-bounds memory corruption in rb_cpu_meta_valid during ring buffer boot meta validation due to unvalidated meta->nr_subbufs. -- --- Patch [3]: [PATCH v7 3/3] ring-buffer: Prevent truncation of nr_pages / nr_subbufs --- commit ca74d5f745c91c8b4a7ee42fae91ab814132919c Author: Vincent Donnefort <[email protected]> ring-buffer: Prevent truncation of nr_pages / nr_subbufs This commit prevents truncation operations on large ring buffers by changing nr_pages and nr_subbufs definitions to unsigned long. It also upgrades subbuf_size to unsigned int. > 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 [ ... ] > @@ -1849,13 +1848,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 can this lead to an out-of-bounds memory overwrite on the kernel heap? In rb_range_meta_init(), subbuf_mask is allocated using the current boot's nr_pages + 1: subbuf_mask = bitmap_alloc(nr_pages + 1, GFP_KERNEL); However, later in rb_cpu_meta_valid(), the legacy persistent metadata is used to clear and set bits in this mask: bitmap_clear(subbuf_mask, 0, meta->nr_subbufs); ... for (i = 0; i < meta->nr_subbufs; i++) { ... set_bit(meta->buffers[i], subbuf_mask); } If the system boots with more CPUs than the previous cycle, the per-CPU nr_pages constraint decreases. Consequently, the legacy meta->nr_subbufs could be larger than the newly allocated subbuf_mask, causing bitmap_clear() and set_bit() to overflow the allocation. [ ... ] > @@ -2782,8 +2782,8 @@ 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; > - int subbuf_size; > + unsigned int subbuf_size; > + unsigned long nr_pages; > int bsize; > int cpu; > int ret; [Severity: High] This is a pre-existing issue, but does this function suffer from an integer underflow that could bypass length boundary validations? Further down in alloc_buffer(), when calculating the per-CPU size for mapped buffers: size = end - buffers_start; size = size / nr_cpu_ids; nr_pages = (size - sizeof(struct ring_buffer_cpu_meta)) / (subbuf_size + sizeof(int)); If the user-provided mapped range is smaller than the meta structure size, size - sizeof(struct ring_buffer_cpu_meta) will underflow. On 32-bit systems, this yields a massive nr_pages value that successfully passes the rb_static_max_pages() threshold check. The subsequently calculated pointers will wrap around the 32-bit address space, defeating the bounds check against the end boundary, and allowing mapped metadata structures to be written into arbitrary out-of-bounds memory. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
