Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Double fetch vulnerability in ring_buffer_read_page() when parsing events from remote/static buffers leads to out-of-bounds memcpy. -- commit c069a5fa2bee1114a0a60482e4b506024892ddbd Author: Vincent Donnefort <[email protected]> ring-buffer: Allow splice reads on static buffers ring_buffer_read_page() rejects splice (full=1) reads on static buffers because the !read check assumes unread pages must be swapped. This commit changes the logic to allow memcpy for static buffers, returning an error only when the writer is still on the reader page. > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c > index b0963ac6fd167..84fd4cdd486f8 100644 > --- a/kernel/trace/ring_buffer.c > +++ b/kernel/trace/ring_buffer.c > @@ -7193,15 +7193,8 @@ int ring_buffer_read_page(struct trace_buffer *buffer, > unsigned int event_size; > unsigned int flags = 0; > > - /* > - * If a full page is expected, this can still be returned > - * if there's been a previous partial read and the > - * rest of the page can be read and the commit page is off > - * the reader page. > - */ > - if (full && > - (!read || (len < (size - read)) || > - cpu_buffer->reader_page == cpu_buffer->commit_page)) > + /* If a full page is requested, it cannot be the commit page */ > + if (full && cpu_buffer->reader_page == cpu_buffer->commit_page) > return -1; > > if (len > (size - read)) [Severity: Critical] This is a pre-existing issue, but is there a double fetch vulnerability when parsing events from remote or static buffers? In ring_buffer_read_page(), when reading from remote buffers, the event pointer directly references shared memory. The event size is first fetched and validated against the remaining length: event_size = rb_event_ts_length(event); if (len < event_size) return -1; Inside the do-while loop, it is refetched and used directly in memcpy(): do { event_size = rb_event_length(event); memcpy(dpage->data + pos, rpage->data + rpos, event_size); ... event_size = rb_event_ts_length(event); } while (len >= event_size); If a malicious remote writer (such as an untrusted guest VM) modifies the length fields in shared memory between the initial check and the memcpy(), can event_size become arbitrarily large? Could this bypass the bounds check and cause an out-of-bounds read from rpage->data and a massive out-of-bounds write to dpage->data? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
