ring_buffer_read_page() checks that its spare page has the current
subbuffer order before taking cpu_buffer->reader_lock. A concurrent
ring_buffer_subbuf_order_set() can change the order and replace the
reader page after that check. The reader then copies a larger subbuffer
into the old allocation, causing an out-of-bounds write.
Keep spare-page allocation and release under buffer->mutex, which already
serializes order changes. Move the read-side order check under
reader_lock, the lock used by resize when replacing per-CPU pages.
Fixes: f9b94daa542a ("ring-buffer: Set new size of the ring buffer sub page")
Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=2dd9d02f60775ce5c1fb
Cc: [email protected]
Signed-off-by: Yousef Alhouseen <[email protected]>
---
kernel/trace/ring_buffer.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 56a328e94395..eed5d7cffdee 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -6950,6 +6950,8 @@ ring_buffer_alloc_read_page(struct trace_buffer *buffer,
int cpu)
if (!cpumask_test_cpu(cpu, buffer->cpumask))
return ERR_PTR(-ENODEV);
+ guard(mutex)(&buffer->mutex);
+
bpage = kzalloc_obj(*bpage);
if (!bpage)
return ERR_PTR(-ENOMEM);
@@ -7000,6 +7002,8 @@ void ring_buffer_free_read_page(struct trace_buffer
*buffer, int cpu,
if (!buffer || !buffer->buffers || !buffer->buffers[cpu])
return;
+ guard(mutex)(&buffer->mutex);
+
cpu_buffer = buffer->buffers[cpu];
/*
@@ -7091,14 +7095,13 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
if (!data_page || !data_page->data)
return -1;
- if (data_page->order != buffer->subbuf_order)
- return -1;
-
dpage = data_page->data;
if (!dpage)
return -1;
guard(raw_spinlock_irqsave)(&cpu_buffer->reader_lock);
+ if (data_page->order != buffer->subbuf_order)
+ return -1;
reader = rb_get_reader_page(cpu_buffer);
if (!reader)
--
2.54.0
