Skip to content

Commit 1d6c39c

Browse files
committed
ring-buffer: Fix persistent buffer when commit page is the reader page
The ring buffer is made up of sub buffers (sometimes called pages as they are by default PAGE_SIZE). It has the following "pages": "tail page" - this is the page that the next write will write to "head page" - this is the page that the reader will swap the reader page with. "reader page" - This belongs to the reader, where it will swap the head page from the ring buffer so that the reader does not race with the writer. The writer may end up on the "reader page" if the ring buffer hasn't written more than one page, where the "tail page" and the "head page" are the same. The persistent ring buffer has meta data that points to where these pages exist so on reboot it can re-create the pointers to the cpu_buffer descriptor. But when the commit page is on the reader page, the logic is incorrect. The check to see if the commit page is on the reader page checked if the head page was the reader page, which would never happen, as the head page is always in the ring buffer. The correct check would be to test if the commit page is on the reader page. If that's the case, then it can exit out early as the commit page is only on the reader page when there's only one page of data in the buffer. There's no reason to iterate the ring buffer pages to find the "commit page" as it is already found. To trigger this bug: # echo 1 > /sys/kernel/tracing/instances/boot_mapped/events/syscalls/sys_enter_fchownat/enable # touch /tmp/x # chown sshd /tmp/x # reboot On boot up, the dmesg will have: Ring buffer meta [0] is from previous boot! Ring buffer meta [1] is from previous boot! Ring buffer meta [2] is from previous boot! Ring buffer meta [3] is from previous boot! Ring buffer meta [4] commit page not found Ring buffer meta [5] is from previous boot! Ring buffer meta [6] is from previous boot! Ring buffer meta [7] is from previous boot! Where the buffer on CPU 4 had a "commit page not found" error and that buffer is cleared and reset causing the output to be empty and the data lost. When it works correctly, it has: # cat /sys/kernel/tracing/instances/boot_mapped/trace_pipe <...>-1137 [004] ..... 998.205323: sys_enter_fchownat: __syscall_nr=0x104 (260) dfd=0xffffff9c (4294967196) filename=(0xffffc90000a0002c) user=0x3e8 (1000) group=0xffffffff (4294967295) flag=0x0 (0 Cc: [email protected] Cc: Mathieu Desnoyers <[email protected]> Link: https://lore.kernel.org/[email protected] Fixes: 5f3b6e8 ("ring-buffer: Validate boot range memory events") Reported-by: Tasos Sahanidis <[email protected]> Tested-by: Tasos Sahanidis <[email protected]> Reviewed-by: Masami Hiramatsu (Google) <[email protected]> Signed-off-by: Steven Rostedt (Google) <[email protected]>
1 parent 11aff32 commit 1d6c39c

File tree

1 file changed

+5
-3
lines changed

1 file changed

+5
-3
lines changed

kernel/trace/ring_buffer.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1887,10 +1887,12 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
18871887

18881888
head_page = cpu_buffer->head_page;
18891889

1890-
/* If both the head and commit are on the reader_page then we are done. */
1891-
if (head_page == cpu_buffer->reader_page &&
1892-
head_page == cpu_buffer->commit_page)
1890+
/* If the commit_buffer is the reader page, update the commit page */
1891+
if (meta->commit_buffer == (unsigned long)cpu_buffer->reader_page->page) {
1892+
cpu_buffer->commit_page = cpu_buffer->reader_page;
1893+
/* Nothing more to do, the only page is the reader page */
18931894
goto done;
1895+
}
18941896

18951897
/* Iterate until finding the commit page */
18961898
for (i = 0; i < meta->nr_subbufs + 1; i++, rb_inc_page(&head_page)) {

0 commit comments

Comments
 (0)