tracing: More fixes for v7.3

tracing fixes for 7.3:

- Use rcu_assign_pointer() for tmp_ops filter hash

The tmp_ops used in update_ftrace_direct_mod() assigns its filter_hash field directly, but that field is annotated as __rcu and sparse complains. Assign it with rcu_assign_pointer()

- Fix use-after-free in enable_trigger_private_data_free()

The trace_event_call is accessed through the event_trigger_data's trace_event_file pointer to put the trace_event_call on freeing. The issue is that the trace_event_file data may have been freed already causing a use-after-free. Add a field to the event_trigger_data that points directly to the trace_event_call so that it can decrement its reference directly without needing to go through the trace_event_file.

- Fix accounting of buffer data remote headers

trace_buffer_desc_size() and trace_remote_alloc_buffer() undercount the number of pages is needed for the asked for size as it doesn't take into account the meta data on each page. Add a helper function to do the calculation properly and use that in these functions.

- Catch nr_page_va overflow in ring_buffer_desc sizing

The number of pages per remote ring buffer is capped by ring_buffer_desc::nr_page_va (32 bits). A buffer_size large enough to overflow that field would silently allocate a descriptor smaller than what was asked for.

- Do not resize the subbuf order if any per_cpu buffer is disabled

The mmapping of ring buffers disables resizing the subbuffers, but it is done per-cpu whereas the subbuf size change is done for all the per_cpu buffers under the buffer->mutex. It could change the size of some while the mapping is happening on others. Have the resize of the subbuf order check all the per_cpu buffers under the lock to see if any of them is disabled before starting and causing an inconsistency between buffers that are being mapped.

git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
trace/fixes

Head SHA1: d860c67c051685abb0460b593b193f0f45f4fa92

Ali Ahmet Memiş (1):
tracing: Don't dereference trace_event_file in deferred trigger free

David Carlier (1):
ring-buffer: Check resize_disabled before publishing the new subbuf order

Leon Hwang (1):
ftrace: Use rcu_assign_pointer() for tmp_ops filter hash

Vincent Donnefort (2):
tracing/remotes: Account for ring buffer page header in size calculation tracing/remotes: Catch nr_page_va overflow in ring_buffer_desc sizing

----

 include/linux/ring_buffer.h         | 19 +++++++++++++++++--
 kernel/trace/ftrace.c               |  2 +-
 kernel/trace/ring_buffer.c          |  8 ++++++++
 kernel/trace/trace.h                |  1 +
 kernel/trace/trace_events_trigger.c |  4 +++-
 kernel/trace/trace_remote.c         |  6 +++++-
 6 files changed, 35 insertions(+), 5 deletions(-)

From: Leon Hwang <[email protected]>

tmp_ops.func_hash->filter_hash is annotated __rcu, but update_ftrace_direct_mod() assigns hash to it directly. Sparse reports an address-space mismatch.

Use rcu_assign_pointer() for the assignment.

---
 kernel/trace/ftrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 53d5db60bfa5..673a54fdf392 100644
@@ -6675,7 +6675,7 @@ int update_ftrace_direct_mod(struct ftrace_ops *ops, struct ftrace_hash *hash, b
 
 	/* Enable the tmp_ops to have the same functions as the hash object. */
 	ftrace_ops_init(&tmp_ops);
-	tmp_ops.func_hash->filter_hash = hash;
+	rcu_assign_pointer(tmp_ops.func_hash->filter_hash, hash);
 
 	err = register_ftrace_function_nolock(&tmp_ops);
 	if (err)

2.53.0

From: =?UTF-8?q?Ali=20Ahmet=20Memi=C5=9F?= <[email protected]>

The enable_event trigger defers trace_event_put_ref() to the trigger free kthread, but the trace_event_file can already be freed when the instance is removed.

Keep the trace_event_call directly in enable_trigger_data so the deferred free does not access the freed trace_event_file.

---
 kernel/trace/trace.h                | 1 +
 kernel/trace/trace_events_trigger.c | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

index bd3c8f80300f..3749485a7d85 100644
@@ -1953,6 +1953,7 @@ struct event_trigger_data {
 
 struct enable_trigger_data {
 	struct trace_event_file		*file;
+	struct trace_event_call		*call;
 	bool				enable;
 	bool				hist;
 };

index 149300cc5e8a..4d2fde996c0f 100644
@@ -1728,7 +1728,8 @@ static void enable_trigger_private_data_free(struct event_trigger_data *data)
 {
 	struct enable_trigger_data *enable_data = data->private_data;
 
-	trace_event_put_ref(enable_data->file->event_call);
+	/* The file may already be freed here, only the call is kept alive */
+	trace_event_put_ref(enable_data->call);
 	kfree(enable_data);
 }
 

@@ -1801,6 +1802,7 @@ int event_enable_trigger_parse(struct event_command *cmd_ops,
 	enable_data->hist = hist;
 	enable_data->enable = enable;
 	enable_data->file = event_enable_file;
+	enable_data->call = event_enable_file->event_call;
 
 	trigger_data = trigger_data_alloc(cmd_ops, cmd, param, enable_data);
 	if (!trigger_data)

2.53.0

From: Vincent Donnefort <[email protected]>

trace_buffer_desc_size() and trace_remote_alloc_buffer() undercount the required pages because every ring buffer page contains a header (BUF_PAGE_HDR_SIZE). Account for that header to ensure allocated remote ring buffers aren't smaller than requested by the user.

The newly introduced helper __calc_nr_pages_ring_buffer_desc() can return a value that overflows the descriptor nr_pages field (32 bits).

---
 include/linux/ring_buffer.h | 15 +++++++++++++--
 kernel/trace/trace_remote.c |  2 +-
 2 files changed, 14 insertions(+), 3 deletions(-)

index afc7daa6ee7d..11bffb6a142d 100644
@@ -3,8 +3,9 @@
 #define _LINUX_RING_BUFFER_H
 
 #include <linux/mm.h>
-#include <linux/seq_file.h>
 #include <linux/poll.h>
+#include <linux/ring_buffer_types.h>
+#include <linux/seq_file.h>
 
 #include <uapi/linux/trace_mmap.h>
 

@@ -279,9 +280,19 @@ static inline struct ring_buffer_desc *__first_ring_buffer_desc(struct trace_buf
 	return (struct ring_buffer_desc *)(&desc->__data[0]);
 }
 
+/*
+ * Returns the number of pages for a ring_buffer_desc. The caller must ensure it
+ * does not overflow ring_buffer_desc::nr_page_va.
+ */
+static inline unsigned long __calc_nr_pages_ring_buffer_desc(size_t size)
+{
+	/* Takes into account the reader page */
+	return max(DIV_ROUND_UP(size, PAGE_SIZE - BUF_PAGE_HDR_SIZE), 2UL) + 1;
+}
+
 static inline size_t trace_buffer_desc_size(size_t buffer_size, unsigned int nr_cpus)
 {
-	unsigned int nr_pages = max(DIV_ROUND_UP(buffer_size, PAGE_SIZE), 2UL) + 1;
+	unsigned long nr_pages = __calc_nr_pages_ring_buffer_desc(buffer_size);
 	struct ring_buffer_desc *rbdesc;
 
 	return size_add(offsetof(struct trace_buffer_desc, __data),

index 75fa1ffc4c96..c17902e42ef1 100644
@@ -980,7 +980,7 @@ int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size,
 			      const struct cpumask *cpumask)
 {
 	size_t min_desc_size = trace_buffer_desc_size(buffer_size, cpumask_weight(cpumask));
-	unsigned int nr_pages = max(DIV_ROUND_UP(buffer_size, PAGE_SIZE), 2UL) + 1;
+	unsigned int nr_pages = __calc_nr_pages_ring_buffer_desc(buffer_size);
 	struct ring_buffer_desc *rb_desc;
 	int cpu, ret = -ENOMEM;
 

2.53.0

From: Vincent Donnefort <[email protected]>

The number of pages per remote ring buffer is capped by ring_buffer_desc::nr_page_va (32 bits). A buffer_size large enough to overflow that field would silently allocate a descriptor smaller than what was asked for.

Return SIZE_MAX from trace_buffer_desc_size() on nr_page_va overflow.

---
 include/linux/ring_buffer.h | 4 ++++
 kernel/trace/trace_remote.c | 6 +++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

index 11bffb6a142d..eac3e9080c3c 100644
@@ -295,6 +295,10 @@ static inline size_t trace_buffer_desc_size(size_t buffer_size, unsigned int nr_
 	unsigned long nr_pages = __calc_nr_pages_ring_buffer_desc(buffer_size);
 	struct ring_buffer_desc *rbdesc;
 
+	/* Capped by ring_buffer_desc::nr_page_va */
+	if (nr_pages > UINT_MAX)
+		return SIZE_MAX;
+
 	return size_add(offsetof(struct trace_buffer_desc, __data),
 			size_mul(nr_cpus, struct_size(rbdesc, page_va, nr_pages)));
 }

index c17902e42ef1..2d5bc423efca 100644
@@ -980,9 +980,12 @@ int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size,
 			      const struct cpumask *cpumask)
 {
 	size_t min_desc_size = trace_buffer_desc_size(buffer_size, cpumask_weight(cpumask));
-	unsigned int nr_pages = __calc_nr_pages_ring_buffer_desc(buffer_size);
 	struct ring_buffer_desc *rb_desc;
 	int cpu, ret = -ENOMEM;
+	unsigned int nr_pages;
+
+	if (min_desc_size == SIZE_MAX)
+		return -E2BIG;
 
 	if (desc_size < min_desc_size)
 		return -EINVAL;

@@ -991,6 +994,7 @@ int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size,
 	desc->struct_len = min_desc_size;
 
 	rb_desc = __first_ring_buffer_desc(desc);
+	nr_pages = __calc_nr_pages_ring_buffer_desc(buffer_size);
 
 	for_each_cpu(cpu, cpumask) {
 		unsigned int id;

2.53.0

From: David Carlier <[email protected]>

ring_buffer_subbuf_order_set() stores the new order and only then walks the CPUs, returning -EBUSY if any of them has resizing disabled. A user mapped buffer has resizing disabled, and __rb_map_vma() reads buffer->subbuf_order without buffer->mutex, so an mmap of an already mapped CPU racing the failing order change sizes the mapping with the new order and inserts pages past the sub-buffer into the VMA.

Check the CPUs before storing the new order.

---
 kernel/trace/ring_buffer.c | 8 ++++++++
 1 file changed, 8 insertions(+)

index 9bc8ce8c5676..04bb94c29f58 100644
@@ -7473,6 +7473,14 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
 
 	old_capacity = rb_subbuf_capacity(buffer);
 
+	/* The mmap fast path reads subbuf_order without buffer->mutex. */
+	for_each_buffer_cpu(buffer, cpu) {
+		if (!cpumask_test_cpu(cpu, buffer->cpumask))
+			continue;
+		if (atomic_read(&buffer->buffers[cpu]->resize_disabled))
+			return -EBUSY;
+	}
+
 	atomic_inc(&buffer->record_disabled);
 
 	/* Make sure all commits have finished */

2.53.0