Concurrent subbuffer resizes may crash trace_pipe_raw readers or leak
uninitialized memory to userspace due to stale size values.
Modify ring_buffer_alloc_read_page() to let it handle the resizing of
a previous buffer_data_read_page if necessary and add a new
ring_buffer_read_page_size() which enable ring-buffer users to not use
the racy ring_buffer_subbuf_size_get(). This makes the spare_size member
of ftrace_buffer_info redundant.
Use those functions in trace_pipe_raw readers and handle in both the
case where the subbuf order is modified in the middle of the read.
Fixes: bce761d75745 ("ring-buffer: Read and write to ring buffers with custom
sub buffer size")
Signed-off-by: Vincent Donnefort <[email protected]>
diff --git a/include/linux/ring_buffer.h b/include/linux/ring_buffer.h
index 0670742b2d60..fafbeb327037 100644
--- a/include/linux/ring_buffer.h
+++ b/include/linux/ring_buffer.h
@@ -219,13 +219,15 @@ size_t ring_buffer_nr_dirty_pages(struct trace_buffer
*buffer, int cpu);
struct buffer_data_read_page;
struct buffer_data_read_page *
-ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu);
+ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu,
+ struct buffer_data_read_page *prev);
void ring_buffer_free_read_page(struct trace_buffer *buffer, int cpu,
struct buffer_data_read_page *page);
int ring_buffer_read_page(struct trace_buffer *buffer,
struct buffer_data_read_page *data_page,
size_t len, int cpu, int full);
void *ring_buffer_read_page_data(struct buffer_data_read_page *page);
+unsigned int ring_buffer_read_page_size(struct buffer_data_read_page *page);
struct trace_seq;
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 94552a433228..f62d6853ee5c 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -6988,22 +6988,34 @@ EXPORT_SYMBOL_GPL(ring_buffer_swap_cpu);
* Returns:
* The page allocated, or ERR_PTR
*/
-struct buffer_data_read_page *
-ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
+struct buffer_data_read_page *ring_buffer_alloc_read_page(struct trace_buffer
*buffer, int cpu,
+ struct
buffer_data_read_page *prev)
{
+ struct buffer_data_read_page *bpage = prev;
struct ring_buffer_per_cpu *cpu_buffer;
- struct buffer_data_read_page *bpage = NULL;
unsigned long flags;
+ unsigned int order;
if (!cpumask_test_cpu(cpu, buffer->cpumask))
return ERR_PTR(-ENODEV);
- bpage = kzalloc_obj(*bpage);
- if (!bpage)
- return ERR_PTR(-ENOMEM);
-
- bpage->order = buffer->subbuf_order;
+ order = buffer->subbuf_order;
cpu_buffer = buffer->buffers[cpu];
+
+ if (!bpage) {
+ bpage = kzalloc_obj(*bpage);
+ if (!bpage)
+ return ERR_PTR(-ENOMEM);
+ } else {
+ if (bpage->order == order)
+ return bpage;
+
+ free_pages((unsigned long)bpage->data, bpage->order);
+ bpage->data = NULL;
+ }
+
+ bpage->order = order;
+
local_irq_save(flags);
arch_spin_lock(&cpu_buffer->lock);
@@ -7020,7 +7032,9 @@ ring_buffer_alloc_read_page(struct trace_buffer *buffer,
int cpu)
} else {
bpage->data = alloc_cpu_data(cpu, bpage->order);
if (!bpage->data) {
- kfree(bpage);
+ if (!prev)
+ kfree(bpage);
+
return ERR_PTR(-ENOMEM);
}
}
@@ -7041,13 +7055,22 @@ void ring_buffer_free_read_page(struct trace_buffer
*buffer, int cpu,
struct buffer_data_read_page *data_page)
{
struct ring_buffer_per_cpu *cpu_buffer;
- struct buffer_data_page *dpage = data_page->data;
- struct page *page = virt_to_page(dpage);
+ struct buffer_data_page *dpage;
unsigned long flags;
+ struct page *page;
if (!buffer || !buffer->buffers || !buffer->buffers[cpu])
return;
+ if (!data_page)
+ return;
+
+ dpage = data_page->data;
+ if (!dpage)
+ goto out;
+
+ page = virt_to_page(dpage);
+
cpu_buffer = buffer->buffers[cpu];
/*
@@ -7312,6 +7335,18 @@ void *ring_buffer_read_page_data(struct
buffer_data_read_page *page)
}
EXPORT_SYMBOL_GPL(ring_buffer_read_page_data);
+/**
+ * ring_buffer_read_page_size - get size of the read page.
+ * @page: the page to get the size from
+ *
+ * Returns size of the page in bytes.
+ */
+unsigned int ring_buffer_read_page_size(struct buffer_data_read_page *page)
+{
+ return PAGE_SIZE << page->order;
+}
+EXPORT_SYMBOL_GPL(ring_buffer_read_page_size);
+
/**
* ring_buffer_subbuf_size_get - get size of the sub buffer.
* @buffer: the buffer to get the sub buffer size from
diff --git a/kernel/trace/ring_buffer_benchmark.c
b/kernel/trace/ring_buffer_benchmark.c
index 593e3b59e42e..39b7dee21ed3 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -114,7 +114,7 @@ static enum event_status read_page(int cpu)
int inc;
int i;
- bpage = ring_buffer_alloc_read_page(buffer, cpu);
+ bpage = ring_buffer_alloc_read_page(buffer, cpu, NULL);
if (IS_ERR(bpage))
return EVENT_DROPPED;
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 395238b2b715..0409d20a168b 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -7080,8 +7080,8 @@ ssize_t tracing_buffers_read(struct file *filp, char
__user *ubuf,
{
struct ftrace_buffer_info *info = filp->private_data;
struct trace_iterator *iter = &info->iter;
- void *trace_data;
- int page_size;
+ void *trace_data, *prev_spare;
+ unsigned int spare_size;
ssize_t ret = 0;
ssize_t size;
@@ -7091,36 +7091,31 @@ ssize_t tracing_buffers_read(struct file *filp, char
__user *ubuf,
if (iter->snapshot && tracer_uses_snapshot(iter->tr->current_trace))
return -EBUSY;
- page_size = ring_buffer_subbuf_size_get(iter->array_buffer->buffer);
+again:
+ prev_spare = info->spare;
+ if (prev_spare) {
+ spare_size = ring_buffer_read_page_size(info->spare);
- /* Make sure the spare matches the current sub buffer size */
- if (info->spare) {
- if (page_size != info->spare_size) {
- ring_buffer_free_read_page(iter->array_buffer->buffer,
- info->spare_cpu,
info->spare);
- info->spare = NULL;
- }
+ /* Do we have previous read data to read? */
+ if (info->read < spare_size)
+ goto read;
}
- if (!info->spare) {
- info->spare =
ring_buffer_alloc_read_page(iter->array_buffer->buffer,
- iter->cpu_file);
- if (IS_ERR(info->spare)) {
- ret = PTR_ERR(info->spare);
- info->spare = NULL;
- } else {
- info->spare_cpu = iter->cpu_file;
- info->spare_size = page_size;
- }
- }
- if (!info->spare)
+ info->read = 0;
+
+ /* Make sure the read page order is aligned with the current buffer
subbuf order */
+ info->spare = ring_buffer_alloc_read_page(iter->array_buffer->buffer,
iter->cpu_file,
+ prev_spare);
+ if (IS_ERR(info->spare)) {
+ ret = PTR_ERR(info->spare);
+ info->spare = NULL;
+ ring_buffer_free_read_page(iter->array_buffer->buffer,
info->spare_cpu, prev_spare);
return ret;
+ }
- /* Do we have previous read data to read? */
- if (info->read < page_size)
- goto read;
+ spare_size = ring_buffer_read_page_size(info->spare);
+ info->spare_cpu = iter->cpu_file;
- again:
trace_access_lock(iter->cpu_file);
ret = ring_buffer_read_page(iter->array_buffer->buffer,
info->spare,
@@ -7129,6 +7124,10 @@ ssize_t tracing_buffers_read(struct file *filp, char
__user *ubuf,
trace_access_unlock(iter->cpu_file);
if (ret < 0) {
+ /* Did we race with ring_buffer_subbuf_order_set ? */
+ if (spare_size !=
ring_buffer_subbuf_size_get(iter->array_buffer->buffer))
+ goto again;
+
if (trace_empty(iter) && !iter->closed) {
if (update_last_data_if_empty(iter->tr))
return 0;
@@ -7142,12 +7141,12 @@ ssize_t tracing_buffers_read(struct file *filp, char
__user *ubuf,
goto again;
}
+
return 0;
}
- info->read = 0;
read:
- size = page_size - info->read;
+ size = spare_size - info->read;
if (size > count)
size = count;
trace_data = ring_buffer_read_page_data(info->spare);
@@ -7268,23 +7267,12 @@ ssize_t tracing_buffers_splice_read(struct file *file,
loff_t *ppos,
};
struct buffer_ref *ref;
bool woken = false;
- int page_size;
int entries, i;
ssize_t ret = 0;
if (iter->snapshot && tracer_uses_snapshot(iter->tr->current_trace))
return -EBUSY;
- page_size = ring_buffer_subbuf_size_get(iter->array_buffer->buffer);
- if (*ppos & (page_size - 1))
- return -EINVAL;
-
- if (len & (page_size - 1)) {
- if (len < page_size)
- return -EINVAL;
- len &= (~(page_size - 1));
- }
-
if (splice_grow_spd(pipe, &spd))
return -ENOMEM;
@@ -7292,9 +7280,10 @@ ssize_t tracing_buffers_splice_read(struct file *file,
loff_t *ppos,
trace_access_lock(iter->cpu_file);
entries = ring_buffer_entries_cpu(iter->array_buffer->buffer,
iter->cpu_file);
- for (i = 0; i < spd.nr_pages_max && len && entries; i++, len -=
page_size) {
+ for (i = 0; i < spd.nr_pages_max && len && entries; i++) {
+ unsigned int page_size;
struct page *page;
- int r;
+ int r = -EINVAL;
ref = kzalloc_obj(*ref);
if (!ref) {
@@ -7304,7 +7293,7 @@ ssize_t tracing_buffers_splice_read(struct file *file,
loff_t *ppos,
refcount_set(&ref->refcount, 1);
ref->buffer = iter->array_buffer->buffer;
- ref->page = ring_buffer_alloc_read_page(ref->buffer,
iter->cpu_file);
+ ref->page = ring_buffer_alloc_read_page(ref->buffer,
iter->cpu_file, NULL);
if (IS_ERR(ref->page)) {
ret = PTR_ERR(ref->page);
ref->page = NULL;
@@ -7313,11 +7302,21 @@ ssize_t tracing_buffers_splice_read(struct file *file,
loff_t *ppos,
}
ref->cpu = iter->cpu_file;
- r = ring_buffer_read_page(ref->buffer, ref->page,
- len, iter->cpu_file, 1);
+ page_size = ring_buffer_read_page_size(ref->page);
+
+ if (IS_ALIGNED(*ppos, page_size) && len >= page_size) {
+ r = ring_buffer_read_page(ref->buffer, ref->page, len,
iter->cpu_file, 1);
+ } else if (!i) {
+ /*
+ * If the first iteration fails this is an invalid
userspace input.
+ * Otherwise, this is because the subbuf order has been
modified. Do not
+ * report an error and finish the read.
+ */
+ ret = -EINVAL;
+ }
+
if (r < 0) {
- ring_buffer_free_read_page(ref->buffer, ref->cpu,
- ref->page);
+ ring_buffer_free_read_page(ref->buffer, ref->cpu,
ref->page);
kfree(ref);
break;
}
@@ -7330,6 +7329,7 @@ ssize_t tracing_buffers_splice_read(struct file *file,
loff_t *ppos,
spd.partial[i].private = (unsigned long)ref;
spd.nr_pages++;
*ppos += page_size;
+ len -= page_size;
entries = ring_buffer_entries_cpu(iter->array_buffer->buffer,
iter->cpu_file);
}
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index bf77331f56a4..981e87b0f5a9 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -748,7 +748,6 @@ struct ftrace_buffer_info {
struct trace_iterator iter;
void *spare;
unsigned int spare_cpu;
- unsigned int spare_size;
unsigned int read;
};
--
2.55.0.691.gc56d675ccc-goog
