tracing: Fixes for v7.3

Fixes for v7.3:

- Fix NULL dereference when copying histogram field variable

If event_hist_trigger_parse() creates an action that uses one of pseudo fields for its keys, the hist_data field element will have a NULL for its field member. When looking to match that key, the lookup uses key_field->field->name which will dereference a NULL pointer. Instead use the helper function that handles such cases: expr_field_str().

- Take the trace_types_lock when dropping the buffer_meta reference

The tracing_buffer_meta_open() error return drops the trace_array reference with __trace_array_put() instead of trace_array_put() where the former doesn't take the lock and the latter does.

- Fix NULL pointer dereference when reading per_cpu/cpuN/buffer_meta

These files are created for every possible CPU. But it only contains data for CPUs that have come online. If one is read for a CPU that hasn't come online yet, it triggers a NULL pointer dereference.

Have reading of CPUs that have not come on line yet return -ENODEV.

- Fix an out-of-bounds shift in the log2 histogram modifier

Histograms can take a log2 modifier to have the buckets defined by log2 increments. If a shift is greater than ULONG_MAX or above 2^63, the key falls into a lower order bucket.

Use order_base_2() instead of "ilog2(n - 1) + 1" to have all overflowed keys go into the top bucket.

- Fix the persistent ring buffer rb_meta_subbuf_idx() accounting

The algorithm for finding the index of a sub-buffer in a persistent ring buffer buffer did not take into account the meta data at the beginning of the buffer causing the wrong index to be returned.

As rb_subbufs_from_meta() does the proper accounting to find the first sub-buffer, use that function instead of repeating the logic (incorrectly) and make the calculations based off of that.

- Fix the symbol name checks for the remote buffer code

The remote ring buffer has a build time check to make sure its not using any code that will not be in the hypervisor. For various architectures, there has been false positive reports from this checker.

Fix them.

- Fix false warning in ring_buffer_map_get_reader()

There is a WARN_ON() after rb_get_reader_page() if no reader is returned. But there are legitimate cases where this can happen. Remove the WARN_ON() and also a check that covers one of the legitimate cases that tries to exit out early. There's no need for the check if it just simply allows the return of NULL from rb_get_reader_page().

- Fix the ignored return value from resize_buffer_duplicate_size()

Some clean up of the code in tracing_snapshot_write() removed checking the return value of resize_buffer_duplicate_size() and if that failed it would not be detected.

- Fix the histogram timestamps for buffered events

The histogram timestamps are taken from the events that are recorded. It currently reads ring_buffer_event_time_stamp() but if the event is buffered it does not read it properly. Use tracing_event_time_stamp() which handles both buffered and normal events.

- Do not initialize tracefs if tracing is disabled at boot

If tracing fails to allocate or some other error is detected, tracing_disabled is set. Do not initialize tracefs in this case as it may expect data to be initialized but is not.

- Free eventfs inode after an RCU grace period

Currently eventfs_inodes are freed after a SRCU grace period as the iterations in the code use SRCU. There are locations that the inode may still be accessed from the VFS that uses RCU which does not have the same synchronization as SRCU. Add an RCU wait before free eventfs inodes.

- Do not fail persistent and remote buffers under memory pressure

As persistent and remote buffers have their own pre-allocated memory, there's no reason to check how much memory is available before created them. By checking the memory pressure, these buffers may fail to be created even though their memory is already present.

- Account for subbuffer size when checking how much memory is available

Before allocating a ring buffer, the size is checked against the current memory pressure of the system. But it assumes every sub-buffer is of page size. This is no longer the case. Use the actual sub-buffer size when calculating how much memory will be used.

- Reject non-stack fields for synthetic stacktrace fields

Stack traces are special fields and the checks currently do not do enough to make sure that the fields being processed as stack traces are indeed stack traces. Update the logic to properly detect what is or is not a stack trace.

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

Head SHA1: a244c241593b5b38a08f0df2cb43c26574efbdcb

Amit Machhiwal (1):
tracing: Do not initialize tracefs work if tracing is disabled

Christian Brauner (1):
eventfs: Free the eventfs_inode after an rcu grace period too

David Carlier (2):
tracing: Take trace_types_lock when dropping the buffer_meta reference
ring-buffer: Do not show buffer_meta for CPUs without a buffer

Dmitry Kandybka (1):
tracing: Fix ignored return value of resize_buffer_duplicate_size()

Donggeun Yoo (3):
tracing: Fix NULL dereference when copying keys for a field variable
tracing: Fix the shift out of bounds in the log2 histogram modifier
ring-buffer: Fix the sub-buffer array base in rb_meta_subbuf_idx()

Krystian Kaniewski (1):
ring-buffer: Fix false warning in ring_buffer_map_get_reader()

Lee Jones (1):
tracing/histograms: Reject non-stacktrace fields for synthetic stacktrace fields

Tengda Wu (1):
tracing: Fix hist trigger timestamps for buffered events

Vincent Donnefort (5):
tracing: Fix empty symbol names in simple_ring_buffer undef check
tracing: Fix simple_ring_buffer undef check on parisc
tracing: Fix simple_ring_buffer undef check on sparc64
ring-buffer: Skip si_mem_available() for persistent and remote buffers
ring-buffer: Use subbuf_order in available memory estimation

----

 fs/tracefs/event_inode.c         | 10 ++++++-
 kernel/trace/Makefile            |  5 ++--
 kernel/trace/ring_buffer.c       | 60 +++++++++++++++++++---------------------
 kernel/trace/trace.c             |  8 +++++-
 kernel/trace/trace_events_hist.c | 21 +++++++++-----
 kernel/trace/trace_snapshot.c    |  5 +++-
 kernel/trace/undefsyms_base.c    |  5 +++-
 7 files changed, 70 insertions(+), 44 deletions(-)

From: Donggeun Yoo <[email protected]>

In event_hist_trigger_parse() where it needs to create actions like "onmatch", it calls:

  event_hist_trigger_parse() {
    create_actions() {
      action_create() {
        trace_action_create() {
          trace_action_create_field_var() {
            create_field_var_hist()

Where create_field_var_hist() does a loop on the hist_data representing the keys. The issue is, if the keys uses one of the pseudo field types (like common_cpu), the hist_data field element will have NULL for its field member causing a NULL pointer dereference when accessing the key_field->field->name.

  # echo 'hist:keys=common_cpu:ts0=common_timestamp.usecs' > \
      events/sched/sched_waking/trigger
  # echo 'my_synth u64 lat; int prio' > synthetic_events
  # echo 'hist:keys=common_cpu:wakeup_lat=common_timestamp.usecs-$ts0:\
      onmatch(sched.sched_waking).my_synth($wakeup_lat,prio)' > \
      events/sched/sched_switch/trigger

  KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
  RIP: 0010:create_field_var_hist+0x771/0x1380
  Call Trace:
   trace_action_create_field_var+0x360/0x600
   trace_action_create+0x393/0xe80
   event_hist_trigger_parse+0x3e47/0x69e0
   trigger_process_regex+0x1a6/0x250
   event_trigger_write+0xce/0x160

Instead of accessing it directly, use the proper handler expr_field_str() to get the name.

The three previous patches are prerequisites: without them a .buckets key and a common_stacktrace key each render into a command that cannot be parsed back.

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

index 8af97fd4ee2d..b3df8862855f 100644
@@ -3090,7 +3090,7 @@ create_field_var_hist(struct hist_trigger_data *target_hist_data,
 		key_field = hist_data->fields[i];
 		if (!first)
 			seq_buf_putc(&s, ',');
-		seq_buf_puts(&s, key_field->field->name);
+		expr_field_str(key_field, &s);
 		first = false;
 	}
 

2.53.0

From: David Carlier <[email protected]>

tracing_buffer_meta_open() drops the trace_array reference with __trace_array_put() when ring_buffer_meta_seq_init() fails, without holding trace_types_lock. Use trace_array_put() like every other open error path.

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

index e4a490d3d08c..8645298579c3 100644
@@ -5881,7 +5881,7 @@ static int tracing_buffer_meta_open(struct inode *inode, struct file *filp)
 
 	ret = ring_buffer_meta_seq_init(filp, tr->array_buffer.buffer, cpu);
 	if (ret < 0)
-		__trace_array_put(tr);
+		trace_array_put(tr);
 	return ret;
 }
 

2.53.0

From: David Carlier <[email protected]>

The per_cpu/cpuN/buffer_meta files are created for every possible CPU, but buffer->buffers[cpu] is only allocated once that CPU comes online. Reading the file of a CPU that never came online dereferences a NULL cpu_buffer in rbm_start().

Fail the open with -ENODEV when the CPU has no buffer, as the other per CPU ring buffer interfaces do.

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

index 04bb94c29f58..ce179619e655 100644
@@ -2380,6 +2380,9 @@ int ring_buffer_meta_seq_init(struct file *file, struct trace_buffer *buffer, in
 	struct seq_file *m;
 	int ret;
 
+	if (!cpumask_test_cpu(cpu, buffer->cpumask))
+		return -ENODEV;
+
 	ret = seq_open(file, &rb_meta_seq_ops);
 	if (ret)
 		return ret;

2.53.0

From: Donggeun Yoo <[email protected]>

The .log2 key modifier files a value under its base-2 order:

  # echo 'hist:keys=bytes_req.log2' > events/kmem/kmalloc/trigger

hist_field_log2() computes that as ilog2(roundup_pow_of_two(val)). The out-of-line form of roundup_pow_of_two() is

	return 1UL << fls_long(n - 1);

which shifts by 64 on a 64-bit kernel both when n is 0, where n - 1 wraps to ULONG_MAX, and when n is above 2^63. Both are ordinary keys, because the field fetch hands every value over as a u64: a syscall argument of 0 reaches the first, and one above 2^63 reaches the second.

Keying sys_enter_lseek on offset.log2 and passing 0, 1, 2, 3, 1000, 2^63+1 and U64_MAX gives

  { offset: ~ 2^1  } hitcount:          1
  { offset: ~ 2^10 } hitcount:          1
  { offset: ~ 2^2  } hitcount:          1
  { offset: ~ 2^0  } hitcount:          4

Keys above 2^63 land in the lowest bucket. With CONFIG_UBSAN_SHIFT=y the shift is also reported:

  UBSAN: shift-out-of-bounds in include/linux/log2.h:57:13
  shift exponent 64 is too large for 64-bit type 'long unsigned int'
   __ubsan_handle_shift_out_of_bounds.cold+0xdd/0x1cb
   hist_fn_call.cold+0x8b/0xd9
   event_hist_trigger+0x1cc/0x790
   ftrace_syscall_enter+0x197/0x360
   do_syscall_64+0x402/0x4b0

Use order_base_2(), which is ilog2(n - 1) + 1 for n > 1 and 0 below that. It returns what the old expression returned on every input the old one was defined for. Where it was not, 0 stays in bucket 0, which is where x86_64 had been putting it, and the keys above 2^63 move into bucket 64.

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

index b3df8862855f..71d7f43a7964 100644
@@ -289,7 +289,7 @@ static u64 hist_field_log2(struct hist_field *hist_field,
 
 	u64 val = hist_fn_call(operand, elt, buffer, rbe, event);
 
-	return (u64) ilog2(roundup_pow_of_two(val));
+	return order_base_2(val);
 }
 
 static u64 hist_field_bucket(struct hist_field *hist_field,

2.53.0

From: Donggeun Yoo <[email protected]>

On a persistent ring buffer, rb_meta_subbuf_idx() can return an index one sub-buffer too high. rb_setup_ids_meta_page() then fills subbuf_ids[] shifted by one and leaves subbuf_ids[0] NULL, which __rb_map_vma() dereferences when userspace maps trace_pipe_raw. rb_update_meta_reader() writes two such indices into meta->buffers[], which rb_cpu_meta_valid() would reject on the next boot, discarding the previous boot's trace.

A CPU's sub-buffer array starts after its meta header and the nr_subbufs integers that follow, aligned up to a sub-buffer. rb_meta_subbuf_idx() inverts that calculation but skips only the integers, not the header, so its base is one sub-buffer low whenever the omitted header size crosses the alignment boundary.

Invert the calculation with rb_subbufs_from_meta() rather than repeating it. rb_range_buffer() maps an index back to an address through the same helper, so the two directions can no longer disagree.

---
 kernel/trace/ring_buffer.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

index ce179619e655..ad3bc48e2670 100644
@@ -3630,10 +3630,8 @@ static void rb_inc_iter(struct ring_buffer_iter *iter)
 /* Return the index into the sub-buffers for a given sub-buffer */
 static int rb_meta_subbuf_idx(struct ring_buffer_cpu_meta *meta, void *subbuf)
 {
-	void *subbuf_array;
+	void *subbuf_array = rb_subbufs_from_meta(meta);
 
-	subbuf_array = (void *)meta + sizeof(int) * meta->nr_subbufs;
-	subbuf_array = (void *)ALIGN((unsigned long)subbuf_array, meta->subbuf_size);
 	return (subbuf - subbuf_array) / meta->subbuf_size;
 }
 

2.53.0

From: Vincent Donnefort <[email protected]>

Sparc64 emits empty symmbol names. Skips them when establishing the allowlist and when extracing undefined symbols.

---
 kernel/trace/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index f934ff586bd4..0e1846f7dda3 100644
@@ -146,11 +146,11 @@ KASAN_SANITIZE_undefsyms_base.o := y
 
 UNDEFINED_ALLOWLIST = __asan __gcov __kasan __kcsan __hwasan __sancov __sanitizer __tsan __ubsan __msan \
 		      __aeabi_unwind_cpp __s390_indirect_jump __x86_indirect_thunk simple_ring_buffer \
-		      $(shell $(NM) -u $(obj)/undefsyms_base.o 2>/dev/null | awk '{print $$2}')
+		      $(shell $(NM) -u $(obj)/undefsyms_base.o 2>/dev/null | awk '$$2 {print $$2}')
 
 quiet_cmd_check_undefined = NM      $<
       cmd_check_undefined = \
-          undefsyms=$$($(NM) -u $< | grep -v $(addprefix -e , $(UNDEFINED_ALLOWLIST)) || true); \
+          undefsyms=$$($(NM) -u $< | awk '$$2 {print $$2}' | grep -v $(addprefix -e , $(UNDEFINED_ALLOWLIST)) || true); \
           if [ -n "$$undefsyms" ]; then \
               echo "Unexpected symbols in $<:" >&2; \
               echo "$$undefsyms" >&2; \

2.53.0

From: Vincent Donnefort <[email protected]>

simple_ring_buffer uses local_set() and cmpxchg() on unsigned long. Add them to undefsyms_base.c to extend the allowlist for the parisc architecture.

---
 kernel/trace/undefsyms_base.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

index e65baf58e6ff..37310baf706b 100644
@@ -9,6 +9,7 @@
 
 #include <linux/atomic.h>
 #include <linux/string.h>
+#include <asm/local.h>
 #include <asm/page.h>
 
 void undefsyms_base(void *p, int n);

@@ -18,11 +19,13 @@ static char page[PAGE_SIZE] __aligned(PAGE_SIZE);
 void undefsyms_base(void *p, int n)
 {
 	char buffer[256] = { 0 };
-
 	u32 u = 0;
+
 	memset((char * volatile)page, 8, PAGE_SIZE);
 	memset((char * volatile)buffer, 8, sizeof(buffer));
 	memcpy((void * volatile)p, buffer, sizeof(buffer));
 	cmpxchg((u32 * volatile)&u, 0, 8);
+	cmpxchg((unsigned long *)p, 0, 8);
+	local_set((local_t *)p, 8);
 	WARN_ON(n == 0xdeadbeef);
 }

2.53.0

From: Vincent Donnefort <[email protected]>

simple_ring_buffer relies on headers that end up including <asm/thread_info.h>. On sparc64, this emits the undefined symbol current_thread_info_reg. Allow this symbol.

---
 kernel/trace/Makefile | 1 +
 1 file changed, 1 insertion(+)

index 0e1846f7dda3..3860bc53a4be 100644
@@ -146,6 +146,7 @@ KASAN_SANITIZE_undefsyms_base.o := y
 
 UNDEFINED_ALLOWLIST = __asan __gcov __kasan __kcsan __hwasan __sancov __sanitizer __tsan __ubsan __msan \
 		      __aeabi_unwind_cpp __s390_indirect_jump __x86_indirect_thunk simple_ring_buffer \
+		      current_thread_info_reg \
 		      $(shell $(NM) -u $(obj)/undefsyms_base.o 2>/dev/null | awk '$$2 {print $$2}')
 
 quiet_cmd_check_undefined = NM      $<

2.53.0

From: Krystian Kaniewski <[email protected]>

The mmap reader can warn when it catches up with a writer that is still committing events. rb_get_reader_page() returns NULL in this case, but ring_buffer_map_get_reader() treats that result as an error.

The initial rb_per_cpu_empty() check filters out an empty buffer, but it does not guarantee that the next reader page has committed data. Writers do not take reader_lock and can advance commit_page before publishing the committed length on the new page. rb_get_reader_page() can swap to that page, catch up with the writer and return NULL.

Handle NULL through the existing no-data exit without warning. Remove the caller's reader_page == commit_page check as well, since the reader-page helper already handles that case. The existing metadata update and return path remain in place.

---
 kernel/trace/ring_buffer.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

index ad3bc48e2670..e2b721d0707f 100644
@@ -7998,12 +7998,8 @@ int ring_buffer_map_get_reader(struct trace_buffer *buffer, int cpu)
 		goto out;
 	}
 
-	/* Did the reader catch up with the writer? */
-	if (cpu_buffer->reader_page == cpu_buffer->commit_page)
-		goto out;
-
 	reader = rb_get_reader_page(cpu_buffer);
-	if (WARN_ON(!reader))
+	if (!reader)
 		goto out;
 
 	/* Check if any events were dropped */

2.53.0

From: Dmitry Kandybka <[email protected]>

When tr->allocated_snapshot is true, resize_buffer_duplicate_size() is called to update the snapshot buffer size. However, its return value was not checked. In case of a failed resize this could lead next snapshot being incomplete.

Add resize_buffer_duplicate_size() result check and return error if resize fails.

Compile tested only.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

---
 kernel/trace/trace_snapshot.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

index 07b43c9863a2..4731ef7325a5 100644
@@ -675,9 +675,12 @@ tracing_snapshot_write(struct file *filp, const char __user *ubuf, size_t cnt,
 		if (iter->cpu_file != RING_BUFFER_ALL_CPUS)
 			return -EINVAL;
 #endif
-		if (tr->allocated_snapshot)
+		if (tr->allocated_snapshot) {
 			ret = resize_buffer_duplicate_size(&tr->snapshot_buffer,
 					&tr->array_buffer, iter->cpu_file);
+			if (ret < 0)
+				return ret;
+		}
 
 		ret = tracing_arm_snapshot_locked(tr);
 		if (ret)

2.53.0

From: Tengda Wu <[email protected]>

Syzkaller triggered a WARN_ON_ONCE(!nest) warning:

  WARNING: kernel/trace/ring_buffer.c:821 at ring_buffer_event_time_stamp
  Call trace:
    ring_buffer_event_time_stamp
    hist_field_timestamp
    hist_fn_call
    event_hist_trigger
    event_triggers_call
    __event_trigger_test_discard
    trace_event_buffer_commit
    do_trace_event_raw_event_sched_switch
    trace_event_raw_event_sched_switch

The warning is probabilistically reproducible and is related to syzkaller executing the following commands:

echo 'prev_pid == 999999' > events/sched/sched_switch/filter echo 'hist:keys=common_timestamp' > events/sched/sched_switch/trigger

The specific triggering process is as follows:

CPU 0 CPU 1
(context switching) (echo 'hist:keys=common_timestamp' > \
events/sched/sched_switch/trigger)
do_trace_event_raw_event_##call
trace_event_buffer_lock_reserve

    if (!tr->no_filter_buffering_ref &&
        trace_file->flags & EVENT_FILE_FL_FILTERED)
        return entry;  // return directly
                                                event_hist_trigger_parse
                                                  hist_register_trigger
                                                    tracing_set_filter_buffering(file->tr, true);
                                                      tr->no_filter_buffering_ref++;
    __trace_buffer_lock_reserve
      rb_start_commit
        local_inc(&cpu_buffer->committing);  // skipped and not executed

trace_event_buffer_commit
__event_trigger_test_discard
event_triggers_call
event_hist_trigger
hist_fn_call
hist_field_timestamp
ring_buffer_event_time_stamp
nest = local_read(&cpu_buffer->committing); WARN_ON_ONCE(!nest) // trigger warning

The root cause is that when an event file has a filter attached and the hist trigger has not yet been attached, trace_event_buffer_lock_reserve() writes the event into the per-CPU trace_buffered_event temp buffer instead of reserving it in the ring buffer, so cpu_buffer->committing is not incremented. When the event is later processed at commit time by a hist trigger, hist_field_timestamp() -> ring_buffer_event_time_stamp() sees a zero committing count, triggering WARN_ON_ONCE(!nest).

tracing_event_time_stamp() was added by commit d8279bfc5e959 ("tracing:
Add tracing_event_time_stamp() API") for exactly this case: if the event is the per-CPU trace_buffered_event, it returns the current ring buffer timestamp. But it never gained a caller. Use it in hist_field_timestamp(), so that a hist trigger that races onto a buffered event records the current time, which is only marginally later than when the event was recorded, instead of warning and recording a bogus timestamp.

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

index 71d7f43a7964..571a57e0f369 100644
@@ -873,7 +873,7 @@ static u64 hist_field_timestamp(struct hist_field *hist_field,
 	struct hist_trigger_data *hist_data = hist_field->hist_data;
 	struct trace_array *tr = hist_data->event_file->tr;
 
-	u64 ts = ring_buffer_event_time_stamp(buffer, rbe);
+	u64 ts = tracing_event_time_stamp(buffer, rbe);
 
 	if (hist_data->attrs->ts_in_usecs && trace_clock_in_ns(tr))
 		ts = ns2usecs(ts);

2.53.0

From: Amit Machhiwal <[email protected]>

If trace buffer allocation fails during early boot (e.g. in early_trace_init() -> allocate_trace_buffers()), tracing_disabled is left set to 1 and the global trace array events list remains uninitialized.

Later during boot, tracer_init_tracefs() (invoked via fs_initcall) queues tracerfs_init_work asynchronously without checking if tracing was disabled. When tracer_init_tracefs_work_func() runs, it calls event_trace_init() and init_tracer_tracefs(&global_trace, NULL) unconditionally. This leads to __find_event_file() dereferencing an uninitialized tr->events list, causing an early boot NULL pointer dereference Oops:

  BUG: Kernel NULL pointer dereference on read at 0x00000010
  Faulting instruction address: 0xc000000000485be0
  Oops: Kernel access of bad area, sig: 7 [#1]
  ...
  NIP [c000000000485be0] __find_event_file+0x70/0x3c0
  LR  [c000000000448874] init_tracer_tracefs+0x274/0xc80
  Call Trace:
   init_tracer_tracefs+0x274/0xc80
   tracer_init_tracefs_work_func+0x50/0x320
   process_one_work+0x1e8/0x5c0
   worker_thread+0x1dc/0x3d0
   kthread+0x194/0x1b0
   start_kernel_thread+0x14/0x18

Check tracing_disabled at the beginning of tracer_init_tracefs_work_func() and return immediately if tracing is disabled or failed to initialize, preventing the crash.

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

index 8645298579c3..3b22e70c6016 100644
@@ -9287,6 +9287,12 @@ static struct notifier_block trace_module_nb = {
 
 static __init void tracer_init_tracefs_work_func(struct work_struct *work)
 {
+	/*
+	 * Do not attempt to initialize tracefs if tracing was disabled or
+	 * failed to allocate
+	 */
+	if (tracing_disabled)
+		return;
 
 	event_trace_init();
 

2.53.0

From: Christian Brauner <[email protected]>

tracefs_d_revalidate() reads ->is_freed of the eventfs_inode that dentry->d_fsdata points to, and ->d_revalidate() is called in rcu pathwalk mode with nothing but rcu_read_lock() held. The dentry itself stays around for such a walker, but tracefs_d_release() drops its reference to the eventfs_inode when the dentry is killed, and release_ei() then frees it after a grace period of eventfs_srcu. An SRCU grace period doesn't wait for rcu_read_lock() sections, so the walker can read an eventfs_inode that is already gone.

With a pathwalk held up in tracefs_d_revalidate() while the dentry goes away:

  BUG: KASAN: slab-use-after-free in tracefs_d_revalidate+0x76/0x90
  Read of size 1 at addr ff1100010152d8d0 by task walker/97
  Call Trace:
   <TASK>
   tracefs_d_revalidate+0x76/0x90
   lookup_fast+0x320/0x640
   path_lookupat+0x160/0xb00
   filename_lookup+0x1f1/0x5b0
   vfs_statx+0xd7/0x3b0
   vfs_fstatat+0x4d/0x90
   __do_sys_newfstatat+0xa0/0x100
   do_syscall_64+0x10e/0x5e0
   entry_SYSCALL_64_after_hwframe+0x77/0x7f
  [...]
  Allocated by task 1:
   __kmalloc_cache_noprof+0x249/0x560
   eventfs_create_dir+0x7a/0x590
   event_create_dir+0x2fb/0xcf0
   trace_add_event_call+0xbf/0x150
  [...]
  Freed by task 11:
   kfree+0x1db/0x5b0
   srcu_invoke_callbacks+0x3a3/0x620
   process_one_work+0x937/0x1810
   worker_thread+0x4f1/0xd60

Before commit 8e556432477e ("eventfs: Use SRCU for freeing eventfs_inodes") release_ei() used call_rcu(), which covered the walker, while eventfs_iterate() needs the SRCU grace period. Wait for both and chain call_rcu() from the SRCU callback.

---
 fs/tracefs/event_inode.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

index 6e3513b13cfa..090aca946717 100644
@@ -91,6 +91,14 @@ static void free_ei_rcu(struct rcu_head *rcu)
 	}
 }
 
+/* read under eventfs_srcu and, by tracefs_d_revalidate(), in rcu pathwalk */
+static void free_ei_srcu(struct rcu_head *rcu)
+{
+	struct eventfs_inode *ei = container_of(rcu, struct eventfs_inode, rcu);
+
+	call_rcu(&ei->rcu, free_ei_rcu);
+}
+
 /*
  * eventfs_inode reference count management.
  *

@@ -112,7 +120,7 @@ static void release_ei(struct kref *ref)
 			entry->release(entry->name, ei->data);
 	}
 
-	call_srcu(&eventfs_srcu, &ei->rcu, free_ei_rcu);
+	call_srcu(&eventfs_srcu, &ei->rcu, free_ei_srcu);
 }
 
 static inline void put_ei(struct eventfs_inode *ei)

2.53.0

From: Vincent Donnefort <[email protected]>

Persistent and remote buffer memory is pre-allocated by the time __rb_allocate_pages() is called. Skip si_mem_available() to avoid wrongly failing the allocation under memory pressure.

For the same reason, unset user_thread.

---
 kernel/trace/ring_buffer.c | 41 +++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

index e2b721d0707f..cbf67183c2e7 100644
@@ -2445,20 +2445,30 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
 	struct trace_buffer *buffer = cpu_buffer->buffer;
 	struct ring_buffer_cpu_meta *meta = NULL;
 	struct buffer_page *bpage, *tmp;
-	bool user_thread = current->mm != NULL;
 	struct ring_buffer_desc *desc = NULL;
+	bool user_thread = false;
 	long i;
 
-	/*
-	 * Check if the available memory is there first.
-	 * Note, si_mem_available() only gives us a rough estimate of available
-	 * memory. It may not be accurate. But we don't care, we just want
-	 * to prevent doing any allocation when it is obvious that it is
-	 * not going to succeed.
-	 */
-	i = si_mem_available();
-	if (i < nr_pages)
-		return -ENOMEM;
+	if (buffer->range_addr_start) {
+		meta = rb_range_meta(buffer, nr_pages, cpu_buffer->cpu);
+	} else if (buffer->remote) {
+		desc = ring_buffer_desc(buffer->remote->desc, cpu_buffer->cpu);
+		if (!desc || WARN_ON(desc->nr_page_va != (nr_pages + 1)))
+			return -EINVAL;
+	} else {
+		/*
+		 * Check if the available memory is there first.
+		 * Note, si_mem_available() only gives us a rough estimate of available
+		 * memory. It may not be accurate. But we don't care, we just want
+		 * to prevent doing any allocation when it is obvious that it is
+		 * not going to succeed.
+		 */
+		i = si_mem_available();
+		if (i < nr_pages)
+			return -ENOMEM;
+
+		user_thread = current->mm != NULL;
+	}
 
 	/*
 	 * If a user thread allocates too much, and si_mem_available()

@@ -2472,15 +2482,6 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
 	if (user_thread)
 		set_current_oom_origin();
 
-	if (buffer->range_addr_start)
-		meta = rb_range_meta(buffer, nr_pages, cpu_buffer->cpu);
-
-	if (buffer->remote) {
-		desc = ring_buffer_desc(buffer->remote->desc, cpu_buffer->cpu);
-		if (!desc || WARN_ON(desc->nr_page_va != (nr_pages + 1)))
-			return -EINVAL;
-	}
-
 	for (i = 0; i < nr_pages; i++) {
 
 		bpage = alloc_cpu_page(cpu_buffer->cpu);

2.53.0

From: Vincent Donnefort <[email protected]>

To avoid failing in the middle of an allocation of a ring-buffer, __rb_allocate_pages() looks at the available memory estimation with si_mem_available(). Use the subbuffer order with this estimation to fail early if necessary for ring-buffers with an order > 0.

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

index cbf67183c2e7..9c4e40847e43 100644
@@ -2443,6 +2443,7 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
 			       unsigned long nr_pages, struct list_head *pages)
 {
 	struct trace_buffer *buffer = cpu_buffer->buffer;
+	unsigned int subbuf_order = buffer->subbuf_order;
 	struct ring_buffer_cpu_meta *meta = NULL;
 	struct buffer_page *bpage, *tmp;
 	struct ring_buffer_desc *desc = NULL;

@@ -2464,7 +2465,7 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
 		 * not going to succeed.
 		 */
 		i = si_mem_available();
-		if (i < nr_pages)
+		if (i < (nr_pages << subbuf_order))
 			return -ENOMEM;
 
 		user_thread = current->mm != NULL;

@@ -2517,12 +2518,11 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
 			bpage->id = i + 1;
 			cpu_buffer->subbuf_ids[i + 1] = bpage;
 		} else {
-			bpage->page = alloc_cpu_data(cpu_buffer->cpu,
-						     cpu_buffer->buffer->subbuf_order);
+			bpage->page = alloc_cpu_data(cpu_buffer->cpu, subbuf_order);
 			if (!bpage->page)
 				goto free_pages;
 		}
-		bpage->order = cpu_buffer->buffer->subbuf_order;
+		bpage->order = subbuf_order;
 
 		if (user_thread && fatal_signal_pending(current))
 			goto free_pages;

2.53.0

From: Lee Jones <[email protected]>

In check_synth_field(), when a synthetic event field is a stacktrace (field->is_stack) and the histogram field is not a "long[" stacktrace array, validation falls through to the generic size and signedness comparison.

Because parse_synth_field() sets a stacktrace field's type to "__data_loc unsigned long[]", its size is sizeof(u64) and synth_field_signed() marks it as signed. Consequently, any 8-byte signed scalar histogram field passes the fallback check, and its scalar value is subsequently dereferenced as a kernel stacktrace pointer in get_field_size() and trace_stack().

Furthermore, checking strstr(hist_field->type, "long[") is insufficient because tracepoint fields defined with __bitmask() or __rel_bitmask() also have type "__data_loc unsigned long[]" or "__rel_loc unsigned long[]", whilst create_hist_field() treats them as 4-byte scalar values without HIST_FIELD_FL_STACKTRACE.

Propagate HIST_FIELD_FL_STACKTRACE to variable references and field variables (whilst excluding variable aliases from per-element stacktrace capture), and reject any mismatch between HIST_FIELD_FL_STACKTRACE and field->is_stack in check_synth_field().

---
 kernel/trace/trace_events_hist.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

index 571a57e0f369..dcb5d7321095 100644
@@ -2121,7 +2121,8 @@ static int init_var_ref(struct hist_field *ref_field,
 	ref_field->size = var_field->size;
 	ref_field->is_signed = var_field->is_signed;
 	ref_field->flags |= var_field->flags &
-		(HIST_FIELD_FL_TIMESTAMP | HIST_FIELD_FL_TIMESTAMP_USECS);
+		(HIST_FIELD_FL_TIMESTAMP | HIST_FIELD_FL_TIMESTAMP_USECS |
+		 HIST_FIELD_FL_STACKTRACE);
 
 	if (system) {
 		ref_field->system = kstrdup(system, GFP_KERNEL);

@@ -3318,6 +3319,7 @@ static struct field_var *create_field_var(struct hist_trigger_data *hist_data,
 		ret = PTR_ERR(var);
 		goto err;
 	}
+	var->flags |= val->flags & HIST_FIELD_FL_STACKTRACE;
 
 	field_var = kzalloc_obj(struct field_var);
 	if (!field_var) {

@@ -3946,7 +3948,10 @@ static int check_synth_field(struct synth_event *event,
 	    && field->is_dynamic)
 		return 0;
 
-	if (strstr(hist_field->type, "long[") && field->is_stack)
+	if (!!(hist_field->flags & HIST_FIELD_FL_STACKTRACE) != field->is_stack)
+		return -EINVAL;
+
+	if (field->is_stack)
 		return 0;
 
 	if (strcmp(field->type, hist_field->type) != 0) {

@@ -4487,7 +4492,8 @@ static int create_var_field(struct hist_trigger_data *hist_data,
 	if (!ret && hist_data->fields[val_idx]->flags & HIST_FIELD_FL_EXECNAME)
 		update_var_execname(hist_data->fields[val_idx]);
 
-	if (!ret && hist_data->fields[val_idx]->flags &
+	if (!ret && !(hist_data->fields[val_idx]->flags & HIST_FIELD_FL_ALIAS) &&
+	    hist_data->fields[val_idx]->flags &
 	    (HIST_FIELD_FL_STRING | HIST_FIELD_FL_STACKTRACE))
 		hist_data->fields[val_idx]->var_str_idx = hist_data->n_var_str++;
 

@@ -5228,7 +5234,8 @@ static void hist_trigger_elt_update(struct hist_trigger_data *hist_data,
 		if (hist_field->flags & HIST_FIELD_FL_VAR) {
 			var_idx = hist_field->var.idx;
 
-			if (hist_field->flags &
+			if (!(hist_field->flags & HIST_FIELD_FL_ALIAS) &&
+			    hist_field->flags &
 			    (HIST_FIELD_FL_STRING | HIST_FIELD_FL_STACKTRACE)) {
 				unsigned int str_start, var_str_idx, idx;
 				char *str, *val_str;

2.53.0