mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [for-linus][PATCH 00/15] tracing: Fixes for 6.6
@ 2023-09-09  3:16 Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 01/15] tracefs: Add missing lockdown check to tracefs_create_dir() Steven Rostedt
                   ` (14 more replies)
  0 siblings, 15 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel; +Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton

Tracing fixes for 6.6:

- Add missing LOCKDOWN checks for eventfs callers
  When LOCKDOWN is active for tracing, it causes inconsistent state
  when some functions succeed and others fail. 

- Use dput() to free the top level eventfs descriptor
  There was a race between accesses and freeing it.

- Fix a long standing bug that eventfs exposed due to changing timings
  by dynamically creating files. That is, If a event file is opened
  for an instance, there's nothing preventing the instance from being
  removed which will make accessing the files cause use-after-free bugs.

- Fix a ring buffer race that happens when iterating over the ring
  buffer while writers are active. Check to make sure not to read
  the event meta data if it's beyond the end of the ring buffer sub buffer.

- Fix the print trigger that disappeared because the test to create it
  was looking for the event dir field being filled, but now it has the
  "ef" field filled for the eventfs structure.

- Remove the unused "dir" field from the event structure.

- Fix the order of the trace_dynamic_info as it had it backwards for the
  offset and len fields for which one was for which endianess.

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

Head SHA1: 595efe1079cd38a5b7d2762bf6d3bd94105bae0f


Naveen N Rao (1):
      selftests/ftrace: Fix dependencies for some of the synthetic event tests

Steven Rostedt (Google) (13):
      tracefs: Add missing lockdown check to tracefs_create_dir()
      tracefs/eventfs: Add missing lockdown checks
      tracefs/eventfs: Use dput to free the toplevel events directory
      tracing: Increase trace array ref count on enable and filter files
      tracing: Have tracing_max_latency inc the trace array ref count
      tracing: Have current_trace inc the trace array ref count
      tracing: Have option files inc the trace array ref count
      tracing: Have event inject files inc the trace array ref count
      tracefs/eventfs: Free top level files on removal
      ring-buffer: Do not attempt to read past "commit"
      tracing: Use the new eventfs descriptor for print trigger
      tracing: Remove unused trace_event_file dir field
      tracing/synthetic: Fix order of struct trace_dynamic_info

Zheng Yejian (1):
      ring-buffer: Avoid softlockup in ring_buffer_resize()

----
 fs/tracefs/event_inode.c                           | 56 +++++++++++++++--
 fs/tracefs/inode.c                                 |  5 +-
 fs/tracefs/internal.h                              |  5 +-
 include/linux/trace_events.h                       |  7 +--
 kernel/trace/ring_buffer.c                         |  7 +++
 kernel/trace/trace.c                               | 72 +++++++++++++++++++---
 kernel/trace/trace.h                               |  2 +
 kernel/trace/trace_events.c                        | 19 ++----
 kernel/trace/trace_events_inject.c                 |  3 +-
 .../trigger-synthetic-event-dynstring.tc           |  2 +-
 .../trigger-synthetic_event_syntax_errors.tc       |  2 +-
 11 files changed, 140 insertions(+), 40 deletions(-)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [for-linus][PATCH 01/15] tracefs: Add missing lockdown check to tracefs_create_dir()
  2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
@ 2023-09-09  3:16 ` Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 02/15] tracefs/eventfs: Add missing lockdown checks Steven Rostedt
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton, stable,
	Ajay Kaher, Ching-lin Yu, kernel test robot

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The function tracefs_create_dir() was missing a lockdown check and was
called by the RV code. This gave an inconsistent behavior of this function
returning success while other tracefs functions failed. This caused the
inode being freed by the wrong kmem_cache.

Link: https://lkml.kernel.org/r/20230905182711.692687042@goodmis.org
Link: https://lore.kernel.org/all/202309050916.58201dc6-oliver.sang@intel.com/

Cc: stable@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ajay Kaher <akaher@vmware.com>
Cc: Ching-lin Yu <chinglinyu@google.com>
Fixes: bf8e602186ec4 ("tracing: Do not create tracefs files if tracefs lockdown is in effect")
Reported-by: kernel test robot <oliver.sang@intel.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 fs/tracefs/inode.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index de5b72216b1a..3b8dd938b1c8 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -673,6 +673,9 @@ static struct dentry *__create_dir(const char *name, struct dentry *parent,
  */
 struct dentry *tracefs_create_dir(const char *name, struct dentry *parent)
 {
+	if (security_locked_down(LOCKDOWN_TRACEFS))
+		return NULL;
+
 	return __create_dir(name, parent, &simple_dir_inode_operations);
 }
 
-- 
2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [for-linus][PATCH 02/15] tracefs/eventfs: Add missing lockdown checks
  2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 01/15] tracefs: Add missing lockdown check to tracefs_create_dir() Steven Rostedt
@ 2023-09-09  3:16 ` Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 03/15] tracefs/eventfs: Use dput to free the toplevel events directory Steven Rostedt
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton, Ajay Kaher,
	Ching-lin Yu, kernel test robot

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

All the eventfs external functions do not check if TRACEFS_LOCKDOWN was
set or not. This can caused some functions to return success while others
fail, which can trigger unexpected errors.

Add the missing lockdown checks.

Link: https://lkml.kernel.org/r/20230905182711.899724045@goodmis.org
Link: https://lore.kernel.org/all/202309050916.58201dc6-oliver.sang@intel.com/

Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ajay Kaher <akaher@vmware.com>
Cc: Ching-lin Yu <chinglinyu@google.com>
Reported-by: kernel test robot <oliver.sang@intel.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 fs/tracefs/event_inode.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index 237c6f370ad9..fa1a1679a886 100644
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -491,6 +491,9 @@ struct dentry *eventfs_create_events_dir(const char *name,
 	struct tracefs_inode *ti;
 	struct inode *inode;
 
+	if (security_locked_down(LOCKDOWN_TRACEFS))
+		return NULL;
+
 	if (IS_ERR(dentry))
 		return dentry;
 
@@ -538,6 +541,9 @@ struct eventfs_file *eventfs_add_subsystem_dir(const char *name,
 	struct eventfs_inode *ei_parent;
 	struct eventfs_file *ef;
 
+	if (security_locked_down(LOCKDOWN_TRACEFS))
+		return NULL;
+
 	if (!parent)
 		return ERR_PTR(-EINVAL);
 
@@ -569,6 +575,9 @@ struct eventfs_file *eventfs_add_dir(const char *name,
 {
 	struct eventfs_file *ef;
 
+	if (security_locked_down(LOCKDOWN_TRACEFS))
+		return NULL;
+
 	if (!ef_parent)
 		return ERR_PTR(-EINVAL);
 
@@ -606,6 +615,9 @@ int eventfs_add_events_file(const char *name, umode_t mode,
 	struct eventfs_inode *ei;
 	struct eventfs_file *ef;
 
+	if (security_locked_down(LOCKDOWN_TRACEFS))
+		return -ENODEV;
+
 	if (!parent)
 		return -EINVAL;
 
@@ -654,6 +666,9 @@ int eventfs_add_file(const char *name, umode_t mode,
 {
 	struct eventfs_file *ef;
 
+	if (security_locked_down(LOCKDOWN_TRACEFS))
+		return -ENODEV;
+
 	if (!ef_parent)
 		return -EINVAL;
 
-- 
2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [for-linus][PATCH 03/15] tracefs/eventfs: Use dput to free the toplevel events directory
  2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 01/15] tracefs: Add missing lockdown check to tracefs_create_dir() Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 02/15] tracefs/eventfs: Add missing lockdown checks Steven Rostedt
@ 2023-09-09  3:16 ` Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 04/15] tracing: Increase trace array ref count on enable and filter files Steven Rostedt
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton, Ajay Kaher,
	Linux Kernel Functional Testing, Naresh Kamboju, Zheng Yejian

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Currently when rmdir on an instance is done, eventfs_remove_events_dir()
is called and it does a dput on the dentry and then frees the
eventfs_inode that represents the events directory.

But there's no protection against a reader reading the top level events
directory at the same time and we can get a use after free error. Instead,
use the dput() associated to the dentry to also free the eventfs_inode
associated to the events directory, as that will get called when the last
reference to the directory is released.

This issue triggered the following KASAN report:

 ==================================================================
 BUG: KASAN: slab-use-after-free in eventfs_root_lookup+0x88/0x1b0
 Read of size 8 at addr ffff888120130ca0 by task ftracetest/1201

 CPU: 4 PID: 1201 Comm: ftracetest Not tainted 6.5.0-test-10737-g469e0a8194e7 #13
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
 Call Trace:
  <TASK>
  dump_stack_lvl+0x57/0x90
  print_report+0xcf/0x670
  ? __pfx_ring_buffer_record_off+0x10/0x10
  ? _raw_spin_lock_irqsave+0x2b/0x70
  ? __virt_addr_valid+0xd9/0x160
  kasan_report+0xd4/0x110
  ? eventfs_root_lookup+0x88/0x1b0
  ? eventfs_root_lookup+0x88/0x1b0
  eventfs_root_lookup+0x88/0x1b0
  ? eventfs_root_lookup+0x33/0x1b0
  __lookup_slow+0x194/0x2a0
  ? __pfx___lookup_slow+0x10/0x10
  ? down_read+0x11c/0x330
  walk_component+0x166/0x220
  link_path_walk.part.0.constprop.0+0x3a3/0x5a0
  ? seqcount_lockdep_reader_access+0x82/0x90
  ? __pfx_link_path_walk.part.0.constprop.0+0x10/0x10
  path_openat+0x143/0x11f0
  ? __lock_acquire+0xa1a/0x3220
  ? __pfx_path_openat+0x10/0x10
  ? __pfx___lock_acquire+0x10/0x10
  do_filp_open+0x166/0x290
  ? __pfx_do_filp_open+0x10/0x10
  ? lock_is_held_type+0xce/0x120
  ? preempt_count_sub+0xb7/0x100
  ? _raw_spin_unlock+0x29/0x50
  ? alloc_fd+0x1a0/0x320
  do_sys_openat2+0x126/0x160
  ? rcu_is_watching+0x34/0x60
  ? __pfx_do_sys_openat2+0x10/0x10
  ? __might_resched+0x2cf/0x3b0
  ? __fget_light+0xdf/0x100
  __x64_sys_openat+0xcd/0x140
  ? __pfx___x64_sys_openat+0x10/0x10
  ? syscall_enter_from_user_mode+0x22/0x90
  ? lockdep_hardirqs_on+0x7d/0x100
  do_syscall_64+0x3b/0xc0
  entry_SYSCALL_64_after_hwframe+0x6e/0xd8
 RIP: 0033:0x7f1dceef5e51
 Code: 75 57 89 f0 25 00 00 41 00 3d 00 00 41 00 74 49 80 3d 9a 27 0e 00 00 74 6d 89 da 48 89 ee bf 9c ff ff ff b8 01 01 00 00 0f 05 <48> 3d 00 f0 ff ff 0f 87 93 00 00 00 48 8b 54 24 28 64 48 2b 14 25
 RSP: 002b:00007fff2cddf380 EFLAGS: 00000202 ORIG_RAX: 0000000000000101
 RAX: ffffffffffffffda RBX: 0000000000000241 RCX: 00007f1dceef5e51
 RDX: 0000000000000241 RSI: 000055d7520677d0 RDI: 00000000ffffff9c
 RBP: 000055d7520677d0 R08: 000000000000001e R09: 0000000000000001
 R10: 00000000000001b6 R11: 0000000000000202 R12: 0000000000000000
 R13: 0000000000000003 R14: 000055d752035678 R15: 000055d752067788
  </TASK>

 Allocated by task 1200:
  kasan_save_stack+0x2f/0x50
  kasan_set_track+0x21/0x30
  __kasan_kmalloc+0x8b/0x90
  eventfs_create_events_dir+0x54/0x220
  create_event_toplevel_files+0x42/0x130
  event_trace_add_tracer+0x33/0x180
  trace_array_create_dir+0x52/0xf0
  trace_array_create+0x361/0x410
  instance_mkdir+0x6b/0xb0
  tracefs_syscall_mkdir+0x57/0x80
  vfs_mkdir+0x275/0x380
  do_mkdirat+0x1da/0x210
  __x64_sys_mkdir+0x74/0xa0
  do_syscall_64+0x3b/0xc0
  entry_SYSCALL_64_after_hwframe+0x6e/0xd8

 Freed by task 1251:
  kasan_save_stack+0x2f/0x50
  kasan_set_track+0x21/0x30
  kasan_save_free_info+0x27/0x40
  __kasan_slab_free+0x106/0x180
  __kmem_cache_free+0x149/0x2e0
  event_trace_del_tracer+0xcb/0x120
  __remove_instance+0x16a/0x340
  instance_rmdir+0x77/0xa0
  tracefs_syscall_rmdir+0x77/0xc0
  vfs_rmdir+0xed/0x2d0
  do_rmdir+0x235/0x280
  __x64_sys_rmdir+0x5f/0x90
  do_syscall_64+0x3b/0xc0
  entry_SYSCALL_64_after_hwframe+0x6e/0xd8

 The buggy address belongs to the object at ffff888120130ca0
  which belongs to the cache kmalloc-16 of size 16
 The buggy address is located 0 bytes inside of
  freed 16-byte region [ffff888120130ca0, ffff888120130cb0)

 The buggy address belongs to the physical page:
 page:000000004dbddbb0 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x120130
 flags: 0x17ffffc0000800(slab|node=0|zone=2|lastcpupid=0x1fffff)
 page_type: 0xffffffff()
 raw: 0017ffffc0000800 ffff8881000423c0 dead000000000122 0000000000000000
 raw: 0000000000000000 0000000000800080 00000001ffffffff 0000000000000000
 page dumped because: kasan: bad access detected

 Memory state around the buggy address:
  ffff888120130b80: 00 00 fc fc 00 05 fc fc 00 00 fc fc 00 02 fc fc
  ffff888120130c00: 00 07 fc fc 00 00 fc fc 00 00 fc fc fa fb fc fc
 >ffff888120130c80: 00 00 fc fc fa fb fc fc 00 00 fc fc 00 00 fc fc
                                ^
  ffff888120130d00: 00 00 fc fc 00 00 fc fc 00 00 fc fc fa fb fc fc
  ffff888120130d80: 00 00 fc fc 00 00 fc fc 00 00 fc fc 00 00 fc fc
 ==================================================================

Link: https://lkml.kernel.org/r/20230907024803.250873643@goodmis.org
Link: https://lore.kernel.org/all/1cb3aee2-19af-c472-e265-05176fe9bd84@huawei.com/

Cc: Ajay Kaher <akaher@vmware.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Fixes: 5bdcd5f5331a2 eventfs: ("Implement removal of meta data from eventfs")
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Reported-by: Zheng Yejian <zhengyejian1@huawei.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 fs/tracefs/event_inode.c | 17 ++++++++++++-----
 fs/tracefs/inode.c       |  2 +-
 fs/tracefs/internal.h    |  5 +++--
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index fa1a1679a886..609ccb5b7cfc 100644
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -185,17 +185,27 @@ static struct dentry *create_dir(const char *name, struct dentry *parent, void *
 
 /**
  * eventfs_set_ef_status_free - set the ef->status to free
+ * @ti: the tracefs_inode of the dentry
  * @dentry: dentry who's status to be freed
  *
  * eventfs_set_ef_status_free will be called if no more
  * references remain
  */
-void eventfs_set_ef_status_free(struct dentry *dentry)
+void eventfs_set_ef_status_free(struct tracefs_inode *ti, struct dentry *dentry)
 {
 	struct tracefs_inode *ti_parent;
+	struct eventfs_inode *ei;
 	struct eventfs_file *ef;
 
 	mutex_lock(&eventfs_mutex);
+
+	/* The top level events directory may be freed by this */
+	if (unlikely(ti->flags & TRACEFS_EVENT_TOP_INODE)) {
+		ei = ti->private;
+		kfree(ei);
+		goto out;
+	}
+
 	ti_parent = get_tracefs(dentry->d_parent->d_inode);
 	if (!ti_parent || !(ti_parent->flags & TRACEFS_EVENT_INODE))
 		goto out;
@@ -510,7 +520,7 @@ struct dentry *eventfs_create_events_dir(const char *name,
 	INIT_LIST_HEAD(&ei->e_top_files);
 
 	ti = get_tracefs(inode);
-	ti->flags |= TRACEFS_EVENT_INODE;
+	ti->flags |= TRACEFS_EVENT_INODE | TRACEFS_EVENT_TOP_INODE;
 	ti->private = ei;
 
 	inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
@@ -806,7 +816,6 @@ void eventfs_remove(struct eventfs_file *ef)
 void eventfs_remove_events_dir(struct dentry *dentry)
 {
 	struct tracefs_inode *ti;
-	struct eventfs_inode *ei;
 
 	if (!dentry || !dentry->d_inode)
 		return;
@@ -815,8 +824,6 @@ void eventfs_remove_events_dir(struct dentry *dentry)
 	if (!ti || !(ti->flags & TRACEFS_EVENT_INODE))
 		return;
 
-	ei = ti->private;
 	d_invalidate(dentry);
 	dput(dentry);
-	kfree(ei);
 }
diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index 3b8dd938b1c8..891653ba9cf3 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -385,7 +385,7 @@ static void tracefs_dentry_iput(struct dentry *dentry, struct inode *inode)
 
 	ti = get_tracefs(inode);
 	if (ti && ti->flags & TRACEFS_EVENT_INODE)
-		eventfs_set_ef_status_free(dentry);
+		eventfs_set_ef_status_free(ti, dentry);
 	iput(inode);
 }
 
diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h
index 69c2b1d87c46..4f2e49e2197b 100644
--- a/fs/tracefs/internal.h
+++ b/fs/tracefs/internal.h
@@ -3,7 +3,8 @@
 #define _TRACEFS_INTERNAL_H
 
 enum {
-	TRACEFS_EVENT_INODE     = BIT(1),
+	TRACEFS_EVENT_INODE		= BIT(1),
+	TRACEFS_EVENT_TOP_INODE		= BIT(2),
 };
 
 struct tracefs_inode {
@@ -24,6 +25,6 @@ struct inode *tracefs_get_inode(struct super_block *sb);
 struct dentry *eventfs_start_creating(const char *name, struct dentry *parent);
 struct dentry *eventfs_failed_creating(struct dentry *dentry);
 struct dentry *eventfs_end_creating(struct dentry *dentry);
-void eventfs_set_ef_status_free(struct dentry *dentry);
+void eventfs_set_ef_status_free(struct tracefs_inode *ti, struct dentry *dentry);
 
 #endif /* _TRACEFS_INTERNAL_H */
-- 
2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [for-linus][PATCH 04/15] tracing: Increase trace array ref count on enable and filter files
  2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
                   ` (2 preceding siblings ...)
  2023-09-09  3:16 ` [for-linus][PATCH 03/15] tracefs/eventfs: Use dput to free the toplevel events directory Steven Rostedt
@ 2023-09-09  3:16 ` Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 05/15] tracing: Have tracing_max_latency inc the trace array ref count Steven Rostedt
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton, stable,
	Linux Kernel Functional Testing, Naresh Kamboju, Zheng Yejian

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

When the trace event enable and filter files are opened, increment the
trace array ref counter, otherwise they can be accessed when the trace
array is being deleted. The ref counter keeps the trace array from being
deleted while those files are opened.

Link: https://lkml.kernel.org/r/20230907024803.456187066@goodmis.org
Link: https://lore.kernel.org/all/1cb3aee2-19af-c472-e265-05176fe9bd84@huawei.com/

Cc: stable@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Fixes: 8530dec63e7b4 ("tracing: Add tracing_check_open_get_tr()")
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Reported-by: Zheng Yejian <zhengyejian1@huawei.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace.c        | 27 +++++++++++++++++++++++++++
 kernel/trace/trace.h        |  2 ++
 kernel/trace/trace_events.c |  6 ++++--
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 35783a7baf15..0827037ee3b8 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4973,6 +4973,33 @@ int tracing_open_generic_tr(struct inode *inode, struct file *filp)
 	return 0;
 }
 
+/*
+ * The private pointer of the inode is the trace_event_file.
+ * Update the tr ref count associated to it.
+ */
+int tracing_open_file_tr(struct inode *inode, struct file *filp)
+{
+	struct trace_event_file *file = inode->i_private;
+	int ret;
+
+	ret = tracing_check_open_get_tr(file->tr);
+	if (ret)
+		return ret;
+
+	filp->private_data = inode->i_private;
+
+	return 0;
+}
+
+int tracing_release_file_tr(struct inode *inode, struct file *filp)
+{
+	struct trace_event_file *file = inode->i_private;
+
+	trace_array_put(file->tr);
+
+	return 0;
+}
+
 static int tracing_mark_open(struct inode *inode, struct file *filp)
 {
 	stream_open(inode, filp);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 5669dd1f90d9..77debe53f07c 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -610,6 +610,8 @@ void tracing_reset_all_online_cpus(void);
 void tracing_reset_all_online_cpus_unlocked(void);
 int tracing_open_generic(struct inode *inode, struct file *filp);
 int tracing_open_generic_tr(struct inode *inode, struct file *filp);
+int tracing_open_file_tr(struct inode *inode, struct file *filp);
+int tracing_release_file_tr(struct inode *inode, struct file *filp);
 bool tracing_is_disabled(void);
 bool tracer_tracing_is_on(struct trace_array *tr);
 void tracer_tracing_on(struct trace_array *tr);
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index ed367d713be0..2af92177b765 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2103,9 +2103,10 @@ static const struct file_operations ftrace_set_event_notrace_pid_fops = {
 };
 
 static const struct file_operations ftrace_enable_fops = {
-	.open = tracing_open_generic,
+	.open = tracing_open_file_tr,
 	.read = event_enable_read,
 	.write = event_enable_write,
+	.release = tracing_release_file_tr,
 	.llseek = default_llseek,
 };
 
@@ -2122,9 +2123,10 @@ static const struct file_operations ftrace_event_id_fops = {
 };
 
 static const struct file_operations ftrace_event_filter_fops = {
-	.open = tracing_open_generic,
+	.open = tracing_open_file_tr,
 	.read = event_filter_read,
 	.write = event_filter_write,
+	.release = tracing_release_file_tr,
 	.llseek = default_llseek,
 };
 
-- 
2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [for-linus][PATCH 05/15] tracing: Have tracing_max_latency inc the trace array ref count
  2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
                   ` (3 preceding siblings ...)
  2023-09-09  3:16 ` [for-linus][PATCH 04/15] tracing: Increase trace array ref count on enable and filter files Steven Rostedt
@ 2023-09-09  3:16 ` Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 06/15] tracing: Have current_trace " Steven Rostedt
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton, stable,
	Zheng Yejian, Linux Kernel Functional Testing, Naresh Kamboju

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The tracing_max_latency file points to the trace_array max_latency field.
For an instance, if the file is opened and the instance is deleted,
reading or writing to the file will cause a use after free.

Up the ref count of the trace_array when tracing_max_latency is opened.

Link: https://lkml.kernel.org/r/20230907024803.666889383@goodmis.org
Link: https://lore.kernel.org/all/1cb3aee2-19af-c472-e265-05176fe9bd84@huawei.com/

Cc: stable@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Zheng Yejian <zhengyejian1@huawei.com>
Fixes: 8530dec63e7b4 ("tracing: Add tracing_check_open_get_tr()")
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 0827037ee3b8..c8b8b4c6feaf 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1772,7 +1772,7 @@ static void trace_create_maxlat_file(struct trace_array *tr,
 	init_irq_work(&tr->fsnotify_irqwork, latency_fsnotify_workfn_irq);
 	tr->d_max_latency = trace_create_file("tracing_max_latency",
 					      TRACE_MODE_WRITE,
-					      d_tracer, &tr->max_latency,
+					      d_tracer, tr,
 					      &tracing_max_lat_fops);
 }
 
@@ -1805,7 +1805,7 @@ void latency_fsnotify(struct trace_array *tr)
 
 #define trace_create_maxlat_file(tr, d_tracer)				\
 	trace_create_file("tracing_max_latency", TRACE_MODE_WRITE,	\
-			  d_tracer, &tr->max_latency, &tracing_max_lat_fops)
+			  d_tracer, tr, &tracing_max_lat_fops)
 
 #endif
 
@@ -6717,14 +6717,18 @@ static ssize_t
 tracing_max_lat_read(struct file *filp, char __user *ubuf,
 		     size_t cnt, loff_t *ppos)
 {
-	return tracing_nsecs_read(filp->private_data, ubuf, cnt, ppos);
+	struct trace_array *tr = filp->private_data;
+
+	return tracing_nsecs_read(&tr->max_latency, ubuf, cnt, ppos);
 }
 
 static ssize_t
 tracing_max_lat_write(struct file *filp, const char __user *ubuf,
 		      size_t cnt, loff_t *ppos)
 {
-	return tracing_nsecs_write(filp->private_data, ubuf, cnt, ppos);
+	struct trace_array *tr = filp->private_data;
+
+	return tracing_nsecs_write(&tr->max_latency, ubuf, cnt, ppos);
 }
 
 #endif
@@ -7778,10 +7782,11 @@ static const struct file_operations tracing_thresh_fops = {
 
 #ifdef CONFIG_TRACER_MAX_TRACE
 static const struct file_operations tracing_max_lat_fops = {
-	.open		= tracing_open_generic,
+	.open		= tracing_open_generic_tr,
 	.read		= tracing_max_lat_read,
 	.write		= tracing_max_lat_write,
 	.llseek		= generic_file_llseek,
+	.release	= tracing_release_generic_tr,
 };
 #endif
 
-- 
2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [for-linus][PATCH 06/15] tracing: Have current_trace inc the trace array ref count
  2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
                   ` (4 preceding siblings ...)
  2023-09-09  3:16 ` [for-linus][PATCH 05/15] tracing: Have tracing_max_latency inc the trace array ref count Steven Rostedt
@ 2023-09-09  3:16 ` Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 07/15] tracing: Have option files " Steven Rostedt
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton, stable,
	Zheng Yejian, Linux Kernel Functional Testing, Naresh Kamboju

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The current_trace updates the trace array tracer. For an instance, if the
file is opened and the instance is deleted, reading or writing to the file
will cause a use after free.

Up the ref count of the trace array when current_trace is opened.

Link: https://lkml.kernel.org/r/20230907024803.877687227@goodmis.org
Link: https://lore.kernel.org/all/1cb3aee2-19af-c472-e265-05176fe9bd84@huawei.com/

Cc: stable@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Zheng Yejian <zhengyejian1@huawei.com>
Fixes: 8530dec63e7b4 ("tracing: Add tracing_check_open_get_tr()")
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c8b8b4c6feaf..b82df33d20ff 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -7791,10 +7791,11 @@ static const struct file_operations tracing_max_lat_fops = {
 #endif
 
 static const struct file_operations set_tracer_fops = {
-	.open		= tracing_open_generic,
+	.open		= tracing_open_generic_tr,
 	.read		= tracing_set_trace_read,
 	.write		= tracing_set_trace_write,
 	.llseek		= generic_file_llseek,
+	.release	= tracing_release_generic_tr,
 };
 
 static const struct file_operations tracing_pipe_fops = {
-- 
2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [for-linus][PATCH 07/15] tracing: Have option files inc the trace array ref count
  2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
                   ` (5 preceding siblings ...)
  2023-09-09  3:16 ` [for-linus][PATCH 06/15] tracing: Have current_trace " Steven Rostedt
@ 2023-09-09  3:16 ` Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 08/15] tracing: Have event inject " Steven Rostedt
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton, stable,
	Zheng Yejian, Linux Kernel Functional Testing, Naresh Kamboju

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The option files update the options for a given trace array. For an
instance, if the file is opened and the instance is deleted, reading or
writing to the file will cause a use after free.

Up the ref count of the trace_array when an option file is opened.

Link: https://lkml.kernel.org/r/20230907024804.086679464@goodmis.org
Link: https://lore.kernel.org/all/1cb3aee2-19af-c472-e265-05176fe9bd84@huawei.com/

Cc: stable@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Zheng Yejian <zhengyejian1@huawei.com>
Fixes: 8530dec63e7b4 ("tracing: Add tracing_check_open_get_tr()")
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index b82df33d20ff..0608ad20cf30 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8988,12 +8988,33 @@ trace_options_write(struct file *filp, const char __user *ubuf, size_t cnt,
 	return cnt;
 }
 
+static int tracing_open_options(struct inode *inode, struct file *filp)
+{
+	struct trace_option_dentry *topt = inode->i_private;
+	int ret;
+
+	ret = tracing_check_open_get_tr(topt->tr);
+	if (ret)
+		return ret;
+
+	filp->private_data = inode->i_private;
+	return 0;
+}
+
+static int tracing_release_options(struct inode *inode, struct file *file)
+{
+	struct trace_option_dentry *topt = file->private_data;
+
+	trace_array_put(topt->tr);
+	return 0;
+}
 
 static const struct file_operations trace_options_fops = {
-	.open = tracing_open_generic,
+	.open = tracing_open_options,
 	.read = trace_options_read,
 	.write = trace_options_write,
 	.llseek	= generic_file_llseek,
+	.release = tracing_release_options,
 };
 
 /*
-- 
2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [for-linus][PATCH 08/15] tracing: Have event inject files inc the trace array ref count
  2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
                   ` (6 preceding siblings ...)
  2023-09-09  3:16 ` [for-linus][PATCH 07/15] tracing: Have option files " Steven Rostedt
@ 2023-09-09  3:16 ` Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 09/15] ring-buffer: Avoid softlockup in ring_buffer_resize() Steven Rostedt
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton, stable,
	Zheng Yejian, Linux Kernel Functional Testing, Naresh Kamboju

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The event inject files add events for a specific trace array. For an
instance, if the file is opened and the instance is deleted, reading or
writing to the file will cause a use after free.

Up the ref count of the trace_array when a event inject file is opened.

Link: https://lkml.kernel.org/r/20230907024804.292337868@goodmis.org
Link: https://lore.kernel.org/all/1cb3aee2-19af-c472-e265-05176fe9bd84@huawei.com/

Cc: stable@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Zheng Yejian <zhengyejian1@huawei.com>
Fixes: 6c3edaf9fd6a ("tracing: Introduce trace event injection")
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_inject.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_inject.c b/kernel/trace/trace_events_inject.c
index abe805d471eb..8650562bdaa9 100644
--- a/kernel/trace/trace_events_inject.c
+++ b/kernel/trace/trace_events_inject.c
@@ -328,7 +328,8 @@ event_inject_read(struct file *file, char __user *buf, size_t size,
 }
 
 const struct file_operations event_inject_fops = {
-	.open = tracing_open_generic,
+	.open = tracing_open_file_tr,
 	.read = event_inject_read,
 	.write = event_inject_write,
+	.release = tracing_release_file_tr,
 };
-- 
2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [for-linus][PATCH 09/15] ring-buffer: Avoid softlockup in ring_buffer_resize()
  2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
                   ` (7 preceding siblings ...)
  2023-09-09  3:16 ` [for-linus][PATCH 08/15] tracing: Have event inject " Steven Rostedt
@ 2023-09-09  3:16 ` Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 10/15] tracefs/eventfs: Free top level files on removal Steven Rostedt
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel; +Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton, Zheng Yejian

From: Zheng Yejian <zhengyejian1@huawei.com>

When user resize all trace ring buffer through file 'buffer_size_kb',
then in ring_buffer_resize(), kernel allocates buffer pages for each
cpu in a loop.

If the kernel preemption model is PREEMPT_NONE and there are many cpus
and there are many buffer pages to be allocated, it may not give up cpu
for a long time and finally cause a softlockup.

To avoid it, call cond_resched() after each cpu buffer allocation.

Link: https://lore.kernel.org/linux-trace-kernel/20230906081930.3939106-1-zhengyejian1@huawei.com

Cc: <mhiramat@kernel.org>
Signed-off-by: Zheng Yejian <zhengyejian1@huawei.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/ring_buffer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 78502d4c7214..72ccf75defd0 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2198,6 +2198,8 @@ int ring_buffer_resize(struct trace_buffer *buffer, unsigned long size,
 				err = -ENOMEM;
 				goto out_err;
 			}
+
+			cond_resched();
 		}
 
 		cpus_read_lock();
-- 
2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [for-linus][PATCH 10/15] tracefs/eventfs: Free top level files on removal
  2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
                   ` (8 preceding siblings ...)
  2023-09-09  3:16 ` [for-linus][PATCH 09/15] ring-buffer: Avoid softlockup in ring_buffer_resize() Steven Rostedt
@ 2023-09-09  3:16 ` Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 11/15] ring-buffer: Do not attempt to read past "commit" Steven Rostedt
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton, Ajay Kaher,
	Zheng Yejian, Naresh Kamboju

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

When an instance is removed, the top level files of the eventfs directory
are not cleaned up. Call the eventfs_remove() on each of the entries to
free them.

This was found via kmemleak:

unreferenced object 0xffff8881047c1280 (size 96):
  comm "mkdir", pid 924, jiffies 4294906489 (age 2013.077s)
  hex dump (first 32 bytes):
    18 31 ed 03 81 88 ff ff 00 31 09 24 81 88 ff ff  .1.......1.$....
    00 00 00 00 00 00 00 00 98 19 7c 04 81 88 ff ff  ..........|.....
  backtrace:
    [<000000000fa46b4d>] kmalloc_trace+0x2a/0xa0
    [<00000000e729cd0c>] eventfs_prepare_ef.constprop.0+0x3a/0x160
    [<000000009032e6a8>] eventfs_add_events_file+0xa0/0x160
    [<00000000fe968442>] create_event_toplevel_files+0x6f/0x130
    [<00000000e364d173>] event_trace_add_tracer+0x14/0x140
    [<00000000411840fa>] trace_array_create_dir+0x52/0xf0
    [<00000000967804fa>] trace_array_create+0x208/0x370
    [<00000000da505565>] instance_mkdir+0x6b/0xb0
    [<00000000dc1215af>] tracefs_syscall_mkdir+0x5b/0x90
    [<00000000a8aca289>] vfs_mkdir+0x272/0x380
    [<000000007709b242>] do_mkdirat+0xfc/0x1d0
    [<00000000c0b6d219>] __x64_sys_mkdir+0x78/0xa0
    [<0000000097b5dd4b>] do_syscall_64+0x3f/0x90
    [<00000000a3f00cfa>] entry_SYSCALL_64_after_hwframe+0x6e/0xd8
unreferenced object 0xffff888103ed3118 (size 8):
  comm "mkdir", pid 924, jiffies 4294906489 (age 2013.077s)
  hex dump (first 8 bytes):
    65 6e 61 62 6c 65 00 00                          enable..
  backtrace:
    [<0000000010f75127>] __kmalloc_node_track_caller+0x51/0x160
    [<000000004b3eca91>] kstrdup+0x34/0x60
    [<0000000050074d7a>] eventfs_prepare_ef.constprop.0+0x53/0x160
    [<000000009032e6a8>] eventfs_add_events_file+0xa0/0x160
    [<00000000fe968442>] create_event_toplevel_files+0x6f/0x130
    [<00000000e364d173>] event_trace_add_tracer+0x14/0x140
    [<00000000411840fa>] trace_array_create_dir+0x52/0xf0
    [<00000000967804fa>] trace_array_create+0x208/0x370
    [<00000000da505565>] instance_mkdir+0x6b/0xb0
    [<00000000dc1215af>] tracefs_syscall_mkdir+0x5b/0x90
    [<00000000a8aca289>] vfs_mkdir+0x272/0x380
    [<000000007709b242>] do_mkdirat+0xfc/0x1d0
    [<00000000c0b6d219>] __x64_sys_mkdir+0x78/0xa0
    [<0000000097b5dd4b>] do_syscall_64+0x3f/0x90
    [<00000000a3f00cfa>] entry_SYSCALL_64_after_hwframe+0x6e/0xd8

Link: https://lore.kernel.org/linux-trace-kernel/20230907175859.6fedbaa2@gandalf.local.home

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ajay Kaher <akaher@vmware.com>
Cc: Zheng Yejian <zhengyejian1@huawei.com>
Cc: Naresh Kamboju <naresh.kamboju@linaro.org>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Fixes: 5bdcd5f5331a2 eventfs: ("Implement removal of meta data from eventfs")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 fs/tracefs/event_inode.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index 609ccb5b7cfc..f168aca45458 100644
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -195,17 +195,39 @@ void eventfs_set_ef_status_free(struct tracefs_inode *ti, struct dentry *dentry)
 {
 	struct tracefs_inode *ti_parent;
 	struct eventfs_inode *ei;
-	struct eventfs_file *ef;
-
-	mutex_lock(&eventfs_mutex);
+	struct eventfs_file *ef, *tmp;
 
 	/* The top level events directory may be freed by this */
 	if (unlikely(ti->flags & TRACEFS_EVENT_TOP_INODE)) {
+		LIST_HEAD(ef_del_list);
+
+		mutex_lock(&eventfs_mutex);
+
 		ei = ti->private;
+
+		/* Record all the top level files */
+		list_for_each_entry_srcu(ef, &ei->e_top_files, list,
+					 lockdep_is_held(&eventfs_mutex)) {
+			list_add_tail(&ef->del_list, &ef_del_list);
+		}
+
+		/* Nothing should access this, but just in case! */
+		ti->private = NULL;
+
+		mutex_unlock(&eventfs_mutex);
+
+		/* Now safely free the top level files and their children */
+		list_for_each_entry_safe(ef, tmp, &ef_del_list, del_list) {
+			list_del(&ef->del_list);
+			eventfs_remove(ef);
+		}
+
 		kfree(ei);
-		goto out;
+		return;
 	}
 
+	mutex_lock(&eventfs_mutex);
+
 	ti_parent = get_tracefs(dentry->d_parent->d_inode);
 	if (!ti_parent || !(ti_parent->flags & TRACEFS_EVENT_INODE))
 		goto out;
-- 
2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [for-linus][PATCH 11/15] ring-buffer: Do not attempt to read past "commit"
  2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
                   ` (9 preceding siblings ...)
  2023-09-09  3:16 ` [for-linus][PATCH 10/15] tracefs/eventfs: Free top level files on removal Steven Rostedt
@ 2023-09-09  3:16 ` Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 12/15] tracing: Use the new eventfs descriptor for print trigger Steven Rostedt
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel; +Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton, Tze-nan Wu

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

When iterating over the ring buffer while the ring buffer is active, the
writer can corrupt the reader. There's barriers to help detect this and
handle it, but that code missed the case where the last event was at the
very end of the page and has only 4 bytes left.

The checks to detect the corruption by the writer to reads needs to see the
length of the event. If the length in the first 4 bytes is zero then the
length is stored in the second 4 bytes. But if the writer is in the process
of updating that code, there's a small window where the length in the first
4 bytes could be zero even though the length is only 4 bytes. That will
cause rb_event_length() to read the next 4 bytes which could happen to be off the
allocated page.

To protect against this, fail immediately if the next event pointer is
less than 8 bytes from the end of the commit (last byte of data), as all
events must be a minimum of 8 bytes anyway.

Link: https://lore.kernel.org/all/20230905141245.26470-1-Tze-nan.Wu@mediatek.com/
Link: https://lore.kernel.org/linux-trace-kernel/20230907122820.0899019c@gandalf.local.home

Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Reported-by: Tze-nan Wu <Tze-nan.Wu@mediatek.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/ring_buffer.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 72ccf75defd0..a1651edc48d5 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2390,6 +2390,11 @@ rb_iter_head_event(struct ring_buffer_iter *iter)
 	 */
 	commit = rb_page_commit(iter_head_page);
 	smp_rmb();
+
+	/* An event needs to be at least 8 bytes in size */
+	if (iter->head > commit - 8)
+		goto reset;
+
 	event = __rb_page_index(iter_head_page, iter->head);
 	length = rb_event_length(event);
 
-- 
2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [for-linus][PATCH 12/15] tracing: Use the new eventfs descriptor for print trigger
  2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
                   ` (10 preceding siblings ...)
  2023-09-09  3:16 ` [for-linus][PATCH 11/15] ring-buffer: Do not attempt to read past "commit" Steven Rostedt
@ 2023-09-09  3:16 ` Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 13/15] tracing: Remove unused trace_event_file dir field Steven Rostedt
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel; +Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton, Ajay Kaher

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The check to create the print event "trigger" was using the obsolete "dir"
value of the trace_event_file to determine if it should create the trigger
or not. But that value will now be NULL because it uses the event file
descriptor.

Change it to test the "ef" field of the trace_event_file structure so that
the trace_marker "trigger" file appears again.

Link: https://lkml.kernel.org/r/20230908022001.371815239@goodmis.org

Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ajay Kaher <akaher@vmware.com>
Fixes: 27152bceea1df ("eventfs: Move tracing/events to eventfs")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 0608ad20cf30..122c23c9eb28 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -9792,8 +9792,8 @@ init_tracer_tracefs(struct trace_array *tr, struct dentry *d_tracer)
 			  tr, &tracing_mark_fops);
 
 	file = __find_event_file(tr, "ftrace", "print");
-	if (file && file->dir)
-		trace_create_file("trigger", TRACE_MODE_WRITE, file->dir,
+	if (file && file->ef)
+		eventfs_add_file("trigger", TRACE_MODE_WRITE, file->ef,
 				  file, &event_trigger_fops);
 	tr->trace_marker_file = file;
 
-- 
2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [for-linus][PATCH 13/15] tracing: Remove unused trace_event_file dir field
  2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
                   ` (11 preceding siblings ...)
  2023-09-09  3:16 ` [for-linus][PATCH 12/15] tracing: Use the new eventfs descriptor for print trigger Steven Rostedt
@ 2023-09-09  3:16 ` Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 14/15] selftests/ftrace: Fix dependencies for some of the synthetic event tests Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 15/15] tracing/synthetic: Fix order of struct trace_dynamic_info Steven Rostedt
  14 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel; +Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton, Ajay Kaher

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Now that eventfs structure is used to create the events directory via the
eventfs dynamically allocate code, the "dir" field of the trace_event_file
structure is no longer used. Remove it.

Link: https://lkml.kernel.org/r/20230908022001.580400115@goodmis.org

Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ajay Kaher <akaher@vmware.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 include/linux/trace_events.h |  1 -
 kernel/trace/trace_events.c  | 13 -------------
 2 files changed, 14 deletions(-)

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index eb5c3add939b..12f875e9e69a 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -650,7 +650,6 @@ struct trace_event_file {
 	struct trace_event_call		*event_call;
 	struct event_filter __rcu	*filter;
 	struct eventfs_file             *ef;
-	struct dentry			*dir;
 	struct trace_array		*tr;
 	struct trace_subsystem_dir	*system;
 	struct list_head		triggers;
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 2af92177b765..065c63991858 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -992,19 +992,6 @@ static void remove_subsystem(struct trace_subsystem_dir *dir)
 
 static void remove_event_file_dir(struct trace_event_file *file)
 {
-	struct dentry *dir = file->dir;
-	struct dentry *child;
-
-	if (dir) {
-		spin_lock(&dir->d_lock);	/* probably unneeded */
-		list_for_each_entry(child, &dir->d_subdirs, d_child) {
-			if (d_really_is_positive(child))	/* probably unneeded */
-				d_inode(child)->i_private = NULL;
-		}
-		spin_unlock(&dir->d_lock);
-
-		tracefs_remove(dir);
-	}
 	eventfs_remove(file->ef);
 	list_del(&file->list);
 	remove_subsystem(file->system);
-- 
2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [for-linus][PATCH 14/15] selftests/ftrace: Fix dependencies for some of the synthetic event tests
  2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
                   ` (12 preceding siblings ...)
  2023-09-09  3:16 ` [for-linus][PATCH 13/15] tracing: Remove unused trace_event_file dir field Steven Rostedt
@ 2023-09-09  3:16 ` Steven Rostedt
  2023-09-09  3:16 ` [for-linus][PATCH 15/15] tracing/synthetic: Fix order of struct trace_dynamic_info Steven Rostedt
  14 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton, linux-kselftest,
	Shuah Khan, Naveen N Rao

From: Naveen N Rao <naveen@kernel.org>

Commit b81a3a100cca1b ("tracing/histogram: Add simple tests for
stacktrace usage of synthetic events") changed the output text in
tracefs README, but missed updating some of the dependencies specified
in selftests. This causes some of the tests to exit as unsupported.

Fix this by changing the grep pattern. Since we want these tests to work
on older kernels, match only against the common last part of the
pattern.

Link: https://lore.kernel.org/linux-trace-kernel/20230614091046.2178539-1-naveen@kernel.org

Cc: <linux-kselftest@vger.kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>
Fixes: b81a3a100cca ("tracing/histogram: Add simple tests for stacktrace usage of synthetic events")
Signed-off-by: Naveen N Rao <naveen@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 .../trigger/inter-event/trigger-synthetic-event-dynstring.tc    | 2 +-
 .../inter-event/trigger-synthetic_event_syntax_errors.tc        | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-dynstring.tc b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-dynstring.tc
index 213d890ed188..174376ddbc6c 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-dynstring.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-dynstring.tc
@@ -1,7 +1,7 @@
 #!/bin/sh
 # SPDX-License-Identifier: GPL-2.0
 # description: event trigger - test inter-event histogram trigger trace action with dynamic string param
-# requires: set_event synthetic_events events/sched/sched_process_exec/hist "char name[]' >> synthetic_events":README ping:program
+# requires: set_event synthetic_events events/sched/sched_process_exec/hist "' >> synthetic_events":README ping:program
 
 fail() { #msg
     echo $1
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic_event_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic_event_syntax_errors.tc
index 955e3ceea44b..b927ee54c02d 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic_event_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic_event_syntax_errors.tc
@@ -1,7 +1,7 @@
 #!/bin/sh
 # SPDX-License-Identifier: GPL-2.0
 # description: event trigger - test synthetic_events syntax parser errors
-# requires: synthetic_events error_log "char name[]' >> synthetic_events":README
+# requires: synthetic_events error_log "' >> synthetic_events":README
 
 check_error() { # command-with-error-pos-by-^
     ftrace_errlog_check 'synthetic_events' "$1" 'synthetic_events'
-- 
2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [for-linus][PATCH 15/15] tracing/synthetic: Fix order of struct trace_dynamic_info
  2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
                   ` (13 preceding siblings ...)
  2023-09-09  3:16 ` [for-linus][PATCH 14/15] selftests/ftrace: Fix dependencies for some of the synthetic event tests Steven Rostedt
@ 2023-09-09  3:16 ` Steven Rostedt
  14 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2023-09-09  3:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Andrew Morton, stable, Sven Schnelle

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

To make handling BIG and LITTLE endian better the offset/len of dynamic
fields of the synthetic events was changed into a structure of:

 struct trace_dynamic_info {
 #ifdef CONFIG_CPU_BIG_ENDIAN
	u16	offset;
	u16	len;
 #else
	u16	len;
	u16	offset;
 #endif
 };

to replace the manual changes of:

 data_offset = offset & 0xffff;
 data_offest = len << 16;

But if you look closely, the above is:

  <len> << 16 | offset

Which in little endian would be in memory:

 offset_lo offset_hi len_lo len_hi

and in big endian:

 len_hi len_lo offset_hi offset_lo

Which if broken into a structure would be:

 struct trace_dynamic_info {
 #ifdef CONFIG_CPU_BIG_ENDIAN
	u16	len;
	u16	offset;
 #else
	u16	offset;
	u16	len;
 #endif
 };

Which is the opposite of what was defined.

Fix this and just to be safe also add "__packed".

Link: https://lore.kernel.org/all/20230908154417.5172e343@gandalf.local.home/
Link: https://lore.kernel.org/linux-trace-kernel/20230908163929.2c25f3dc@gandalf.local.home

Cc: stable@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Sven Schnelle <svens@linux.ibm.com>
Fixes: ddeea494a16f3 ("tracing/synthetic: Use union instead of casts")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 include/linux/trace_events.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 12f875e9e69a..21ae37e49319 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -62,13 +62,13 @@ void trace_event_printf(struct trace_iterator *iter, const char *fmt, ...);
 /* Used to find the offset and length of dynamic fields in trace events */
 struct trace_dynamic_info {
 #ifdef CONFIG_CPU_BIG_ENDIAN
-	u16	offset;
 	u16	len;
+	u16	offset;
 #else
-	u16	len;
 	u16	offset;
+	u16	len;
 #endif
-};
+} __packed;
 
 /*
  * The trace entry - the most basic unit of tracing. This is what
-- 
2.40.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2023-09-09  3:24 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-09  3:16 [for-linus][PATCH 00/15] tracing: Fixes for 6.6 Steven Rostedt
2023-09-09  3:16 ` [for-linus][PATCH 01/15] tracefs: Add missing lockdown check to tracefs_create_dir() Steven Rostedt
2023-09-09  3:16 ` [for-linus][PATCH 02/15] tracefs/eventfs: Add missing lockdown checks Steven Rostedt
2023-09-09  3:16 ` [for-linus][PATCH 03/15] tracefs/eventfs: Use dput to free the toplevel events directory Steven Rostedt
2023-09-09  3:16 ` [for-linus][PATCH 04/15] tracing: Increase trace array ref count on enable and filter files Steven Rostedt
2023-09-09  3:16 ` [for-linus][PATCH 05/15] tracing: Have tracing_max_latency inc the trace array ref count Steven Rostedt
2023-09-09  3:16 ` [for-linus][PATCH 06/15] tracing: Have current_trace " Steven Rostedt
2023-09-09  3:16 ` [for-linus][PATCH 07/15] tracing: Have option files " Steven Rostedt
2023-09-09  3:16 ` [for-linus][PATCH 08/15] tracing: Have event inject " Steven Rostedt
2023-09-09  3:16 ` [for-linus][PATCH 09/15] ring-buffer: Avoid softlockup in ring_buffer_resize() Steven Rostedt
2023-09-09  3:16 ` [for-linus][PATCH 10/15] tracefs/eventfs: Free top level files on removal Steven Rostedt
2023-09-09  3:16 ` [for-linus][PATCH 11/15] ring-buffer: Do not attempt to read past "commit" Steven Rostedt
2023-09-09  3:16 ` [for-linus][PATCH 12/15] tracing: Use the new eventfs descriptor for print trigger Steven Rostedt
2023-09-09  3:16 ` [for-linus][PATCH 13/15] tracing: Remove unused trace_event_file dir field Steven Rostedt
2023-09-09  3:16 ` [for-linus][PATCH 14/15] selftests/ftrace: Fix dependencies for some of the synthetic event tests Steven Rostedt
2023-09-09  3:16 ` [for-linus][PATCH 15/15] tracing/synthetic: Fix order of struct trace_dynamic_info Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®