Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Potential integer underflow in nr_pages calculation is not caught by the new static limit on 32-bit systems, allowing bounds-check bypass and memory corruption. -- commit bfd768f8d8a3d8fc68b36abf088742ebd0904911 Author: Vincent Donnefort <[email protected]> ring-buffer: Cap static ring buffer nr_pages This commit introduces a limit on the number of pages for static ring buffers to ensure they fit into the 31-bit bpage::id field. The limit is enforced on ring buffer creation or user-mapping. > 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 [ ... ] > @@ -2842,6 +2851,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; > + [Severity: High] This is a pre-existing issue, but does this new check fully protect 32-bit systems from an integer underflow here? If an administrator specifies a small ring buffer size (e.g., < 40 bytes) via the trace_instance boot parameter, the calculation size - sizeof(struct ring_buffer_cpu_meta) will underflow. On 32-bit systems, this underflow results in a huge unsigned value that, when divided by subbuf_size, yields an nr_pages value around 1,047,552. This easily passes the new rb_static_max_pages() limit of (1UL << 31) - 1. Later in alloc_buffer(), the pointer calculation wraps around the 32-bit address space: for (n = 0, ptr = buffers_start; n < nr_cpu_ids; n++) { ptr += sizeof(struct ring_buffer_cpu_meta) + sizeof(int) * nr_pages; ptr = ALIGN(ptr, subbuf_size); ptr += subbuf_size * nr_pages; } if (ptr > end) { Because of the wrap around, it bypasses the physical bounds check if (ptr > end) and proceeds to rb_range_meta_init(), which then executes a negative-length memset and panics the kernel. While this patch successfully catches the underflow on 64-bit systems because the massive nr_pages exceeds the 31-bit limit, 32-bit systems remain exposed. Could we add an explicit check to ensure size is larger than sizeof(struct ring_buffer_cpu_meta) before the subtraction? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
